conan-io / conan

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

[question] Run scripts when doing `conan config install` #8261

Closed madebr closed 1 year ago

madebr commented 3 years ago

Hello!

For supporting some retro computing folks, I have created a ps2toolchain conan recipe at https://github.com/madebr/conan-ps2dev. This effectively packages the ps2toolchain at https://github.com/ps2dev/ps2toolchain/. This ps2toolchain is currently using gcc 3.2. This (old) gcc version is not added to the stock settings.yml of conan. The playstation2 os is also not available. I currently add it as follows: https://github.com/madebr/conan-ps2dev/blob/abed78f4fc9afdbc4315d0d5ce6809fb96c764fe/.github/workflows/build-cmake-conan.yml#L34-L39 Is there a better way to do this?

On #conan at cpplang.slack, I have received the suggestion to use conan config install. When modifying settings.yml and installing it this way, the target settings.yml is overwritten. This means that all user customizations are lost.

It was also suggested to open a pr here to add playstation2 and 3.2 compiler version to the default settings.yml in this repo. I don't think this is desired as this will open floodgates for pr's wanting to add their favourite operating system.

madebr commented 3 years ago

I have created a small POC in the configinstall branch at my fork. When doing conan config install /path/to/configinstall.py, it is able to transform settings.yml. It should also be possible to transform other key conan files (such as conan.conf)

When using the following file:

configinstall.py

def settings_transform(settings):
    settings["os"]["playstation2"] = None
    settings["compiler"]["gcc"]["version"].append("3.2")
    settings["compiler"]["gcc"]["version"] = sorted(set(settings["compiler"]["gcc"]["version"]))
    return settings

And use it with conan config install configinstall.py, os=playstation2 and self.settings.compiler.version=3.2 become available.

memsharded commented 3 years ago

Yep, I have tried to do something similar (round trip yaml) a couple of times, and failed. The problem I have with the current pyyaml implementation is that it doesn't allow round trip, so it will not preserve format, comments, etc. This is unfortunate, because we are using the file contents to know if it has been user modified or not, in order to upgrade to the latest one. being sure we are not overwriting user one. So applying the above transform will make changes, so the next Conan version will not know to automatically upgrade the settings.yml anymore, and then you will have the playstation2 and gcc 3.2 settings, but not the new one. Exactly the same as if you capture today the settings.yml, add the values manually and put it in a gist forever.

So we would need a full round-trip yaml functionality, and that means changing the pyyaml dependency, and has some risk, maybe something to check for Conan 2.0.

madebr commented 3 years ago

Indeed, the built-in settings.yml contains references, which are not preserved by this.

I was thinking about adding a hook that inserts itself after the settings.yml file is read by conan. Does such a hook already exist? Would you allow one to be added?

I think a combination of conan config install and conan hook could be useful.

memsharded commented 3 years ago

I think there is no available extension point for such a hook, it would need to be developed, not sure if it is worth, because this still reads like a workaround and the proper solution is still the ability to programmatically modify the settings.yml, we should focus on that, IMHO.

madebr commented 3 years ago

I've switched the use of PyYAML with ruamel.yaml (https://github.com/madebr/conan/tree/configinstall) The only diffs after using the following configinstall.py:

def settings_transform(settings):
    from conans import tools
    settings["os"]["playstation2"] = None
    settings["compiler"]["gcc"]["version"].append("3.2")
    settings["compiler"]["gcc"]["version"].sort(key=tools.Version)
    return settings

is whitespace:

Diff before and after ```patch --- settings.yml.original 2020-12-29 01:08:02.883091476 +0100 +++ settings.yml 2020-12-29 01:08:03.454095240 +0100 @@ -1,12 +1,16 @@ - # Only for cross building, 'os_build/arch_build' is the system that runs Conan os_build: [Windows, WindowsStore, Linux, Macos, FreeBSD, SunOS, AIX] -arch_build: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv5el, armv5hf, armv6, armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, mips64, avr, s390, s390x, sh4le] +arch_build: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv5el, armv5hf, armv6, + armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, + mips64, avr, s390, s390x, sh4le] # Only for building cross compilation tools, 'os_target/arch_target' is the system for # which the tools generate code -os_target: [Windows, Linux, Macos, Android, iOS, watchOS, tvOS, FreeBSD, SunOS, AIX, Arduino, Neutrino] -arch_target: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv5el, armv5hf, armv6, armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, mips64, avr, s390, s390x, asm.js, wasm, sh4le] +os_target: [Windows, Linux, Macos, Android, iOS, watchOS, tvOS, FreeBSD, SunOS, AIX, + Arduino, Neutrino] +arch_target: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv5el, armv5hf, armv6, + armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, + mips64, avr, s390, s390x, asm.js, wasm, sh4le] # Rest of the settings are "host" settings: # - For native building/cross building: Where the library/program will run. @@ -21,15 +25,20 @@ version: ["5.0", "6.0", "7.0", "8.0"] Linux: Macos: - version: [None, "10.6", "10.7", "10.8", "10.9", "10.10", "10.11", "10.12", "10.13", "10.14", "10.15", "11.0"] + version: [None, "10.6", "10.7", "10.8", "10.9", "10.10", "10.11", "10.12", + "10.13", "10.14", "10.15", "11.0"] Android: api_level: ANY iOS: - version: ["7.0", "7.1", "8.0", "8.1", "8.2", "8.3", "9.0", "9.1", "9.2", "9.3", "10.0", "10.1", "10.2", "10.3", "11.0", "11.1", "11.2", "11.3", "11.4", "12.0", "12.1", "12.2", "12.3", "12.4", "13.0", "13.1", "13.2", "13.3", "13.4", "13.5", "13.6"] + version: ["7.0", "7.1", "8.0", "8.1", "8.2", "8.3", "9.0", "9.1", "9.2", "9.3", + "10.0", "10.1", "10.2", "10.3", "11.0", "11.1", "11.2", "11.3", "11.4", + "12.0", "12.1", "12.2", "12.3", "12.4", "13.0", "13.1", "13.2", "13.3", + "13.4", "13.5", "13.6"] watchOS: version: ["4.0", "4.1", "4.2", "4.3", "5.0", "5.1", "5.2", "5.3", "6.0", "6.1"] tvOS: - version: ["11.0", "11.1", "11.2", "11.3", "11.4", "12.0", "12.1", "12.2", "12.3", "12.4", "13.0"] + version: ["11.0", "11.1", "11.2", "11.3", "11.4", "12.0", "12.1", "12.2", + "12.3", "12.4", "13.0"] FreeBSD: SunOS: AIX: @@ -38,20 +47,20 @@ Emscripten: Neutrino: version: ["6.4", "6.5", "6.6", "7.0", "7.1"] -arch: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv4, armv4i, armv5el, armv5hf, armv6, armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, mips, mips64, avr, s390, s390x, asm.js, wasm, sh4le] + playstation2: +arch: [x86, x86_64, ppc32be, ppc32, ppc64le, ppc64, armv4, armv4i, armv5el, armv5hf, + armv6, armv7, armv7hf, armv7s, armv7k, armv8, armv8_32, armv8.3, sparc, sparcv9, + mips, mips64, avr, s390, s390x, asm.js, wasm, sh4le] compiler: sun-cc: version: ["5.10", "5.11", "5.12", "5.13", "5.14", "5.15"] threads: [None, posix] libcxx: [libCstd, libstdcxx, libstlport, libstdc++] gcc: &gcc - version: ["4.1", "4.4", "4.5", "4.6", "4.7", "4.8", "4.9", - "5", "5.1", "5.2", "5.3", "5.4", "5.5", - "6", "6.1", "6.2", "6.3", "6.4", "6.5", - "7", "7.1", "7.2", "7.3", "7.4", "7.5", - "8", "8.1", "8.2", "8.3", "8.4", - "9", "9.1", "9.2", "9.3", - "10", "10.1"] + version: ['3.2', "4.1", "4.4", "4.5", "4.6", "4.7", "4.8", "4.9", "5", "5.1", + "5.2", "5.3", "5.4", "5.5", "6", "6.1", "6.2", "6.3", "6.4", "6.5", "7", + "7.1", "7.2", "7.3", "7.4", "7.5", "8", "8.1", "8.2", "8.3", "8.4", "9", + "9.1", "9.2", "9.3", "10", "10.1"] libcxx: [libstdc++, libstdc++11] threads: [None, posix, win32] # Windows MinGW exception: [None, dwarf2, sjlj, seh] # Windows MinGW @@ -59,21 +68,20 @@ Visual Studio: &visual_studio runtime: [MD, MT, MTd, MDd] version: ["8", "9", "10", "11", "12", "14", "15", "16"] - toolset: [None, v90, v100, v110, v110_xp, v120, v120_xp, - v140, v140_xp, v140_clang_c2, LLVM-vs2012, LLVM-vs2012_xp, - LLVM-vs2013, LLVM-vs2013_xp, LLVM-vs2014, LLVM-vs2014_xp, - LLVM-vs2017, LLVM-vs2017_xp, v141, v141_xp, v141_clang_c2, v142, - llvm, ClangCL] + toolset: [None, v90, v100, v110, v110_xp, v120, v120_xp, v140, v140_xp, v140_clang_c2, + LLVM-vs2012, LLVM-vs2012_xp, LLVM-vs2013, LLVM-vs2013_xp, LLVM-vs2014, + LLVM-vs2014_xp, LLVM-vs2017, LLVM-vs2017_xp, v141, v141_xp, v141_clang_c2, + v142, llvm, ClangCL] cppstd: [None, 14, 17, 20] clang: - version: ["3.3", "3.4", "3.5", "3.6", "3.7", "3.8", "3.9", "4.0", - "5.0", "6.0", "7.0", "7.1", - "8", "9", "10", "11"] + version: ["3.3", "3.4", "3.5", "3.6", "3.7", "3.8", "3.9", "4.0", "5.0", "6.0", + "7.0", "7.1", "8", "9", "10", "11"] libcxx: [None, libstdc++, libstdc++11, libc++, c++_shared, c++_static] cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20] runtime: [None, MD, MT, MTd, MDd] apple-clang: &apple_clang - version: ["5.0", "5.1", "6.0", "6.1", "7.0", "7.3", "8.0", "8.1", "9.0", "9.1", "10.0", "11.0", "12.0"] + version: ["5.0", "5.1", "6.0", "6.1", "7.0", "7.3", "8.0", "8.1", "9.0", "9.1", + "10.0", "11.0", "12.0"] libcxx: [libstdc++, libc++] cppstd: [None, 98, gnu98, 11, gnu11, 14, gnu14, 17, gnu17, 20, gnu20] intel: ```
memsharded commented 3 years ago

This is fantastic, and very promising. Thanks for the investigation, I had a look to that fork of pyyaml and looked great, but never had the time to test it. From now on (Conan 1.33) we are trying not to add any new feature to Conan 1.X that are not exclusively oriented towards Conan 2.0, so I would like to suggest the following plan:

memsharded commented 1 year ago

I am proposing in https://github.com/conan-io/conan/pull/12980 to do an update, but not script based. Running scripts at config install seems a bit unsafe. I suggest trying with a settings_user.yml file that updates on the fly the main settings.yml (without doing the round trip, which is one of the major complexities), and lets see how far it goes, it could be enough.

memsharded commented 1 year ago

Implemented https://github.com/conan-io/conan/pull/12980 for beta.9, closing this as this would solve the main issue with merging user settings.yml