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

Prevent conan alias from overwriting existing packages #4423

Closed
monsdar opened this issue Jan 30, 2019 · 4 comments · Fixed by #4495
Closed

Prevent conan alias from overwriting existing packages #4423

monsdar opened this issue Jan 30, 2019 · 4 comments · Fixed by #4495
Assignees
Labels
complex: low component: ux No changes to core business logic priority: medium
Milestone

Comments

@monsdar
Copy link
Contributor

monsdar commented Jan 30, 2019

When creating an alias package with conan alias it is easy to mix up the order of parameters:

  • Correct: conan alias boost/ALIAS@conan/stable boost/1.69.0@conan/stable
  • Wrong: conan alias boost/1.69.0@conan/stable boost/ALIAS@conan/stable

In case you're accidentally calling the wrong order you're overwriting the package you initially wanted to reference. If this happens you render your original package useless and need to remove it. Then you can start all over again.
It would be perfect to have a built-in check that tells the user that the existing package boost/1.69.0@conan/stable already exists and will be overwritten by an alias package - and if the user is really sure this is what he wants to do.

Happens to me with Conan 1.11.0.

@monsdar
Copy link
Contributor Author

monsdar commented Jan 30, 2019

I just found that a similar feature has been discussed within #3805 - the issue is just named in a way that it's hard to see on first glance what's going on there.

@memsharded
Copy link
Member

I have added it as "ux", because it is mostly a check. There are some possibilities:

  • Throw an error if the package exists, and require --force. Good, but will be breaking.
  • Interactive: "are you sure you want to overwrite?". This might break also users using alias in CI (which is typical)
  • Opt-in: define a --check argument (or something similar), that forces the alias to do this check, and will throw an error

It is a simple feature, but I see no way to do it both effectively and nonbreaking.

@monsdar
Copy link
Contributor Author

monsdar commented Jan 31, 2019

Agree, --check is not the most intuitive. This kind of issue happens to people who are using conan alias for the first few times - typically before you know about optional parameters. When a user knows about the possible parameters he probably also knows in which order to use conan alias.

I'd say it's better to have it than to miss it.

There's another option that is non-breaking and more intuitive: Add a new command like conan alias-with-check (I'm sure there's a better name for it) that'll do the same as conan alias --check. This would be non-breaking, but newer users would stumble upon it intuitively. Though this would add to the complexity of conan itself (new command, possible issues within future versions, users mixing up alias and alias-with-check...). Just wanted to throw it out there.

@memsharded
Copy link
Member

Another idea which might get the consensus as non-breaking: throw an error if the package already exists and it is NOT an alias already. This was the case that would be breaking, when you want to redefine an existing alias. But a check that you are not breaking an existing package can be totally OK

@jgsogo jgsogo self-assigned this Feb 8, 2019
@ghost ghost added stage: review and removed stage: queue labels Feb 8, 2019
@ghost ghost removed the stage: review label Feb 11, 2019
@memsharded memsharded added this to the 1.13 milestone Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complex: low component: ux No changes to core business logic priority: medium
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants