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 Metals import instructions #2605

Merged
merged 1 commit into from
Dec 2, 2022
Merged

Fix Metals import instructions #2605

merged 1 commit into from
Dec 2, 2022

Conversation

adpi2
Copy link
Collaborator

@adpi2 adpi2 commented Dec 2, 2022

For some reason, if I don't run clean before mill.contrib.bloop.Bloop/install, Mill only generates five Bloop configuration files, instead of 77. @julienrf was also able to reproduce this.

For now I think it is safer to ask people to always run ./mill clean before ./mill mill.contrib.bloop.Bloop/install

For some reason, if I don't run `clean` before `mill.contrib.bloop.Bloop/install`, Mill only generate five Bloop configuration files, instead of 77. @julienrf was also able to reproduce this.

For now I think it is safer to ask people to always run `clean` before `./mill mill.contrib.bloop.Bloop/install`
Copy link
Collaborator

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM! I have managed to import via metals without issues previously, but not sure if I needed the clean.

@tgodzik tgodzik merged commit de7ac23 into main Dec 2, 2022
@tgodzik tgodzik deleted the adpi2-patch-1 branch December 2, 2022 13:13
@lefou
Copy link
Collaborator

lefou commented Dec 2, 2022

For some reason, if I don't run clean before mill.contrib.bloop.Bloop/install, Mill only generates five Bloop configuration files, instead of 77. @julienrf was also able to reproduce this.

For now I think it is safer to ask people to always run ./mill clean before ./mill mill.contrib.bloop.Bloop/install

Hi, I'm interested in a reproduction of this. As maintainer of Mill, I'm curious, whether this is some issue in Mill or specific to mill.contrib.bloop.

@adpi2
Copy link
Collaborator Author

adpi2 commented Dec 2, 2022

@lefou thanks for jumping in.

This should reproduce the bug in coursier/coursier:

  1. Run ./mill clean
  2. Run ./mill mill.contrib.bloop.Bloop/install => it creates 77 Json file in .bloop
  3. Run rm -Rf .bloop
  4. Run ./mill mill.contrib.bloop.Bloop/install => it does not create any Json file

@lefou
Copy link
Collaborator

lefou commented Dec 3, 2022

This looks like an design problem of the Bloop plugin. The <module>.bloop.writeConfig is designed as target, which is cached, but it writes a file outside of T.dest. Instead, it should be a command, which is not cached.

@lefou
Copy link
Collaborator

lefou commented Dec 3, 2022

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