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

Tidy up init command impl, make it unpack into current folder #3626

Merged
merged 57 commits into from
Sep 30, 2024

Conversation

lihaoyi
Copy link
Member

@lihaoyi lihaoyi commented Sep 30, 2024

Unpacking into the current folder is much more useful due to the fact that we recommend people use a bootstrap script.

  • With the previous approach, you end up with a bootstrap script in the parent dir and a subfolder with another bootstrap script with the real build inside.
  • With the new approach, you download a bootstrap script, run init, and it fleshes out the example in place

Since we now unpack into an existing folder that may or may not have things already in it, we add a new heuristic to fail early if the unpacking would over-write any existing files. This is a compromise, since we cannot easily assume the folder is 100% empty (what if there's a .DS_STORE in it???), nor can we remove any previous files we do not understand (what if there's already a .gitignore or .git?), and we also do not want to stomp over any existing files (what if there's already a build.mill and someone accidentally calls init?).

Tweak the implementation to avoid using so many nested Trys, normal exception handling is enough

Tested manually via ./mill installLocalCache

@lihaoyi lihaoyi marked this pull request as ready for review September 30, 2024 08:35
@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 30, 2024

CC @pawelsadlo

Copy link
Member

@lefou lefou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be at least a message/warning for every file we delete or overwrite.

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 30, 2024

@lefou with the current PR impl, we do not delete/overwrite any file, and fail early if there's any conflict between the zip example and the files already on disk

@lihaoyi
Copy link
Member Author

lihaoyi commented Sep 30, 2024

There's something funky with ./mill -iw main.init.test looping indefinitely, but that's already present in master and not unique to this PR. Will look into it

@lihaoyi lihaoyi merged commit 754b2eb into com-lihaoyi:main Sep 30, 2024
24 checks passed
@lefou
Copy link
Member

lefou commented Sep 30, 2024

I was referring to the deletion of a pre-existing mill script.

os.remove(T.workspace / "mill")

@lihaoyi lihaoyi added this to the 0.12.0-RC3 milestone Sep 30, 2024
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.

3 participants