Skip to content

Commit sorting is wrong when commits are from different timezones #4496

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

Open
braykov opened this issue Apr 19, 2025 · 7 comments
Open

Commit sorting is wrong when commits are from different timezones #4496

braykov opened this issue Apr 19, 2025 · 7 comments
Labels
bug Something isn't working

Comments

@braykov
Copy link

braykov commented Apr 19, 2025

Describe the bug
Sorting the commits is based on date and time, but the timezones are not unified to the local one.
Therefore older commits appear higher in the list.

To Reproduce
Find a repo where people from different timezones have committed.

  1. Go to 'Commits'
  2. Click on j and k to go up and down between the commits and notice the date time in the patch sections
  3. See how sorting of commits within one day is wrong because the time zone is different

Expected behavior
I expect all commits of the repo to have CommitDate in the local timezone

Screenshots
not applicable - it's my company's data

Version info:
commit=5f809809dda06c98e51743ac8c19af6a26a5984b, build date=2025-04-14T08:30:35Z, build source=binaryRelease, version=0.49.0, os=darwin, arch=arm64, git version=2.49.0

git version 2.49.0

@braykov braykov added the bug Something isn't working label Apr 19, 2025
@ChrisMcD1
Copy link
Contributor

Thanks for the report! Can you please confirm what is the value of your commit sort order currently? This is accessed via control + l <c-l> which in the commits menu, and then selecting Commit sort order.

The default behavior is to sort by topo-order, which doesn't attempt to make any guarantees about time. If someone was to make commit A backdated into the past that has a more recent B in its history, we would still expect B to come before A. If you do have date-order or author-date-order, we would expect them to obey the ordering of those respective fields.

Assuming you have selected date-order or author-date-order, I am also seeing some weird behavior around this point, so I think you are onto something. I am continuing to look into this.


Separately, on your point here:

I expect all commits of the repo to have CommitDate in the local timezone

I would not expect that to be the case. I would expect that the patch window would display the time in terms of the timezone that the author committed it in.

Image

This view is generated by a

git -C /home/chrismcdonnell/linux-repos/git-date-testing -c diff.noprefix=false show --no-ext-diff --submodule --color=always --unified=3 --stat --decorate -p a9b1eed626914414adaf29050d7f034b566778ff --find-renames=50%

which is produced here

cmdArgs := NewGitCmd("show").
Config("diff.noprefix=false").
ConfigIf(extDiffCmd != "", "diff.external="+extDiffCmd).
ArgIfElse(extDiffCmd != "", "--ext-diff", "--no-ext-diff").
Arg("--submodule").
Arg("--color="+self.UserConfig().Git.Paging.ColorArg).
Arg(fmt.Sprintf("--unified=%d", contextSize)).
Arg("--stat").
Arg("--decorate").
Arg("-p").
Arg(hash).
ArgIf(self.AppState.IgnoreWhitespaceInDiffView, "--ignore-all-space").
Arg(fmt.Sprintf("--find-renames=%d%%", self.AppState.RenameSimilarityThreshold)).
ArgIf(filterPath != "", "--", filterPath).
Dir(self.repoPaths.worktreePath).
ToArgv()

which which without any configuration settings for a --format flag, is just relying on default git behavior.

@ChrisMcD1
Copy link
Contributor

ChrisMcD1 commented Apr 24, 2025

Okay, I've done some more digging. I wasn't even aware of the difference between commit timestamp and author timestamp prior to this. https://git-scm.com/docs/git-log#_commit_ordering for those similarly unaware. The LazyGit default of --topo-order is different from the default of no argument, which might be a point worth considering. (Although changing existing defaults will always make someone mad)

The order of commits comes from

cmdArgs := NewGitCmd("log").
Arg(refSpec).
ArgIf(gitLogOrder != "default", "--"+gitLogOrder).
ArgIf(opts.All, "--all").
Arg("--oneline").
Arg(prettyFormat).
Arg("--abbrev=40").
ArgIf(opts.FilterAuthor != "", "--author="+opts.FilterAuthor).
ArgIf(opts.Limit, "-300").
ArgIf(opts.FilterPath != "", "--follow").
Arg("--no-show-signature").
ArgIf(opts.RefToShowDivergenceFrom != "", "--left-right").
Arg("--").
ArgIf(opts.FilterPath != "", opts.FilterPath).
ToArgv()

and I created this little bash script to set up a repo with some interesting properties.

MUST RUN IN NEW DIRECTORY. WE NUKE .git

#!/bin/bash

rm -rf .git/
git init
GIT_COMMITTER_DATE="1970-01-01 12:00:00 +00:00" git commit --allow-empty -m "Commit 1" --date "1970-01-01 12:00:00 +00:00"
GIT_COMMITTER_DATE="1970-01-02 12:00:00 +00:00" git commit --allow-empty -m "Commit 2" --date "1970-01-02 12:00:00 +00:00"
git switch -c branch-old-commits 
GIT_COMMITTER_DATE="1970-01-03 12:00:00 +00:00" git commit --allow-empty -m "Commit 3" --date "1970-01-03 12:00:00 +00:00"
git switch master
git switch -c branch-new-commits 
GIT_COMMITTER_DATE="1970-02-01 12:00:00 +00:00" git commit --allow-empty -m "Commit 5" --date "1970-02-01 12:00:00 +00:00"
git switch master
git switch -c branch-slightly-older-than-new
GIT_COMMITTER_DATE="1970-02-01 12:00:00 +01:00" git commit --allow-empty -m "Commit 4" --date "1970-02-01 12:00:00 +01:00"
git switch master
git merge branch-new-commits -m "Merge New Commits"
git merge branch-old-commits -m "Merge Old Commits"
git merge branch-slightly-older-than-new -m "Merge Middle Commits"

If I'm thinking straight, the literal timeline of events is commits 1 through 5 in number order. The LazyGit default of providing --topo-order makes, which does have the commits out of order.

Image

If we reset back to git's default by going into the <c-l> menu, we get what you are likely expecting

Image

Can you try out switching to the git default explicitly and tell us if the sort order still looks weird? I found no evidence of LazyGit code that does any sorting based on date, or timezone math, so fingers crossed it shouldn't be possible for us to screw up git's presumably correct algorithm.

@braykov
Copy link
Author

braykov commented Apr 24, 2025

OK, so I tried all the options.
The sorting is wrong only for topo-order.
For the rest of the options, it is correct.

@ChrisMcD1
Copy link
Contributor

Thanks for trying them out!

I think your definition of correct and wrong might be misaligned with how git defines topo-order: https://git-scm.com/docs/git-log#Documentation/git-log.txt---topo-order

Show no parents before all of its children are shown, and avoid showing commits on multiple lines of history intermixed.

In that attempt to "avoid showing commits on multiple lines of history intermixed", it is allowed (and even required) to show commits from different branches not in time order. Do you see LazyGit ordering any commits differently in the commit graph than a raw git log --topo-order?

@braykov
Copy link
Author

braykov commented Apr 24, 2025

As commits are displayed one per line, I expect that they are displayed in chronological order, regardless of options. For me it is wrong when an older commit is listed above a newer commit. The repo I am looking at is hosted on GitLab. On its Repository Graph - the display is sorted chronologically. I also checked in several other clients and it is the same. It was only on lazygit that those two commits appeared switched.

I tested git log --topo-order and the sorting there is the same as with topo-order option in lazygit. Therefore, it IS NOT a bug in lazygit. You can probably close this report.

Finally, I do not remember what was the sort option originally when I installed lazygit (as I switched it so many times already) but I would expect the default to be as it is in other clients - anything but "topo-order".

@ChrisMcD1
Copy link
Contributor

Glad it's not a bug on our end!

@stefanhaller @jesseduffield Do either of you have some context you can share about why topo-order was chosen as the default sort order for commit graphs, and if that's a thing we wish to keep going foward?

@stefanhaller
Copy link
Collaborator

I can't answer your first question, because this was done before I joined the project.

As for the second question, yes, I definitely would keep topo-order as the default, I think this is the most reasonable default for most people. I think it should also be the default in command-line git, and I can only speculate why it isn't; maybe because of performance concerns (topo-order can be a lot slower in very large repositories).

Honestly, I don't understand the OP's complaint about the order being "wrong" with topo-order; it's very deliberate to regroup the commits so that commits on a given branch are grouped together, and I think it makes a lot of sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants