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

Fix empty buildinfo when using conan export-package #155

Closed
wants to merge 2 commits into from

Conversation

getrostt
Copy link

When building a conan package using conan export-pkg, the resulting build-info when running conan art:build-info create is empty. This is caused by an unconditional removal of the 1st graph node in the resulting json file.

This fix only removes the 1st node when it is a ref of type conanfile.

@CLAassistant
Copy link

CLAassistant commented Sep 11, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Thanks very much for your contribution @getrostt

Adding @czoido so he might be able to help checking the potential issue (maybe the way to avoid it is checking the existing of the path field?

@@ -389,7 +389,8 @@ def build_info_create(conan_api: ConanAPI, parser, subparser, *args):
data = load_json(args.json)

# remove the 'conanfile' node
data["graph"]["nodes"].pop("0")
if data["graph"]["nodes"]["0"]["ref"] == "conanfile":
Copy link
Member

Choose a reason for hiding this comment

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

I am concern about cases where the consumer might have defined a name and version, so the ref might not be "conanfile", but the actual name/version.

Maybe it would be necessary to add a test for that scenario.

Copy link
Author

Choose a reason for hiding this comment

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

I tried to replay the scenario on the commandline using the following commands:

conan new cmake_lib -d name=mypkg -d version=1.0 --force
conan create . --format json -tf='' -s build_type=Release > create_release.json

That results in the following create_release.json (I only pasted the 1st lines):

{
    "graph": {
        "nodes": {
            "0": {
                "ref": "conanfile",
                "id": "0",

Is this the behavior you were thinking of for a test?

Copy link
Contributor

Choose a reason for hiding this comment

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

May we remove the node 0 in the case that "prev": null to differentiate both cases @memsharded ?

@czoido
Copy link
Contributor

czoido commented Sep 12, 2024

Hi @getrostt,
Thanks a lot for the contribution. I proposed some changes there to consider all cases. I'll have to open a separate PR for this because we can only run tests marked as requires_credentials if they run from a branch from the repo not from forks. I'll cherry-pick your changes so you still appear as contributor.

@czoido
Copy link
Contributor

czoido commented Sep 12, 2024

Closing this one in favor of #157

@czoido czoido closed this Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants