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

Use unique out path for private named tasks - fixes #2107 #2108

Merged
merged 3 commits into from
Jan 21, 2023

Conversation

lefou
Copy link
Member

@lefou lefou commented Nov 8, 2022

This change ensures, we always use the super-out path when we try to store metadata and scratch files for private named tasks. As private tasks can't be seen from other traits/classes in the same module hierarchy, our previous logic to detect potential collisions of the metadata location fails. Hence we now keep the isPrivate property in the named target.

I also added a test case that reproduce the initial issue but now succeeds.

This is a binary compatibility breaking change.

By applying the same approach and tracking also public-ness of a target, there is potential to fix also other issues like

This PR should be merged after

@lefou lefou changed the title Added test case for issue https://github.com/com-lihaoyi/mill/issues/2107 Added test case for clashing private targets out-files issue #2107 Jan 16, 2023
@lefou lefou changed the title Added test case for clashing private targets out-files issue #2107 Added test case for clashing private target out-files issue #2107 Jan 16, 2023
@lefou lefou changed the title Added test case for clashing private target out-files issue #2107 Test case for clashing private target out-files issue #2107 Jan 16, 2023
@lefou lefou added the compat-breaker This PR is good to merge, but breaks compatibility and needs to wait till we prepare a major release label Jan 19, 2023
@lefou
Copy link
Member Author

lefou commented Jan 19, 2023

As this is a binary breaking change, the bootstrap tests currently fail. I need to prepare and apply some bootstrap patch, so we don't use external incompatible plugins.

@lefou lefou changed the title Test case for clashing private target out-files issue #2107 Use unique out path for private named tasks - fixes #2107 Jan 19, 2023
@lefou
Copy link
Member Author

lefou commented Jan 19, 2023

I added the bootstrap patch, all tests succeed now.

@lefou lefou requested review from lihaoyi and lolgab January 19, 2023 09:17
@lefou lefou marked this pull request as ready for review January 19, 2023 09:17
@lefou lefou added this to the 0.11.0-M3 milestone Jan 19, 2023
lefou added 3 commits January 21, 2023 16:18
That way, we can better determine the cache/dest location of a task,
as we always need to factor in the full origin.
@lefou lefou merged commit 5f8de2a into com-lihaoyi:main Jan 21, 2023
@lefou lefou deleted the private-task-mix branch January 21, 2023 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compat-breaker This PR is good to merge, but breaks compatibility and needs to wait till we prepare a major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

private targets name clashes overwrite outputs
1 participant