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

lib/core: Support <remote>: syntax when listing refs #1500

Closed
wants to merge 1 commit into from

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Mar 16, 2018

Allow users to pass <remote>: to list all refs we have locally
belonging to <remote>.

@cgwalters
Copy link
Member

LGTM, @alexlarsson want to do the r+?

@alexlarsson
Copy link
Member

alexlarsson commented Mar 16, 2018

I agree that it is nicer, with remote: than remote:., but old versions of flatpak will still not work even if this is merged. I can update flatpak master and bump the its req to the latest ostree release, but anyone that updates ostree but not flatpak will still run into this issue.

@jlebon
Copy link
Member Author

jlebon commented Mar 16, 2018

But ostree is broken right now anyway, right? Given that flatpak releases faster than ostree, would it be safe to assume that by the time ostree has this fixed, there's also going to be an updated flatpak as well?

I'm fine adding a special-case for :. for backwards compatibility, just want to make sure there's value in carrying it in the first place.

@alexlarsson
Copy link
Member

I can update flatpak and if you're using the latest flatpak you'll likely pick up the latest ostree too.
However, I guess the issue is if you're using an older flatpak, but for some other reason you get the new ostree.

For instance, Fedora 27 has the stable flatpak (0.10.x), and I can see project atomic needing a new ostree version in F27, which could cause issues here.

@jlebon
Copy link
Member Author

jlebon commented Mar 16, 2018

OK, added a fixup! ⬆️
It's part of the listing refs path only since for all other ostree_parse_refspec callers, we do want it to error out on ..

@jlebon
Copy link
Member Author

jlebon commented Mar 16, 2018

Hmm, actually, I think we should make this completely a list_refs thing. I don't think most users of the parse_refspec API (which is also public) would be ready to handle an empty ref portion.

Will get back to this in a bit.

@jlebon jlebon added the WIP label Mar 16, 2018
Allow users to pass `<remote>:` to list all refs we have locally
belonging to `<remote>`. Also (re-)allow the similar `<remote>:.` syntax
for backwards compatibility with flatpak.
@jlebon jlebon force-pushed the pr/fix-refs-match branch from 676ba21 to 2964194 Compare March 16, 2018 18:21
@jlebon jlebon removed the WIP label Mar 16, 2018
@cgwalters
Copy link
Member

@rh-atomic-bot r+ 2964194

@rh-atomic-bot
Copy link

⚡ Test exempted: merge already tested.

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