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

Fix init command and support running Mill without existing build.sc #2662

Merged
merged 8 commits into from
Jul 29, 2023

Conversation

lefou
Copy link
Member

@lefou lefou commented Jul 17, 2023

I first added an integration test for the init command to reproduce the issues and make sure the feature now works.

Instead of failing in case there is no build.sc, we now check the targets against a whitelist of tasks (init and version) and continue with an empty setup.

We also accept a missing build.sc in case additional imports are given, which could mean, the imports contain Mill plugins which itself provide commands that could work without a build.sc.

Only when we discover an error afterwards, we add a message about the missing build.sc to the underlying error message. That means, we don't see the spurious "no build.sc" message when everything works as expected.

@lefou lefou changed the title Fix init command and support running Mill without existing build.sc Fix init command and support running Mill without existing build.sc Jul 17, 2023
@lefou lefou requested a review from lihaoyi July 17, 2023 09:35
@lefou
Copy link
Member Author

lefou commented Jul 17, 2023

@lihaoyi Before continuing it would be nice to have your feedback on this issue. Beside initialization of new projects an maybe the version command, I don't see why Mill should run without a top-level build.sc. This has often the potential to leave out directories in unwanted locations. OTOH, we have that init feature, which can't run without this ability.

We could make it an built-in option though. We already have --version to avoid evaluating MainModule.version.

@lihaoyi
Copy link
Member

lihaoyi commented Jul 17, 2023

Yeah this goes back to the "how do evaluator commands work" discussion. I'd be fine with either magic whitelisting the init command or baking it in as --init. Since we don't have an overarching principle yet, I expect things would be messy regardless of what we do, and I also expect we'll clean it up later in some new version so I don't have strong opinion on how it's implemented now

I first added an integration test for the init command to reproduce the issues and make sure the feature now works.

Instead of failing in case there is no `build.sc`, we continue with an empty setup.

Only when we discover an error afterwards, we add a message about the missing `build.sc` to the underlying error message.
@lefou lefou marked this pull request as ready for review July 27, 2023 13:00
@lefou lefou requested review from lihaoyi and removed request for lihaoyi July 27, 2023 13:01
@lefou
Copy link
Member Author

lefou commented Jul 27, 2023

I added a whitelist of tasks, so we only continue without a build.sc, if we want to execute a known well-behaving task.

@lefou lefou merged commit 8172ca7 into main Jul 29, 2023
37 checks passed
@lefou lefou deleted the fix-init branch July 29, 2023 09:43
@lefou lefou added this to the 0.11.2 milestone Jul 29, 2023
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

Successfully merging this pull request may close these issues.

Lost ability to install Mill for GHA when build.sc is not at the project root
2 participants