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

[feature] Integrity check in install command #16262

Closed
1 task done
sharpepd opened this issue May 15, 2024 · 10 comments
Closed
1 task done

[feature] Integrity check in install command #16262

sharpepd opened this issue May 15, 2024 · 10 comments
Assignees

Comments

@sharpepd
Copy link

What is your suggestion?

Hi! We are now using Conan 2.x (specifically on 2.1.0) and are looking at the "cache check-integrity" command.
This is great and something we feel we were missing, as it seems too easy for people change the cache content from that on the server and this allows us to at least check if that happens.

However, if we want to be sure that the Conan cache has the latest content from the Conan server when we install dependencies, the workflow seems to be:

  1. call conan install
  2. check integrity
  3. If integrity check fails, remove local version from cache and install again.

Are we missing a trick?

It would be handy if we could tell the Conan install command we want to ensure that the cache is in sync with the server. I looked at the --update option, but that does not seem to be doing the integrity check - just checking if a newer version is available.

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded memsharded self-assigned this May 15, 2024
@memsharded
Copy link
Member

Hi @sharpepd

Thanks for your suggestion.

This is intentional at the moment. Packages are almost never corrupted at download or install time, and the conan install already has too many command line args, so we were trying not to add more.

The integrity checks makes a bit more sense to check if the users have manually modified the cache, like editing some files via an editor that found those files with its navigation capabilities. This is the reason the check is done as a conan cache check-integrity operation, or in the conan upload command, to make sure that corrupted packages are not uploaded.

Also, the conan install implementation code is already complex. It is not planned to add this kind of auxiliary behavior in it, the extra complexity on the code would be bad for the overall maintenance of the project, specially because this should be quite an exceptional case, and considered more an error to be aware and solve the root cause, instead of having a conan install command to automatically/silently fix it.

So I am afraid this would be too much complexity for the value, and not very aligned with the idea that this error should be an exceptional thing and not part of the normal dependencies installation flow. Thanks very much for the feedback anyway!

@memsharded
Copy link
Member

This is great and something we feel we were missing, as it seems too easy for people change the cache content from that on the server and this allows us to at least check if that happens.

It is also not completely clear what it means that people can change the content on the server, this is not a thing, server doesn't have caches, and they cannot corrupt packages, because they store things in .tgz format.

@sharpepd
Copy link
Author

Thanks for your comments.
For clarification, I was not saying the content on the server is getting corrupted, but rather that users modify the local cache. I just want to be sure when I do a Conan install that the correct content is in the local cache.

It kinda feels like that should be default behaviour, but also understand why it is not.

If you are not going to add this to install, do you have any suggestions on how we can ensure the integrity of the cache?

@memsharded
Copy link
Member

For clarification, I was not saying the content on the server is getting corrupted, but rather that users modify the local cache. I just want to be sure when I do a Conan install that the correct content is in the local cache.

But it is happening that often? This should be pretty unusual from my experience.

It kinda feels like that should be default behaviour, but also understand why it is not.

As another extra reason, there is also the cost. The operation can be pretty slow if there are many large packages. Having the time of conan install that slow even when packages are in the cache would be annoying for many users.

If you are not going to add this to install, do you have any suggestions on how we can ensure the integrity of the cache?

Have you considered doing a custom command that does the integrity checks as you want? Custom commands are very powerful feature in Conan 2 and allows to do this relatively easily

@memsharded
Copy link
Member

Hi @sharpepd

Any further feedback here? I was interested in learning about the frequency you are seeing these issues.
Also if you have checked the custom commands, that can provide this functionality from the user side without being built-in.

@memsharded memsharded added staled The issue has been inactive for a while and will be closed soon unplanned labels Jul 29, 2024
@sharpepd
Copy link
Author

No, it is not happening often. But when it does, it does cause confusion, as not all users are "power users" of Conan. From their perspective, they are running a script to prepare the working tree for development and when it goes wrong (because the cache is corrupted) then they usually have to ask for help. It would be nice to be able to automatically detect that this has happened. In some cases (for legacy reasons) we are creating symbolic links to the cache contents, which increases the risk of corruption.

@memsharded
Copy link
Member

Understood, thanks for the feedback.

Still, it seems that the value/effort+risk ratio is a bit too low. I am leaving this ticket open but labeled as "unplanned" just in case, but I'd recommend to explore the custom command approach. We are also trying to add something like #16646, which would highly reduce some of the "accidental but kind of expected" corruptions in the cache, so maybe after that feature has been stabilized we could analyze this again.

@memsharded memsharded added unplanned and removed staled The issue has been inactive for a while and will be closed soon unplanned labels Sep 26, 2024
@memsharded
Copy link
Member

I was re-considering this.
But I have done some quick tests, and it happens that the integrity check is quite slow, it can easily add 5-10 secs for a regular conan install with all the dependencies in the cache, for a relatively mid-size graph (around 50 packages).

If this was enabled for all developers, to be executed by default (or with a global.conf that is defined organization wise, but for developers it would be by default), then the developers perception and feeling would be that Conan is slow, and why it is so slow if everything is already in my local cache.

I think that it is possible to also improve this via other tooling, like configuring the IDE to mark ReadOnly files, I think that VSCode and Jetbrains CLion have this feature, have you considered it?

@memsharded
Copy link
Member

Even if I have the code in a branch to implement this feature, I think this would be a net negative effect on developer experience, and I would explore other approaches like IDE configuration to prevent possible issues. So I am closing this as unplanned at the moment, feel free to re-open if there are further questions or you think there might be other reasons to re-consider. Thanks again for your feedback!

@memsharded memsharded closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
@sharpepd
Copy link
Author

Thank you for your consideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants