Skip to content
New issue

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

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

Already on GitHub? # to your account

Mask colons in segment paths #2965

Merged
merged 7 commits into from
Jan 23, 2024
Merged

Mask colons in segment paths #2965

merged 7 commits into from
Jan 23, 2024

Conversation

lefou
Copy link
Member

@lefou lefou commented Jan 10, 2024

Again a Windows thing, but the evaluator paths should not contain colons. We mask them with $colon.

Fix #2923

@lihaoyi
Copy link
Member

lihaoyi commented Jan 13, 2024

Since we're escaping things with $, should we also mask $ with $$ to avoid potential conflicts?

@lefou
Copy link
Member Author

lefou commented Jan 14, 2024

Since we're escaping things with $, should we also mask $ with $$ to avoid potential conflicts?

Can you a bit more specific? What do we mask with $ and which conflicts do you expect with this concrete $colon?

@lefou
Copy link
Member Author

lefou commented Jan 19, 2024

@lihaoyi Did you mean the name mangling for private inner classes in Scala? Or something else?

I think the masking I introduce here doesn't need to be bijective. It's only from segment path to file path. So, the question is, will two different segment paths project to the same file path?

@lihaoyi
Copy link
Member

lihaoyi commented Jan 20, 2024

@lefou oh I wasn't thinking about Scala compiler name mangling, I was more thinking about potential collisions caused by Mill's own mangling. Like if we have two tasks, with names a:b and a$colonb, they would collide unless we escaped the $ such that a$colonb becomes a$$colonb

This isn't an edge case likely to cause any issues in practice, but it's definitely a thing

Base automatically changed from reserved-names to main January 22, 2024 14:35
@lefou lefou marked this pull request as ready for review January 22, 2024 16:23
@lefou lefou merged commit 8902251 into main Jan 23, 2024
38 checks passed
@lefou lefou deleted the mask-colons branch January 23, 2024 09:09
@lefou lefou added this to the 0.11.7 milestone Jan 23, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mill cache/metadata path for cross segments need to escape colons (:) on Windows
2 participants