Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Project structure read incorrectly in 0.11.6 #2888

Closed
Gedochao opened this issue Nov 22, 2023 · 6 comments
Closed

Project structure read incorrectly in 0.11.6 #2888

Gedochao opened this issue Nov 22, 2023 · 6 comments

Comments

@Gedochao
Copy link

This is a sequel to #2844, really, as it seems that 0.11.6 merely fails in a different way for https://github.com/VirtusLab/scala-cli now.

We tried to upgrade to 0.11.6 here: VirtusLab/scala-cli#2572

It fails on our integration tests definition with:

[703/1155] runner[3.0.2].publishLocalNoFluff 
Publishing Artifact(org.virtuslab.scala-cli,runner_3,1.0.7-SNAPSHOT) to ivy repo /Users/pchabelski/IdeaProjects/scala-cli/out/repo/1.0.7-SNAPSHOT
[761/1155] test-runner[3.0.2].publishLocalNoFluff 
Publishing Artifact(org.virtuslab.scala-cli,test-runner_3,1.0.7-SNAPSHOT) to ivy repo /Users/pchabelski/IdeaProjects/scala-cli/out/repo/1.0.7-SNAPSHOT
[825/1155] runner[2.13.12].publishLocalNoFluff 
Publishing Artifact(org.virtuslab.scala-cli,runner_2.13,1.0.7-SNAPSHOT) to ivy repo /Users/pchabelski/IdeaProjects/scala-cli/out/repo/1.0.7-SNAPSHOT
[889/1155] test-runner[2.13.12].publishLocalNoFluff 
Publishing Artifact(org.virtuslab.scala-cli,test-runner_2.13,1.0.7-SNAPSHOT) to ivy repo /Users/pchabelski/IdeaProjects/scala-cli/out/repo/1.0.7-SNAPSHOT
[953/1155] runner[2.12.18].publishLocalNoFluff 
Publishing Artifact(org.virtuslab.scala-cli,runner_2.12,1.0.7-SNAPSHOT) to ivy repo /Users/pchabelski/IdeaProjects/scala-cli/out/repo/1.0.7-SNAPSHOT
[1017/1155] test-runner[2.12.18].publishLocalNoFluff 
Publishing Artifact(org.virtuslab.scala-cli,test-runner_2.12,1.0.7-SNAPSHOT) to ivy repo /Users/pchabelski/IdeaProjects/scala-cli/out/repo/1.0.7-SNAPSHOT
[1154/1155] integration.test.forkWorkingDir 
Unknown ctx of target integration.test.test: millbuild.build$CliIntegration$IntegrationScalaTests$TestHelper@50b4364

The important part seems to be the last line:

Unknown ctx of target integration.test.test: millbuild.build$CliIntegration$IntegrationScalaTests$TestHelper@50b4364

Which seems to be coming from here:

throw new MillException(s"Unknown ctx of target ${namedTask}: $unknown")

My best guess is that the fix from #2883 doesn't fix it for our case.

--disable-callgraph-invalidation seems to make the integration tests pass, but that shouldn't be necessary.

@lihaoyi
Copy link
Member

lihaoyi commented Nov 22, 2023

This looks like a code problem: you are defining a T.command inside the class TestHelper that does not extend mill.Module.

That is not expected to work I think: commands must be defined inside modules, and modules must be either traits or objects. Can you move the T.command out of the TestHelper class?

@Gedochao
Copy link
Author

Right. So in other words, we've been using syntax that's no longer supported?

@lefou
Copy link
Member

lefou commented Nov 22, 2023

This looks like a code problem: you are defining a T.command inside the class TestHelper that does not extend mill.Module.

That is not expected to work: commands must be defined inside modules, and modules must be either traits or objects

Might be some copy and paste stuff, which I already fixed in scala-js-cli. VirtusLab/scala-js-cli#39

@Gedochao
Copy link
Author

@lefou right, thanks!
Lemme try to apply your fixes from scala-js-cli in scala-cli then

@lefou
Copy link
Member

lefou commented Nov 22, 2023

Right. So in other words, we've been using syntax that's no longer supported?

More the kind of: You used some code constructs which should never have compiled before, but somehow do. We try hard to make invalid thing not compile, which is not always possible with macros. So we catch those only by accident here. Meaning, defining a T should never be possible if you don't inherit Module.

@Gedochao
Copy link
Author

Managed to make it work, thanks for the tips!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants