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

Add an API to set/unset a deployment tree's mutability #58

Closed
wants to merge 1 commit into from

Conversation

cgwalters
Copy link
Member

This will be used by rpm-ostree to unset the immutable bit temporarily
in order to do package layering. We could add an API to deploy a tree
without the immutable bit, but this is simpler.

This will be used by rpm-ostree to unset the immutable bit temporarily
in order to do package layering.  We could add an API to deploy a tree
without the immutable bit, but this is simpler.
@mbarnes
Copy link
Contributor

mbarnes commented Feb 20, 2015

_ostree_linuxfs_alter_immutable_flag() doesn't really block and could drop its GCancellable parameter. Then you wouldn't need it for ostree_sysroot_deployment_set_mutable() either.
(I just find unnecessary GCancellable params to be misleading.)

Otherwise looks okay to me.

@cgwalters
Copy link
Member Author

I guess I've found it easier so far to just carry them along everywhere, but I've no problem with dropping them either.

@mbarnes
Copy link
Contributor

mbarnes commented Feb 22, 2015

Just to note where I'm coming from on this -- on my old desktop project, blocking the user interface was a major problem. In reworking the APIs I used GCancellable as one way to clearly indicate which functions can block and may require an asynchronous version. I understand it's not as applicable here, but the tendency carries over.

@cgwalters
Copy link
Member Author

To me this gets to a big conceptual question - e.g. opening a local file a blocking operation? What about directory enumeration? Gio at least includes GCancellable pretty much everywhere, and that's why I was too.

Perhaps a middle ground is that "single operations" like open() or unlink aren't cancellable, but enumerating a directory (or operating recursively on dirs, like glnx_shutil_rm_rf_at are cancellable even though they operate locally.

@mbarnes
Copy link
Contributor

mbarnes commented Feb 24, 2015

Right, multi-step operations have natural check points for cancellation, especially when the steps are repetitive. I guess my objection is more with taking a GCancellable argument but then not using it.

For something like _ostree_linuxfs_fd_alter_immutable_flag() it only really makes sense to check up front if the GCancellable is already cancelled before proceeding. But then I tend to think that should be left up to the caller.

@cgwalters
Copy link
Member Author

I'm still waffling on this. If we added the stanza for checking the cancellable, i.e. match what e.g. g_file_delete does, would that address your objection?

The dual factors of

  • Matching what Gio is doing
  • Also given the fact that OSTree has so many APIs that all take cancellables, we'd create divergence by not doing so

have me leaning towards keeping them.

@mbarnes
Copy link
Contributor

mbarnes commented Feb 25, 2015

Didn't mean to make this into a big issue so I'll relax a bit.

I think as long as a function is just doing something with its GCancellable -- either checking it directly or passing it on -- I'm cool with keeping it. So yeah, the added stanza would address my objection.

@cgwalters
Copy link
Member Author

I updated this to:

  • Check the cancellable
  • Use *at()

@cgwalters cgwalters closed this Apr 11, 2015
@cgwalters cgwalters deleted the deploydir-mutable-api branch April 11, 2015 14:07
krnowak pushed a commit to kinvolk-archives/ostree that referenced this pull request Feb 24, 2017
Backport fixes for list_objects bindings
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.

2 participants