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

Make duplicate output paths an error instead of a warning #682

Closed
matts1 opened this issue Mar 16, 2023 · 1 comment · Fixed by #683
Closed

Make duplicate output paths an error instead of a warning #682

matts1 opened this issue Mar 16, 2023 · 1 comment · Fixed by #683

Comments

@matts1
Copy link
Contributor

matts1 commented Mar 16, 2023

pkg_tar can detect when there are file conflicts (code). However, when it detects them, it prints them instead of failing. I see no reason why anyone would want it on print other than for backwards compatibility (which I assume is why it was first added), but if anyone can think of a reason, please tell me.

I'm happy to implement this myself. However, there are a few ways we could do it, and I don't know what the best approach would be to take. I was looking for approval / feedback on the best approach to take.

Option 1: Simple cutoff
Change print to fail, and maybe increment the major version to indicate backwards incompatible changes. Alternatively, we may consider waiting for a major version increase to do this.

Option 2: Feature flag
Create a bool_flag that controls whether it prints or fails. Initially default it to false, and when we release other backwards incompatible changes, we can set it to true.

@aiuto
Copy link
Collaborator

aiuto commented Mar 16, 2023

Agreed. It should fail, and it only prints a warning for backwards compatibility with bad use cases. I am happy to go with option 2, but with a rule attribute rather than fail as the default.
allow_duplicates_with_different_content=False. That name nicely hints that you are asking for something bad if you set it true. On fail, the messages can say what attribute to add.

I think the backwards compatibility problem is not as bad as it could be. There are a lot of dups, but for the same source file. Few people are doing sophisticated attribute rewrites and also having dups. If a large organization really needs a global default change, they can vendor in the rules and fix it there.

We'll do the next release as 0.9.0.

matts1 added a commit to matts1/rules_pkg that referenced this issue Mar 16, 2023
See bazelbuild#682 for discussion about the appropriate action to take.
aiuto added a commit that referenced this issue Sep 16, 2023
* Add support for failing on file conflicts.

See #682 for discussion about the appropriate action to take.

* Update pkg_files.bzl

---------

Co-authored-by: aiuto <[email protected]>
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 a pull request may close this issue.

2 participants