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

No Storing Channel* and MRepo* in Solvables #2409

Merged
merged 9 commits into from
Apr 11, 2023

Conversation

AntoinePrv
Copy link
Member

@AntoinePrv AntoinePrv commented Mar 24, 2023

This removes

  • the capture to mamba::Channel* in mamba::MRepo
  • the capture of mamba::MRepo* in ::Repo::appdata
  • break dependency between mamba::MRepo and mamba::Channel

@AntoinePrv AntoinePrv marked this pull request as ready for review April 4, 2023 12:59
@AntoinePrv AntoinePrv self-assigned this Apr 4, 2023
@AntoinePrv AntoinePrv requested review from JohanMabille and wolfv and removed request for JohanMabille April 4, 2023 13:50
m_real_repo_key = pool_str2id(pool, "solvable:real_repo_url", 1);
m_mrepo_key = pool_str2id(pool, "solvable:mrepo_url", 1);
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference between repo and real_repo ?

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'd like to know too! This is a mechanical operation that stores the value of MRepo::url for retrival.
I tried to merge real_repo_url and mrepo_url but it was significantly more tricky than the scope of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is also m_metadata.url/"mamba:url" in the mix

@@ -165,6 +165,8 @@ namespace mamba

History::UserRequest m_history_entry;
Transaction* m_transaction;
::Id m_real_repo_key = 0; // Must match that in ``MRepo``
::Id m_mrepo_key = 0; // Must match that in ``MRepo``
Copy link
Member

Choose a reason for hiding this comment

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

I guess this leaking of libsolv details in the transaction class is temporary and will be replaced with PackageInfo list on the long run? In the meantime, they (and those in MRepo) should call the same initialization function to ensure their "synchronization"

Copy link
Member Author

Choose a reason for hiding this comment

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

This constraint was already here, the PR just made it more obvious.
At the libsolv level (which will be private), #2401 will introduce clean getter/setter in the bindings so that we don't need to match the keys manually.
At the user level, this will be invisible. The user will only deal in PackageInfo style of data.

@JohanMabille JohanMabille merged commit 5fa2ce3 into mamba-org:main Apr 11, 2023
@AntoinePrv AntoinePrv deleted the solv-repo-url branch April 12, 2023 08:17
@jonashaag jonashaag mentioned this pull request Jun 14, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants