lib/repo-refs: Fix resolving collection-refs #1825
Closed
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
My last commit "lib/repo-refs: Resolve collection-refs in-memory and in parent repos" changed ostree_repo_resolve_collection_ref() to check the in-memory set of refs *after* failing to find the ref on disk but that's not what we want. We want to use the in-memory set of refs first, because those are the most up to date commits, and then fall back to the on-disk repo and finally fall back to checking any parent repo. This commit makes such a change to the order of operations, which is consistent with how ostree_repo_resolve_rev() works. Aside from this change being logical, it also fixes some unit test failures on an unmerged branch of flatpak: flatpak/flatpak#2705 Also, tweak the comments here.
mwleeds
force-pushed
the
resolve-collection-refs-even-better
branch
from
0443ea7
to
070126e
Compare
|
bot, retest this please |
1 similar comment
|
bot, retest this please |
pwithnall
reviewed
Feb 21, 2019
|
I'm not able to reproduce the failure of So I guess we hit that |
|
This branch now has two more commits which need review |
mwleeds
force-pushed
the
resolve-collection-refs-even-better
branch
from
8067fae
to
130b02d
Compare
Currently the flag OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES for ostree_repo_list_collection_refs() means that refs in refs/remotes/ should be excluded but refs in refs/mirrors/ should still be checked, in addition to refs/heads/ which is always checked. However in some situations you want to exclude both remote and mirrored refs and only check local "owned" ones. So this commit adds a new flag OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_MIRRORS which lets you exclude refs/mirrors/ from the listing. This way we can avoid breaking API but still allow the listing of local collection-refs. The impetus for this change is that I'm changing Flatpak to make more use of refs/mirrors, and we need a way to specify that a collection-ref is local when using ostree_repo_resolve_collection_ref() in, for example, the implementation of the repo command. The subsequent commit will make the changes needed there.
Currently for a "normal" refspec you can choose to use ostree_repo_resolve_rev_ext() instead of ostree_repo_resolve_rev() if you only want to look at local refs (in refs/heads/) not remote ones. This commit provides the analogous functionality for ostree_repo_resolve_collection_ref() by adding a flag OSTREE_REPO_RESOLVE_REV_EXT_LOCAL_ONLY and implementing it. This will be used by Flatpak.
mwleeds
force-pushed
the
resolve-collection-refs-even-better
branch
from
130b02d
to
8cf5043
Compare
pwithnall
reviewed
Feb 26, 2019
|
Addressed the feedback, thanks |
|
This should be merged before the next release to avoid breakage since #1821 was already merged. |
|
Anyone able to review the fixups and merge this? It's blocking flatpak/flatpak#2705 |
jlebon
reviewed
Apr 15, 2019
| @@ -1295,7 +1326,7 @@ ostree_repo_list_collection_refs (OstreeRepo *self, | |||
|
|
|||
| g_string_truncate (base_path, 0); | |||
|
|
|||
| for (const char **iter = refs_dirs; iter && *iter; iter++) | |||
| for (const char **iter = (const char **)refs_dirs->pdata; iter && *iter; iter++) | |||
There was a problem hiding this comment.
Since we have a GPtrArray now, it'd be cleaner to not append a final NULL and use refs_dir->len. But eh, not worth a respin!
|
LGTM! |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Apr 15, 2019
Currently the flag OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_REMOTES for ostree_repo_list_collection_refs() means that refs in refs/remotes/ should be excluded but refs in refs/mirrors/ should still be checked, in addition to refs/heads/ which is always checked. However in some situations you want to exclude both remote and mirrored refs and only check local "owned" ones. So this commit adds a new flag OSTREE_REPO_LIST_REFS_EXT_EXCLUDE_MIRRORS which lets you exclude refs/mirrors/ from the listing. This way we can avoid breaking API but still allow the listing of local collection-refs. The impetus for this change is that I'm changing Flatpak to make more use of refs/mirrors, and we need a way to specify that a collection-ref is local when using ostree_repo_resolve_collection_ref() in, for example, the implementation of the repo command. The subsequent commit will make the changes needed there. Closes: #1825 Approved by: jlebon
rh-atomic-bot
pushed a commit
that referenced
this pull request
Apr 15, 2019
Currently for a "normal" refspec you can choose to use ostree_repo_resolve_rev_ext() instead of ostree_repo_resolve_rev() if you only want to look at local refs (in refs/heads/) not remote ones. This commit provides the analogous functionality for ostree_repo_resolve_collection_ref() by adding a flag OSTREE_REPO_RESOLVE_REV_EXT_LOCAL_ONLY and implementing it. This will be used by Flatpak. Closes: #1825 Approved by: jlebon
|
☀️ 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.
My last commit "lib/repo-refs: Resolve collection-refs in-memory and in
parent repos" changed ostree_repo_resolve_collection_ref() to check the
in-memory set of refs after failing to find the ref on disk but that's
not what we want. We want to use the in-memory set of refs first,
because those are the most up to date commits, and then fall back to the
on-disk repo and finally fall back to checking any parent repo. This
commit makes such a change to the order of operations, which is
consistent with how ostree_repo_resolve_rev() works.
Aside from this change being logical, it also fixes some unit test
failures on an unmerged branch of flatpak:
flatpak/flatpak#2705