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 user/channel when searching patterns in a local-recipes-index #17408

Merged

Conversation

AbrilRBS
Copy link
Member

@AbrilRBS AbrilRBS commented Dec 4, 2024

Changelog: Fix: Fix user/channel when searching patterns in a local-recipes-index.
Docs: Omit

Some minor edits to the code still need to be made, but I think this is a better approach than the current one

Closes #17403

# just add all candidates for now
pattern = f"{pattern_ref.name}/{pattern_ref.version}"
except:
# pattern = pattern
Copy link
Member Author

Choose a reason for hiding this comment

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

I dont think this can actually happen, pattern is already str of a RecipeReference afaik from https://github.com/conan-io/conan/blob/2.10.0/conans/client/graph/range_resolver.py#L55

Copy link
Member

Choose a reason for hiding this comment

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

It is fine to exclude the user/channel for this initial check, but then it has to be re-checked again after the actual conanfile has been loaded and to match the real user/channel defined by recipe?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can tell, this is being done in _search_remote_recipes https://github.com/conan-io/conan/blob/develop2/conans/client/graph/range_resolver.py#L80 as it expects that the search has ignored the user/channel

Copy link
Member

Choose a reason for hiding this comment

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

There it contains a:

# TODO: This is still necessary to filter user/channel, until search_recipes is fixed

So basically trying this to return the right thing and not rely on RangeResolved to fix it later?

@AbrilRBS AbrilRBS changed the title Ignore user/channel when searching patterns in a local-recipes-index Fix user/channel when searching patterns in a local-recipes-index Dec 4, 2024
# just add all candidates for now
pattern = f"{pattern_ref.name}/{pattern_ref.version}"
except:
# pattern = pattern
Copy link
Member

Choose a reason for hiding this comment

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

It is fine to exclude the user/channel for this initial check, but then it has to be re-checked again after the actual conanfile has been loaded and to match the real user/channel defined by recipe?

tc = TestClient(light=True)
tc.run(f"remote add local '{c3i_user_channel_folder}'")
tc.run(f"graph info --requires=pkg/[*]@")
assert f"pkg/[*]: pkg/2.0" in tc.out
Copy link
Member Author

Choose a reason for hiding this comment

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

Not quite sure that this is expected

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 sure what you mean. The patterns ending in @ means "no user/channel", that was necessary to represent this use case, otherwise it is not possible via normal patterns.

@AbrilRBS AbrilRBS marked this pull request as ready for review December 5, 2024 13:23
@memsharded memsharded added this to the 2.11.0 milestone Dec 5, 2024
@memsharded memsharded merged commit 50a16ba into conan-io:develop2 Dec 5, 2024
33 checks passed
@AbrilRBS AbrilRBS deleted the ar/fix-local-recipe-index-user-channel branch December 5, 2024 17:09
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.

[question] Install latest version (or version range) from local recipes index
2 participants