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.

Sign up for GitHub

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

Jump to bottom

Add Tree#find_blob. Account for symlink blobs in Tree class. #59

Merged
merged 3 commits into from
Mar 28, 2023
Merged

Add Tree#find_blob. Account for symlink blobs in Tree class. #59

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1e3063f
Add Tree#find_blob. Account for symlink blobs in Tree class.
Jan 15, 2023
d7881be
Decomplicate file mode logic by always expecting an integer
Jan 15, 2023
9d60ba2
Make Tree#find_blob more flexible
Jan 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions lib/rugged_adapter/git_layer_rugged.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,11 +654,7 @@ def log(commit = Gollum::Git.default_ref_for_repo(@repo), path = nil, options =
def lstree(sha, options = {})
results = []
@repo.lookup(sha).tree.walk(:postorder) do |root, entry|
entry[:sha] = entry[:oid]
entry[:mode] = entry[:filemode].to_s(8)
entry[:type] = entry[:type].to_s
entry[:path] = "#{root}#{entry[:name]}"
results << entry
results << ::Gollum::Git::Tree.tree_entry_from_rugged_hash(entry, root)
end
results
end
Expand Down Expand Up @@ -689,6 +685,16 @@ def find_branch(search_list)

class Tree

def self.tree_entry_from_rugged_hash(entry, root = '')
dometto marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

I'm not loving this name of this class method. One reason is that the name suggests that the method returns a tree entry based on hash input ("from hash"), but the name of the first argument and the body of the method imply that an entry (whatever that is) is transformed into a hash (thus the other way around).

A second reason is that I have no sense of what a rugged_hash is. It seems as if is is an ordinary hash originating from a method in rugged, but then I don't see why we need to know the origin of the hash.

Would it make sense to rename this to something like tree_entry_to_hash(entry, root='')?

Copy link
Member

Choose a reason for hiding this comment

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

(Sorry, I know this is merged already.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good points, thanks! The wording is tricky since a tree entry is essentially just a hash representation of an object in a git tree, as used by rugged, and we want to convert this into our own hash representation of an object in a git tree. tree_entry_to_hash misleadingly sounds as if what we're passing in is a hash instead of a tree entry. What about something like canonicalize_tree_entry?

{
sha: entry[:oid],
mode: entry[:filemode],
type: entry[:type].to_s,
name: entry[:name],
path: "#{root}#{entry[:name]}"
}
end

def initialize(tree)
@tree = tree
end
Expand Down Expand Up @@ -719,10 +725,22 @@ def /(file)

def blobs
blobs = []
@tree.each_blob {|blob| blobs << Gollum::Git::Blob.new(@tree.owner.lookup(blob[:oid]), blob) }
@tree.each_blob {|blob| blobs << blob_for_tree_entry(blob) }
blobs
end

def find_blob(&block)
return nil unless block_given?
blob = @tree.each_blob.find {|blob| yield blob[:name] }
blob ? blob_for_tree_entry(blob) : nil
end

private

def blob_for_tree_entry(blob)
Gollum::Git::Blob.new(@tree.owner.lookup(blob[:oid]), self.class.tree_entry_from_rugged_hash(blob))
end

end

end
Expand Down