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

OstreeMutableTree: add _remove method #1724

Closed
wants to merge 1 commit into from

Conversation

ramcq
Copy link
Contributor

@ramcq ramcq commented Sep 18, 2018

There is no API method to remove a file from a MutableTree besides directly manipulating the GHashTable returned by _get_files. This isn't possible from an introspection binding that transforms the returned GHashTable, and may also leave the tree checksum in an invalid state. Allow NULL to remove a file entry, add GI annotations to show that checksum is nullable. Introduce a new method to allow removing files or subdirectories.

@jlebon
Copy link
Member

jlebon commented Sep 18, 2018

Hmm, there's no way either to remove directories, right? Maybe a dedicated ostree_mutable_tree_remove would make more sense to address both of them instead?

I guess we could enhance the semantics of _replace() so that a NULL checksum can also nuke subdirs, though it wouldn't be at all clear from its name.

@ramcq
Copy link
Contributor Author

ramcq commented Sep 19, 2018

Makes sense. I was thinking about how to work around the lack of removal by recursively walking my MutableTree and making a new one without the stuff I wanted to remove, but it seems there's also no API for adding a subdir which is an already existing MutableTree. This whole thing is driving me nuts. Anyway - here is a _remove method, which solves the problem at hand!

@ramcq ramcq changed the title OstreeMutableTree: allow NULL checksum to _replace_file OstreeMutableTree: add _remove method Sep 19, 2018
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one suggestion below, otherwise LGTM!

but it seems there's also no API for adding a subdir which is an already existing MutableTree

Hmm, yeah you're right. If it's a tree that was initialized from an ostree commit, you can at least use ostree_repo_write_directory_to_mtree() with the root tree (or a subdir).

My impression is that it's more common to do any extensive manipulations by just checking out the files to disk, edit things there, and then recommit it back. With the devino cache, performance is pretty good. (#1651 is related here; it adds a new --prefix commit option).

src/libostree/ostree-mutable-tree.c Show resolved Hide resolved
There is no API method to remove a file or subdirectory from a MutableTree
besides directly manipulating the GHashTable returned by _get_files or
_get_subdirs. This isn't possible from an introspection binding that transforms
the returned GHashTable, and may also leave the tree checksum in an invalid
state. Introduce a new method so that removing files or subdirectories is
safe, and possible from bindings.
@jlebon
Copy link
Member

jlebon commented Sep 20, 2018

Awesome, thanks a lot!
@rh-atomic-bot r+ d089305

(That whole file needs to be ported to the new style, but let's do that separately.)

@rh-atomic-bot
Copy link

⌛ Testing commit d089305 with merge ca32ff7...

rh-atomic-bot pushed a commit that referenced this pull request Sep 20, 2018
There is no API method to remove a file or subdirectory from a MutableTree
besides directly manipulating the GHashTable returned by _get_files or
_get_subdirs. This isn't possible from an introspection binding that transforms
the returned GHashTable, and may also leave the tree checksum in an invalid
state. Introduce a new method so that removing files or subdirectories is
safe, and possible from bindings.

Closes: #1724
Approved by: jlebon
@pwithnall
Copy link
Member

@rh-atomic-bot retry

@rh-atomic-bot
Copy link

⌛ Testing commit d089305 with merge 3477c39...

rh-atomic-bot pushed a commit that referenced this pull request Sep 20, 2018
There is no API method to remove a file or subdirectory from a MutableTree
besides directly manipulating the GHashTable returned by _get_files or
_get_subdirs. This isn't possible from an introspection binding that transforms
the returned GHashTable, and may also leave the tree checksum in an invalid
state. Introduce a new method so that removing files or subdirectories is
safe, and possible from bindings.

Closes: #1724
Approved by: jlebon
@rh-atomic-bot
Copy link

💔 Test failed - status-atomicjenkins

@jlebon
Copy link
Member

jlebon commented Sep 20, 2018

@rh-atomic-bot retry

@rh-atomic-bot
Copy link

⌛ Testing commit d089305 with merge b32c9e0...

@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: jlebon
Pushing b32c9e0 to master...

mwleeds pushed a commit to mwleeds/ostree that referenced this pull request Sep 24, 2018
There is no API method to remove a file or subdirectory from a MutableTree
besides directly manipulating the GHashTable returned by _get_files or
_get_subdirs. This isn't possible from an introspection binding that transforms
the returned GHashTable, and may also leave the tree checksum in an invalid
state. Introduce a new method so that removing files or subdirectories is
safe, and possible from bindings.

Closes: ostreedev#1724
Approved by: jlebon

(cherry picked from commit ca32ff7)
Signed-off-by: Robert McQueen <rob@endlessm.com>
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.

None yet

4 participants