Re: [PATCH 0/1] Preserve topological ordering after merges

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Johannes,
thanks for you comments!
please find my replies below


07.01.2020 15:11, Johannes Schindelin wrote:
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)




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux