Re: [PATCH 0/1] Preserve topological ordering after merges
- From
- Sergey Rudyshin <540555@gmail.com>
- Date
- Jan 8, 2020, 12:50 UTC
- Message-ID
- <fa570dcf-b19b-f658-456e-b4174d767ba1@gmail.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.2001071308290.46@tvgsbejvaqbjf.bet>
Hi Johannes, thanks for you comments! please find my replies below
07.01.2020 15:11, Johannes Schindelin wrote:
Show 57 quoted lines
> Hi Sergey, > > On Tue, 7 Jan 2020, Sergey Rudyshin via GitGitGadget wrote: > >> This modifies the algoritm of topopological ordering. The problem with the >> current algoritm is that it displays the graph below as following >> >> * E >> |\ >> | * D >> | |\ >> | |/ >> |/| >> * | C >> | * B >> |/ >> * A >> >> commit B is placed between A and C, which is wrong because E stays that D >> and B comes after C >> >> the only correct solution here is >> >> * E >> |\ >> | * D >> | |\ >> | |/ >> |/| >> | * B >> * | C >> |/ >> * A >> >> while it seems that it contradicts to D staiting that C shoud be between D >> and B The new algorithm solves this issue >> >> Here is an example: walk to to the root via "first" parents go E -> C -> A >> print A because it has no parents step back to C print C because it has no >> other parents then step back to E go D -> B -> A do not print A becase A is >> already printed step back to B print B so on >> >> This change makes option "--topo-order" obsolete, because for a single >> commit there is only one way to order parents. "--date-order" and >> "--author-date-order" are preserved and make sence only for the case when >> multiple commits are given to be able to sort those commits. > > I have to admit that I am not a fan of this change, as I find that there > are legitimate use cases where I want to order the commits by commit > topology, and other use cases where I want to order them by date. > > Maybe other reviewers agree with your reasoning, though, in that case you > still will need to address the test failures in t4202, t4215 and t6012: > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=25867&view=ms.vss-test-web.build-test-results-tab > > Ciao, > Johannes
let me explain in more detail why I thought it would make sense to stop using "--topo-order". in case if a user specifies a single commit, like this git rev-list E with a new algorithm the options "--date-order", "--author-date-order", "--topo-order" do not change the ordering. Because there is only one way to sort any git graph with a single tip.
in case if a user specifies multiple tips, like this git rev-list --topo-order C B ^A current version of git displays commits ordered by commit timestamp which does not seem like a topological ordering.
so I decided to change the documentation so that "--topo-order" and "--date-order" be the same. And since "--topo-order" does not add anything new decided to deprecate it.
Form my point of view there should be no options to sort at all. The topology should be derived from the order provided by the user git rev-list --topo-order C B ^A should result in C, B git rev-list --topo-order B C ^A should result in B, C
Regarding the failed test I'll try to find the reason but what puzzles me is why those tests (t4202, t4215 and t6012) succeeded on all other platforms (linux32, osx-clang, windows, ...) and only failed on linux-gcc. In my machine those tests do not fail either (gcc (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0 20160609)