After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 741577 - Add more flexible _remote_change() API , expose via 'ostree remote'
Add more flexible _remote_change() API , expose via 'ostree remote'
Status: RESOLVED FIXED
Product: ostree
Classification: Infrastructure
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: OSTree maintainer(s)
OSTree maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2014-12-15 21:46 UTC by Colin Walters
Modified: 2014-12-16 02:30 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add more flexible _remote_change() API , expose via 'ostree remote' (13.24 KB, patch)
2014-12-15 21:46 UTC, Colin Walters
reviewed Details | Review
Add more flexible _remote_change() API , expose via 'ostree remote' (13.65 KB, patch)
2014-12-15 23:46 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2014-12-15 21:46:50 UTC
For Anaconda, I needed OSTREE_REPO_REMOTE_CHANGE_ADD_IF_NOT_EXISTS,
with the GFile *sysroot argument to avoid ugly hacks.  We want to
write the content provided via "ostreesetup" as a remote to the target
chroot only in the case where it isn't provided as part of the tree
content itself.

This is also potentially useful in idempotent systems management tools
like Ansible.
Comment 1 Colin Walters 2014-12-15 21:46:53 UTC
Created attachment 292773 [details] [review]
Add more flexible _remote_change() API , expose via 'ostree remote'
Comment 2 Colin Walters 2014-12-15 23:46:16 UTC
Created attachment 292782 [details] [review]
Add more flexible _remote_change() API , expose via 'ostree remote'

Fix a bug in IF_NOT_EXISTS, adjust test cases to cover this.
Comment 3 Matthew Barnes 2014-12-16 01:51:47 UTC
Review of attachment 292773 [details] [review]:

The blurb for "ostree remote add --if-not-exists" is backwards.  Should be "Do nothing if the provided remote already exists".

I was trying to think of an easier-to-remember option name that could be used for both commands, as --if-exists and --if-not-exists seem a tad verbose to me.  The best I could come up with was --maybe, but I'm not sure if it's an improvement.

Should ostree_repo_remote_add() and ostree_repo_remote_delete() be tagged as deprecated (however you do that for libostree)?  Seems they're redundant now.

LGTM otherwise.
Comment 4 Colin Walters 2014-12-16 02:29:31 UTC
(In reply to comment #3)
> Review of attachment 292773 [details] [review]:
> 
> The blurb for "ostree remote add --if-not-exists" is backwards.  Should be "Do
> nothing if the provided remote already exists".

Fixed, thanks!

> I was trying to think of an easier-to-remember option name that could be used
> for both commands, as --if-exists and --if-not-exists seem a tad verbose to me.

One option is "--idempotent", but the question that raises is "what happens if i try to add a remote but change the url"?

"if exists" makes this clear, that we only look at existence.

> Should ostree_repo_remote_add() and ostree_repo_remote_delete() be tagged as
> deprecated (however you do that for libostree)?  Seems they're redundant now.

Those APIs are definitely redundant now, but:

1) We don't have the advanced versioning macros and deprecation system that GLib does yet (though that would be nice)
2) I would tend to deprecate when there's a *reason* to change, the old one performs badly, etc.  

> LGTM otherwise.

Thanks!
Comment 5 Colin Walters 2014-12-16 02:30:01 UTC
Attachment 292782 [details] pushed as f6a6e68 - Add more flexible _remote_change() API , expose via 'ostree remote'