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

Hook: export metadata #21

Merged
merged 9 commits into from
Feb 27, 2019
Merged

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Jan 31, 2019

Built on top of #20

Add a hook to create a metadata.json file at export time that will be uploaded with the recipe, afterwards it could be queried using the conan get command.

@jgsogo jgsogo force-pushed the hooks/export_metadata branch 2 times, most recently from 73b3c14 to 39fbde1 Compare January 31, 2019 15:37
Copy link
Contributor Author

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

Changes needed!

hooks/export_metadata.py Show resolved Hide resolved
@danimtb
Copy link
Member

danimtb commented Feb 1, 2019

This will need a merge-rebase from master @jgsogo

repo = repo_class(path)
try:
kwargs = {}
if not semver.satisfies(__version__, "<=1.12.x", loose=True):
Copy link
Member

Choose a reason for hiding this comment

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

Why not direct comparison using Version class? Is loose necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try it a little bit, it was not easy to get a working expression, but I would be very pleased if you suggest something simpler.

Copy link
Member

@danimtb danimtb Feb 1, 2019

Choose a reason for hiding this comment

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

Not sure, but something like this?

Suggested change
if not semver.satisfies(__version__, "<=1.12.x", loose=True):
if Version(__version__) >= "1.12.0":

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Version("1.12.0-dev") >= "1.12.0"

Copy link
Member

Choose a reason for hiding this comment

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

What about... ?

>>> Version("1.12.0-dev") >= "1.12"
True

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could work, but conans.model.version.Version doesn't belong to the public Conan API 😝

Copy link
Member

Choose a reason for hiding this comment

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

🙄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😆 😆 😆 😆

Copy link
Member

Choose a reason for hiding this comment

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

Still don't know why you are not using the Version class as it is something we are somehow recommending fo recipes...

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 is an open issue about conans.model.Version that is proposing to define an API for that class. The operator >= could dissapear in favor of a satisfies function like in SemVer, and then this hook (production code) will be broken requiring an update from the user... I prefer to rely on public APIs, once it is defined we can change it.

hooks/export_metadata.py Show resolved Hide resolved
hooks/export_metadata.py Show resolved Hide resolved
hooks/export_metadata.py Show resolved Hide resolved
@sztomi
Copy link

sztomi commented Feb 5, 2019

I have a use-case for this: I'd like to capture the value of an environment variable at build time (kind of a version number generated on the CI). However, as I expressed elsewhere, the centrally installed nature of hooks makes me uneasy towards using them. I'd much rather have member functions in ConanFile and override them as-needed. If I need something globally, I would override it in my specific ConanFile-derived base class, which conan now has support for using via the python_requires feature.

@jgsogo
Copy link
Contributor Author

jgsogo commented Feb 6, 2019

Hi @sztomi, first of all, I would be very pleased to know your concerns about the centrally installed nature of hooks, feedback about them in this early stage could be really valuable. Feel free to open a new issue or post it elsewhere, but please share the link, I would want to read it.

About your use case, you can have a look at this issue (conan-io/conan#2456) and the ones that are linked to it, I know it is not structured because there are bits of information split into several places, but I think that one solution for those issues (and your use case) is to have a function that will be executed at export time and its result serialized and hardcoded into the recipe file (implementation is not relevant at this stage).

That issue is tagged with the whiteboard label because I want to discuss it with the rest of the team, so there will be news related to that soon.

@sztomi
Copy link

sztomi commented Feb 6, 2019

@jgsogo opened a GHI here, see the link above.

@jgsogo jgsogo requested a review from danimtb February 27, 2019 10:46
@jgsogo jgsogo added the enhancement New feature or request label Feb 27, 2019
@danimtb danimtb merged commit 893b952 into conan-io:master Feb 27, 2019
@jgsogo jgsogo deleted the hooks/export_metadata branch February 27, 2019 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants