lib/repo-pull: Fix remote names in refspecs from non-mirror P2P pulls #1202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pwithnall
force-pushed
the
p2p-refspec-naming
branch
5 times, most recently
from
8d3d8d3
to
c3c63ef
Compare
|
☔ The latest upstream changes (presumably 2f9f722) made this pull request unmergeable. Please resolve the merge conflicts. |
If override-remote-name is specified in the options to ostree_repo_pull_with_options(), but the remote_name_or_baseurl argument is also set to a remote name, the override-remote-name would be leaked. Note that this is currently an invalid configuration, so this leak is basically never hit. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Instead of returning just the keyring filename, return the entire OstreeRemote, which has the keyring filename as one of its members. This will simplify some upcoming changes, and allows slightly improved debug logging. Signed-off-by: Philip Withnall <withnall@endlessm.com>
When pulling from a dynamic (peer to peer) remote, the remote’s name is set to a unique, generated string which doesn’t exist in repo/config. If doing a non-mirror pull, however, we don’t want to use this name in the refspecs for newly created or updated refs — we want to use the name of the remote which provided the keyring for the pull (this will be a remote from repo/config whose collection ID matches that being used for the peer to peer pull). Store both names in OstreeRemote. The name to use for refspecs is stored as refspec_name, and is typically NULL unless it differs from name. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Propagate the refspec_name from the OstreeRemote returned by an OstreeRepoFinder through to the set_ref() call. This changes ostree_repo_pull_with_options() to accept the previously-disallowed combination of passing override-remote-name in options and also setting a remote name in remote_name_or_baseurl. ostree_repo_pull_with_options() will continue to pull using the remote config named in remote_name_or_baseurl as before; but will now use the remote name from override-remote-name when it’s setting the refs at the end of the pull. This is consistent with the documentation for override-remote-name. Signed-off-by: Philip Withnall <withnall@endlessm.com>
pwithnall
force-pushed
the
p2p-refspec-naming
branch
from
c3c63ef
to
d92a020
Compare
|
Rebased now that #1182 has been merged; ready for review. |
|
This makes sense to me and the code all looks good. I have some lingering uncertainty which probably just boils down to me needing read more of the collections code and understand it more. |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Sep 27, 2017
Instead of returning just the keyring filename, return the entire OstreeRemote, which has the keyring filename as one of its members. This will simplify some upcoming changes, and allows slightly improved debug logging. Signed-off-by: Philip Withnall <withnall@endlessm.com> Closes: #1202 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Sep 27, 2017
When pulling from a dynamic (peer to peer) remote, the remote’s name is set to a unique, generated string which doesn’t exist in repo/config. If doing a non-mirror pull, however, we don’t want to use this name in the refspecs for newly created or updated refs — we want to use the name of the remote which provided the keyring for the pull (this will be a remote from repo/config whose collection ID matches that being used for the peer to peer pull). Store both names in OstreeRemote. The name to use for refspecs is stored as refspec_name, and is typically NULL unless it differs from name. Signed-off-by: Philip Withnall <withnall@endlessm.com> Closes: #1202 Approved by: cgwalters
rh-atomic-bot
pushed a commit
that referenced
this pull request
Sep 27, 2017
Propagate the refspec_name from the OstreeRemote returned by an OstreeRepoFinder through to the set_ref() call. This changes ostree_repo_pull_with_options() to accept the previously-disallowed combination of passing override-remote-name in options and also setting a remote name in remote_name_or_baseurl. ostree_repo_pull_with_options() will continue to pull using the remote config named in remote_name_or_baseurl as before; but will now use the remote name from override-remote-name when it’s setting the refs at the end of the pull. This is consistent with the documentation for override-remote-name. Signed-off-by: Philip Withnall <withnall@endlessm.com> Closes: #1202 Approved by: cgwalters
|
☀️ Test successful - status-atomicjenkins |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A set of commits to fix the situation where doing a non-mirror pull using the P2P functionality (
ostree find-remotes --pull …) would result in the generated refs having an invalid remote name. They would use the name of the dynamic remote which the pull operation used, rather than a remote inrepo/configwhich matches the collection ID for those refs.See the commit messages for more details.
This set of commits depends on PR #1182, although I could possibly rebase it against master to avoid blocking on that. (Both PRs touch
OstreeRepoFinderMountnon-trivially.)