conan-io / conan

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

conan short_paths are not deterministic when CONAN_USER_HOME is different #5284

Closed david-sinuela-pix4d closed 5 years ago

david-sinuela-pix4d commented 5 years ago

(following the same issue structure as https://github.com/conan-io/conan/issues/3971)

What challenge are you facing?

This issue is a continuation of https://github.com/conan-io/conan/issues/3971. Our CI system is configured to reuse some Windows VMs machines during the day and they are re-created from scratch the day after. Many jobs land on the same machine but are "isolated" in different build folders, something of the lines of C:\task_1, C:\task_2, etc.

We build some packages that unfortunately generate very long paths so we have to enable the short_paths feature or they don't compile.

Last week, we had problems sharing the conan caches between tasks (we changed global settings in some tasks), so we started implementing more isolation by defining different CONAN_USER_HOME and CONAN_USER_HOME_SHORT per task.

We are also using clcache to save compilation time on big projects. As in https://github.com/conan-io/conan/issues/3971, in order to maximize the use of the compiler cache the short paths should be deterministic globally or relative to CLCACHE_BASEDIR.

We tried the following:

With this all the paths generated during the build should be deterministic relative to CLCACHE_BASEDIR.

We tried and we observed that the short paths generated are different each build, for example:

So the compiler cache cannot be reused between jobs.

We saw this problem in production with conan 1.14.0 but the issue seems to be present in the develop branch too.

What would make this better?

The paths generated when short paths is enabled should be deterministic even if the CONAN_USER_HOME changes.

Are you interested in implementing this yourself?

Yes, I had a look at the code and I think the issue is that the path given as input to hashed_redirect already contains CONAN_USER_HOME as a prefix. I will submit a PR.

memsharded commented 5 years ago

Hi @david-sinuela-pix4d-dev

I have 2 concerns with this change:

The second one is not big issue, but the first one, I don't know what could be done. As CONAN_USER_HOME and CONAN_USER_HOME_SHORT are not necessarily related, I think it cannot be assumed that different CONAN_USER_HOMES should result in the same shortened path.

What do you think?

david-sinuela-pix4d commented 5 years ago

Hi @memsharded, thanks for the feedback.

As support for the discussion, here is a table with all possible combinations:

USER_HOME default USER_HOME set
USER_HOME_SHORT default home user / short path global home local / short path global
USER_HOME_SHORT set home user / short path local home local / short path local

user = user home, for example C:\Users\ci_user local = user assigned path, for example C:\Users\ci_user\conan global = fixed global path, for example C:\conan

With USER_HOME set and USER_HOME_SHORT default, there is indeed an increased chance of collisions. But that would happen if two runs of conan with different USER_HOME fetch two different recipes with the same reference and the same package id, is that correct? What are the chances? If you consider this a very problematic scenario we could only remove the USER_HOME prefix from the hash computation if USER_HOME_SHORT is defined.

Regarding the changing paths, yes, they will have a different root but what matters is that the relative path within CLCACHE_BASEDIR is the same.

david-sinuela-pix4d commented 5 years ago

Ping, should I contribute a PR and we discuss over the implementation?

david-sinuela-pix4d commented 5 years ago

We discussed in the office and we will not attempt to isolate conan runs using separate homes, so this issue is not relevant for us anymore. I will close it and the PR. If anybody is interested feel free to re-open the topic.