conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.17k stars 974 forks source link

[bug] Conan bugs out when ~/.conan2/extensions/ is removed #14137

Closed iskunk closed 1 year ago

iskunk commented 1 year ago

Environment details

Steps to reproduce

$ conan-new install -g AutotoolsDeps --requires=zlib/1.2.13
[...]
Install finished successfully

$ ls -F ~/.conan2/
extensions/  p/         remotes.json  version.txt
global.conf  profiles/  settings.yml

$ mv ~/.conan2/extensions ~/.conan2/extensions.AWOL

$ conan-new install -g AutotoolsDeps --requires=zlib/1.2.13
ERROR: /home/username/.conan2/extensions/plugins/profile.py not found!

If I remove the extensions directory, then I expect Conan to continue working, sans extensions. Currently, this leaves Conan in an unusable state, with no clear means of restoring its functionality aside from wiping the user config and re-creating it from scratch.


I have a set of changes that allows Conan to handle this properly, and will prepare a PR soon.

Logs

No response

memsharded commented 1 year ago

Hi @iskunk

Thanks for the report, and your offer to contribute.

I think we should probably align the behavior with the management of other files in Conan, like settings.yml, remotes.json and global.conf, files that will be automatically created if not existing, on the fly, when they are requested, in a lazy creation way. So profile.py, compatibility.py files that will make Conan fail if not existing should also be automatically created in the same way. In that regard, this wouldn't be a bug, at the moment, manually changing the contents of the Cache is an unsupported use case. But in any case, it doesn't matter if it is bug fix or a UX improvement, it seems it makes sense to align the behavior for these files.

If we do that, renaming one folder will not be a valid mechanism for deactivating such extensions, because new files will be created on the fly. The correct mechanism for that would be removing the contents of the file.

iskunk commented 1 year ago

The immediate concern is Conan breaking when the directory is removed, and recreating it if absent (similar to when there is no user config to begin with) is a valid way of handling this.

However, I'd point out that (temporarily) removing the directory has its own utility, too. Maybe there is a problem with the extension code, and you want to quickly check how things work without it. Maybe you're concerned that there is bad stuff hiding in those Turing-complete-language files, and you'd feel better just not having them there. Recreating the directory with off-the-shelf code files would be counterproductive in both those cases.

(For my part, I'm not making use of the logic in those files, so I'd be happy not to have an extensions directory at all.)

memsharded commented 1 year ago

However, I'd point out that (temporarily) removing the directory has its own utility, too. Maybe there is a problem with the extension code, and you want to quickly check how things work without it. Maybe you're concerned that there is bad stuff hiding in those Turing-complete-language files, and you'd feel better just not having them there. Recreating the directory with off-the-shelf code files would be counterproductive in both those cases.

Yeah, I totally understand the use case, but I think in this case, it is more important to keep the consistency in the behavior for all files. For example, if to debug some remote issues you are removing or renaming the remotes.json file, a new one will be created with the default remote enabled. Same for global.conf, at the moment is not critical because the default one only contains comments, but if it changes in future Conan versions and it defaults to some enabled new confs, we would have the same problem.

Then, there is also the problem of the migrations. When a new Conan version is released it will run migrations, that will check the default implementation of the files and update them if they havent been modified by users, or create them if not existing.

I will discuss this item with the team to get more feedback about what would be the best approach, please wait a bit until we decide on it. Thanks!

iskunk commented 1 year ago

I appreciate the consideration.

I'd suggest this as a possible point of compromise: Recreate the extensions directory if missing, but if a regular file with the same name is present, then leave it be. (This file could contain a note like "This Conan config does not use extensions")

Also or alternately, allow the specific .py files that Conan looks for to be empty, or contain nothing more than a comment. This would allow a user to "reject" some auto-created extensions, while making use of others.

iskunk commented 1 year ago

FWIW, here are my changes for this. I just added appropriate try-catches around the bits that read extension files. If the long-term fix is going to need more time to figure out / implement, then this can be a stopgap measure that simply gets rid of the error condition.

memsharded commented 1 year ago

After discussing with the team it seems we want to go in this direction:

iskunk commented 1 year ago

Sounds good, especially the warning. I'd like to give the implementation of this a try, once you have it.

memsharded commented 1 year ago

I am proposing https://github.com/conan-io/conan/pull/14376 with the original idea of not using plugins if the files are removed, without errors and without regenerating the files. I have gone this way after the discussions for 2 reasons:

Maybe the behavior "only core files like settings.yml or remotes.json are regenerated if they are missing", but not "plugins" also makes sense. I will discuss these ideas and new perspective with the team.

memsharded commented 1 year ago

Finally https://github.com/conan-io/conan/pull/14376 closed this ticket by implementing a clear reporting of the error if a expected file has been manually removed from the cache, and the actions that should be done (editing the files, not removing it), after re-reading the threads with the team and discussing the different possibilities.

haleyk10198 commented 8 months ago

Finally #14376 closed this ticket by implementing a clear reporting of the error if a expected file has been manually removed from the cache, and the actions that should be done (editing the files, not removing it), after re-reading the threads with the team and discussing the different possibilities.

Can we please have an additional message on how to recover / reinstall this profile.py, or whatever the general actions should the general users take if they accidentally cleared this?

memsharded commented 8 months ago

Thanks for the feedback @haleyk10198

The easiest action would be to remove the cache folder, and everything will be regenerated.

This one should be very easy to improve, do you want to submit a PR yourself? The current message reads: The 'profile.py' plugin file doesn't exist. If you want to disable it, edit its contents instead of removing it. Then it would be adding "remove the cache folder and the file will be regenerated?"

haleyk10198 commented 8 months ago

A clean command suggestion would be better I'd say, as a passer-by that runs the cmake | npm build equivalent may not know how cache works in Conan.

In my case, my intuition of removing the cache folder led me into running conan cache clean which did not fix the issue

memsharded commented 8 months ago

A clean command suggestion would be better I'd say, as a passer-by that runs the cmake | npm build equivalent may not know how cache works in Conan.

But they did remove the extensions folder in the cache in the first place, even if they don't know how the cache works?

The cache is a private implementation thing of Conan, it is not intended to be modified by users. It is explicitly documented in https://docs.conan.io/2/knowledge/guidelines.html

My concern is that we cannot protect against all possible ways in which users can be modifying the cache, and all will have different modes of erroring, and we cannot provide commands to remediate all possible cache corruptions due to forbidden user modifications, this would be an endless work.

haleyk10198 commented 7 months ago

Sorry for being late on coming back to this.

Now knowing cache is an internal thing without API promises I would definitely agree not to prompt a command. Wondering if there's something that we could do better in explaining which folder should conceptually map to cache though.

For example, I built up a bad habit of panick removing cd .conda; rm -rf * from using anaconda and hence cd .conan; rm -rf * which didn't solve the issue. Does the cache actually mean .conan.db here?

( as for why i cd into rm instead of just rm all, I have .conan tied to a soft link so i don't have to read docs on how to configure .conan to somewhere with more space, it's a very niche use case i guess )

memsharded commented 7 months ago

For example, I built up a bad habit of panick removing cd .conda; rm -rf from using anaconda and hence cd .conan; rm -rf which didn't solve the issue. Does the cache actually mean .conan.db here?

Actually , removing the <userhome>/.conan2 cache also completely removes things (unless you relocated the storage with storage_path conf), including the DB, the extensions, etc. so it is good to start from scratch. The problem is only when removing a random folder in the cache that is expected to be there if the rest of the cache is there.