Skip to content

Conversation

@karajan1001
Copy link
Contributor

@karajan1001 karajan1001 commented Dec 6, 2021

fix: #4 and iterative/dvc#6051
In dvc we get a random toggling branch name in exp show. It is because
when we iter branches the dulwich.refs.keys gives an unordered Set
of refs. Here we reordered it, and makes current active branch always be
in the first order.

  1. Add order to the result of dulwich.refs.keys.
  2. Make active branch always in the first place.
  3. Add a new test to it.


best_ref = None

for ref in sorted(self.iter_refs(base=base)):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here the best solution is to order it in dulwich by the time the branch was modified. But I don't know if they wish to modify the return with List instead of Set.

There was an old discussion related to it, but here the suggestion on reflog only gives the time of the last commit, in our case because both of the branch shares the same commit, they would have the same time in reflogs. what we need here is the branch modification time of files in .git/refs/heads
jelmer/dulwich#392

@karajan1001 karajan1001 requested a review from pmrowla December 6, 2021 09:36
Copy link
Contributor

@pmrowla pmrowla left a comment

Choose a reason for hiding this comment

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

Also, this new check for whether or not HEAD is a branch and matches rev when base=refs/heads should maybe be added at the top level Git.describe() instead of at the individual backend level (since the order won't be guaranteed by any backend, but we want to at least be consistent for in this particular case regardless of what backend we are using)

So at the top scmrepo.git.Git.describe() level we would do this specific check (using the generic self.get_ref() calls) and then fall back to returning self._describe() (where _describe = _backend_func("describe"))

@karajan1001 karajan1001 force-pushed the fix_order_of_describe_in_dulwich_backend branch from b63a81f to b9fb39a Compare December 6, 2021 12:42
@karajan1001 karajan1001 force-pushed the fix_order_of_describe_in_dulwich_backend branch from b9fb39a to e5484ce Compare December 6, 2021 13:03
@karajan1001 karajan1001 force-pushed the fix_order_of_describe_in_dulwich_backend branch from e5484ce to c03b4b5 Compare December 6, 2021 14:54
@karajan1001 karajan1001 force-pushed the fix_order_of_describe_in_dulwich_backend branch from c03b4b5 to c19555e Compare December 7, 2021 08:30
…terative#4)

fix: iterative#4 and iterative/dvc#6051

In dvc we get a random toggling branch name in exp show. It is because
when we iter branches the dulwich.refs.keys gives an unordered Set
of refs. Here we choose the active branch if we are `describe` its
commit rev.

1. Make active branch always in the first place.
2. Add a new test to it.

Co-authored-by: Peter Rowlands (변기호) <peter@pmrowla.com>
@karajan1001 karajan1001 force-pushed the fix_order_of_describe_in_dulwich_backend branch from c322eec to 976a0f2 Compare December 7, 2021 12:14
@pmrowla pmrowla merged commit 5edfbcc into iterative:main Dec 7, 2021
@karajan1001 karajan1001 deleted the fix_order_of_describe_in_dulwich_backend branch December 7, 2021 12:32
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.

Fix output of describe in dulwich backends.

2 participants