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: Add conan config init command #6959

Merged
merged 5 commits into from May 13, 2020

Conversation

e-i-n-s
Copy link
Contributor

@e-i-n-s e-i-n-s commented May 5, 2020

Changelog: Feature: Add conan config init command.
Docs: conan-io/docs#1704

  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Background:
We need to customize the settings.yml file, because we need to add the distro setting as described in https://docs.conan.io/en/latest/extending/custom_settings.html#adding-new-settings.
We don't like to maintain our own settings.yml, because this means for every new release of clang or gcc we would need to update the settings.yml.

To automatically create the settings.yml (and the other configuration files) I propose to introduce the conan config init command.

Our workflow:

  1. Run conan config init
  2. A script will change the settings.yml
  3. Run conan install [...] or other conan commands.

@memsharded memsharded self-assigned this May 5, 2020
@memsharded
Copy link
Member

@memsharded memsharded commented May 5, 2020

Hi @e-i-n-s

I am not sure if I understood correctly the pain that you are trying to solve. And why not conan config install installing a new settings.yml file would be better.

What you are explaining needs to be executed by every Conan client installation, in CI, by developers in their machines. While the conan config install will require to be running that script only once, by the maintainer, and all developers and CI servers will have it way more easily. Furthermore, Conan 1.25 is releasing a new "auto conan config install", so machines can periodically auto update based on their "conan config install".

Can you please clarify your intent? Thanks!

@e-i-n-s
Copy link
Contributor Author

@e-i-n-s e-i-n-s commented May 5, 2020

@memsharded
Thank you for the answer.

Our pain is if we use conan config install, this would mean that we have to maintain our custom settings.yml file. As a result we would have to change the custom settings.yml multiple times per year manually (for instance in a conan config git repository) and in my eyes this is an overhead for a very small change in settings.yml.

There are lot of compiler related changes in the settings.yml (i.e. #6911, #6772).

Do I understand correctly that conan config install always overwrites the whole settings.yml?
So the only way to change just a part of settings.yml is to modify the file manually, right?

@memsharded
Copy link
Member

@memsharded memsharded commented May 5, 2020

Hi @e-i-n-s

I am sorry, but you are still maintaining a custom settings.yml. 😄

You are doing it in every Conan installation, in every developer machine and every CI server. Here:

  1. A script will change the settings.yml

The only difference that I am suggesting is doing that operation once, controlled by the admin/maintainer, and storing that changed settings.yml in a place, where all Conan clients can sync from it. Way more robust to operate, you don't rely on having that custom script on every developer machine. What if that script needs updating, how are you managing that?

Are you in the CppLang slack? If you are there, we might discuss live, maybe that helps to understand better.

@e-i-n-s
Copy link
Contributor Author

@e-i-n-s e-i-n-s commented May 6, 2020

@memsharded
I joined the CppLang slack! We could discuss there: https://cpplang.slack.com/archives/C41CWV9HA/p1588752313022500

@memsharded memsharded added this to the 1.26 milestone May 6, 2020
@memsharded memsharded requested a review from jgsogo May 6, 2020
@memsharded
Copy link
Member

@memsharded memsharded commented May 6, 2020

It seems there is still a good case that it is very convenient to initialize the conan cache with the default things, so this command might make sense. It might be also a convenient place to also create the default profile if not there, as the conan profile new --detect default is complicated for users.

@jgsogo
Copy link
Member

@jgsogo jgsogo commented May 6, 2020

I think there is something around the config we are not covering yet. For me it'd make sense to have a command like conan config init that:

  • (init) will populate missing files in the cache with the default content
  • (init --force) will clean the cache and write the default content

But how to deal with it when you just want to get the default content for one of those files? I usually want the default for settings.yml but I want to keep my remotes.json. Maybe conan config init can take the name/id of the file to initialize?


About the underlying pain in this PR: to add a line to the settings.yml, will require running 4 commands:

  • Remove the settings.yml file from the cache
  • Run conan config init
  • A script will change the settings.yml
  • Run conan install [...] or other conan commands.
  • (Never use conan config install because it will override the changes)

IMO it would make sense as a hook: the logic will check for os.distro inside settings.yml and modify it accordingly (more or less the same script you are already running). The question would be when to run this hook? It has to run after a command that modifies the settings.yml, so after conan config install AND after Conan populates the cache (right now it will be the first time you run a command, or with this PR after conan config init). Does it make sense? Is it overkill?

I know this comment can be a little bit offtopic, but probably I need some more context from that slack chat.

@memsharded
Copy link
Member

@memsharded memsharded commented May 6, 2020

IMO it would make sense as a hook: the logic will check for os.distro inside settings.yml and modify it accordingly (more or less the same script you are already running). The question would be when to run this hook?

Not sure I agree. Hooks so far are orthogonal checks to recipes flows, while this lies much on the Conan client configuration side.

I agree that it would be nice to have some idea how this can be extended in the future for other cases, as only initialization one file, but I would certainly want to keep this as simple as possible in this first iteration.

I don't think the way to go is to do this work on the users/developers/ci machines, but have them provided by conan config install. The thing is that conan config init might still be needed in order to easily generate the settings.yml that goes into the conan config install origin.

@jgsogo
Copy link
Member

@jgsogo jgsogo commented May 6, 2020

I totally agree some kind of conan config init is needed, IMO, conan config init to populate only missing files and conan config init --force to remove and populate all of them is perfect, great, needed (will it fit into the next CLI interface, @czoido ?).

With that command, the pain described here can be solved. I was just trying to offer an alternative, I don't see any limitation why we can't add a hook executor not related to recipes, so we let users "to customize the client behavior without modifying the client sources".

@memsharded memsharded merged commit 90e6d64 into conan-io:develop May 13, 2020
2 checks passed
@memsharded
Copy link
Member

@memsharded memsharded commented May 13, 2020

Thanks for the contribution! Merged, will be in 1.26.

I see the docs are TBD, if you can please contribute them that would be great. If not possible, please let us know. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants