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

docs: v2 migration guidence for conf_info #13490

Merged
merged 3 commits into from
Oct 19, 2022

Conversation

prince-chrismc
Copy link
Contributor

Docs!

This came up in #12896
and in #13429


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the conan-center hook activated.

The [2.0 migrations docs](https://docs.conan.io/en/latest/migrating_to_2.0/recipes.html#removed-self-user-info)
should over the technical details however for ConanCenterIndex we need to make sure there are no collisions
`conf_info` must be named `user.<recipe_name>:<variable>`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an example would be nice to have here,
e.g. we have:

self.user_info.CONFIG_GUESS = os.path.join(bin_path, "config.guess")
self.user_info.CONFIG_SUB = os.path.join(bin_path, "config.sub")
+
shutil.copy(self._user_info_build["gnu-config"].CONFIG_SUB,
os.path.join(self._source_subfolder, "config.sub"))
shutil.copy(self._user_info_build["gnu-config"].CONFIG_GUESS,
os.path.join(self._source_subfolder, "config.guess"))

what's the direct replacement?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've been seeing a lot of V2 recipe updates which use gnu-config's user_info items. It would be great to provide an example of how to do it and upgrade the gnu-config recipe at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the Conan doc examples not clear?

Copy link
Contributor

@SSE4 SSE4 Oct 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately, they are not very clear...
specifically, that part:

    def generate(self):
       my_value = self.dependencies[pkg].conf_info.get("user.myconf:foo")
  1. what is pkg here? is it just "pkg" or "pkg/1.0" or something else?
  2. is it only possible to access in generate method? we at least need in build
  3. user.myconf:foo is too vague/abstract. can I just write self.conf_info.define("CONFIG_SUB", "config.sub"), or does it need to be self.conf_info.define("user.CONFIG_SUB", "config.sub"), or self.conf_info.define("user:CONFIG_SUB", "config.sub") or self.conf_info.define("user.CONFIG_SUB:CONFIG_SUB", "config.sub")? it's not fully clear which parts of the syntax user.myconf:foo are mandatory or optional... and if user is somehow special or e.g. gnu-config can be used instead of user namespace?

Copy link
Member

@uilianries uilianries Oct 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conan docs is good, but too generic. I see no problem adding a more specific example case in CCI docs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a boat load of discussion around should we even add this #12896

And I asked Space to stick it in a PR to make sure it works https://github.com/conan-io/conan-center-index/pull/13581/files#r998575640.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a smaller example + captured some of the outside conversations. let me know if thats good!

docs/v2_migration.md Outdated Show resolved Hide resolved
The [2.0 migrations docs](https://docs.conan.io/en/latest/migrating_to_2.0/recipes.html#removed-self-user-info)
should over the technical details however for ConanCenterIndex we need to make sure there are no collisions
`conf_info` must be named `user.<recipe_name>:<variable>`.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I've been seeing a lot of V2 recipe updates which use gnu-config's user_info items. It would be great to provide an example of how to do it and upgrade the gnu-config recipe at the same time.

Co-authored-by: Jordan Williams <jordan@jwillikers.com>
@prince-chrismc
Copy link
Contributor Author

there's a conversation if we should even be using them #12896

@prince-chrismc prince-chrismc requested a review from jcar87 October 18, 2022 18:47
@prince-chrismc prince-chrismc mentioned this pull request Oct 19, 2022
4 tasks
@conan-center-bot conan-center-bot merged commit 842c55b into conan-io:master Oct 19, 2022
jellespijker added a commit to Ultimaker/conan-center-index that referenced this pull request Oct 23, 2022
As descibed in conan-io#13490

Applies code-review comments
jellespijker added a commit to Ultimaker/conan-center-index that referenced this pull request Oct 23, 2022
As described in conan-io#13490

Added a fixme note to the user_info entries, I also kept
these conf_info, Such that downstream projects which relied
on the `user_info` can migrate to `conf_info`.
conan-center-bot pushed a commit that referenced this pull request Nov 8, 2022
* autoconf v2 ready

Note: test_package can't use the new Autools generator
due to conan-io/conan#11975

* Revert removal of PATH from env_info

From review comment

* use unix_path instead of private import functions

According review comment

* use unix_path instead of private import functions

According review comment

* pass self to unix_path

* applied review comments

KB-H013 states that everything needs to be installed under bin for tools.
This also means that the `PATH` for the actual bin location `bin/bin` needs
to be added to the `build_env`

* Add licenses to package

* Explicitly use an empty includedirs

Should fix KB-H071

* Fix setting unix_path for package_info

subsystem isn't known at this time

* simplified test_package

No need to actually compile something with this tool
we simply need to know if it runs.

The win_bash for the run needs to be set explicitly
due to the conf not working properly in the test_package
see conan-io/conan#11975

* Revert "simplified test_package"

This reverts commit d9ee4de.

* use modern tools in test_package

* add conan test_v1_package

* m4 is a build requirement

* Ducktype Autotools `configure` and `make`

This allows us to use the msys2 bash without using the conf_info
in the test_package. Which is currently problematic due to:
conan-io/conan#11975

Contributes to CURA-9574

* Fix paths on windows in test method

Contributes to CURA-8831

* Don't ducktype but inherit

The ducktyping seems to affect the usage
of the Autotools in the main recipe as well

Contributes to CURA-8831

* use unix_path for env_info

Contributes to CURA-8831

* remove extra lin

Contributes to CURA-8831

* don't use unix_path

The subsystem isn't known when consuming projects request the info

Contributes to CURA-8831

* fix subsystems on Windows

Using some duck typing and inheritance to work around
GH issue conan-io/conan#11980

Contributes to CURA-9595

* updated autoconf for conan v2 ready support

Contribute to CURA-8831

* autoconf bin contains shell scripts not executable

Contribute to CURA-8831

* Removed Autotools workaround from test_package

Still an issue: conan-io/conan#11975

Contribute to CURA-8831

* m4 is a run requirement not a tool_req

Contribute to CURA-8831

* use mingw32 for gnu tuple

version 2.69 doesn't known mingw64,
it should hold no difference for the
end result.

Contribute to CURA-8831

* Run the configure command in the build_folder

Contributes to CURA-8831

* Use conanbuild env as scope for testing

Contribute to CURA-8831

* Ensure that M4 binary from dependency is used

Contributes to CURA-8831

* Set win_bash in configure stage

Contributes to CURA-8831

* Move conf keys to user space

Applies code-review comments

* Use *_folder in favor of undocumented *_path

Applies code-review comments

* Reuse source files from test_package

Applies code-review comments

* Apply suggestions from code review

Co-authored-by: Jordan Williams <jordan@jwillikers.com>

* Use export_conandata_patches

Bump up the minimum conan version to 1.52

Applies code-review comments

* Removed unused conf_info values

As descibed in #13490

Applies code-review comments

* Check against empty string for conf value bash path

Apply code review comment

* Update recipes/autoconf/all/conanfile.py

Co-authored-by: Chris Mc <prince.chrismc@gmail.com>

* Apply suggestions from code review

Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>

Co-authored-by: j.spijker@ultimaker.com <jelle spijker>
Co-authored-by: Jordan Williams <jordan@jwillikers.com>
Co-authored-by: Chris Mc <prince.chrismc@gmail.com>
Co-authored-by: SpaceIm <30052553+SpaceIm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants