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.

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

gitstatus ignores the ignore field of .gitmodules #356

Open
woody77 opened this issue Oct 21, 2022 · 12 comments · May be fixed by #357
Open

gitstatus ignores the ignore field of .gitmodules #356

woody77 opened this issue Oct 21, 2022 · 12 comments · May be fixed by #357

Comments

@woody77
Copy link

woody77 commented Oct 21, 2022

When used with a superproject that specifies submodules with the ignore field set to all, gitstatus will count it as an 'unstaged change' if the submodule is not at its own HEAD.

submodule configuration from .gitmodules:

[submodule "external/foo"]
        path = external/foo
        url = https://some.server.com/foo
        name = foo
        ignore = all

running git status doesn't show it as having unstaged changes:

╰─ git status
On branch bar
Your branch is ahead of 'origin/main' by 1 commit.
  (use "git push" to publish your local commits)

nothing to commit, working tree clean

unless it's run with --ignore-submodules=<value other than 'all'>:

╰─ git status --ignore-submodules=dirty
On branch bar
Your branch is ahead of 'origin/main' by 1 commit.
  (use "git push" to publish your local commits)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   external/foo (new commits)

no changes added to commit (use "git add" and/or "git commit -a")

but the prompt string constructed by gitstatus does count it as having unstaged changes:

bar:main ⇡1 !1 
@woody77
Copy link
Author

woody77 commented Oct 21, 2022

I wouldn't be surprised if this is a weirdness from our superproject setup (we have 115 submodules defined), but these that are being marked dirty don't seem to be any different from those that aren't being marked dirty. They're in detached-HEAD state, and may or may not be detached at a point in history which is the same as the upstream:

╰─ git log
commit 755ae0abe204b1821117e61645c5a3579a4a0dcd (HEAD, origin/main, origin/HEAD)

@woody77
Copy link
Author

woody77 commented Oct 21, 2022

One of my coworkers just found that gitstatusd is always passing --ignore-submodules=dirty:

https://github.com/romkatv/powerlevel10k/blob/58f5470cd98343581853dafe2a99430c0a6975e5/gitstatus/src/repo.cc#L280

@romkatv
Copy link
Owner

romkatv commented Oct 21, 2022

Might be related: romkatv/powerlevel10k#1608

@romkatv
Copy link
Owner

romkatv commented Oct 21, 2022

One of my coworkers just found that gitstatusd is always passing --ignore-submodules=dirty:

https://github.com/romkatv/powerlevel10k/blob/58f5470cd98343581853dafe2a99430c0a6975e5/gitstatus/src/repo.cc#L280

Do you believe this explains something?

@orn688
Copy link

orn688 commented Oct 21, 2022

One of my coworkers just found that gitstatusd is always passing --ignore-submodules=dirty:
https://github.com/romkatv/powerlevel10k/blob/58f5470cd98343581853dafe2a99430c0a6975e5/gitstatus/src/repo.cc#L280

Do you believe this explains something?

Yes, we're admittedly in a weird situation – our project (fuchsia.googlesource.com/fuchsia) is migrating from a custom multi-Git-repo management system ("jiri") to Git submodules, and during the transition we'll have submodules enabled but let jiri choose what version to check out. During that time there may be some skew between submodules' pinned versions and the versions that jiri checks out, which causes them to be considered dirty.

However, I think it's strange that gitstatusd sets non-default flags on git status. It's confusing for git status to show that everything is clean (which happens for us because we have ignore = all set for every submodule in .gitmodules) but for powerlevel10k to show that the repo is dirty, but with no indication of why it differs from git status's output. So I feel like it would make sense to have gitstatusd stop setting ignore_submodules to ensure it shows the same thing as git status by default.

Does that make sense, or is there a strong reason for gitstatusd to differ from git status by default?

@woody77
Copy link
Author

woody77 commented Oct 21, 2022

I can confirm that if I change .gitmodules to set ignore=dirty on one of the afflicted modules, git status shows it as dirty:

╰─ git status
On branch foo
Your branch is ahead of 'origin/main' by 1 commit.
  (use "git push" to publish your local commits)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   .gitmodules
	modified:   third_party/Vulkan-Loader (new commits)

no changes added to commit (use "git add" and/or "git commit -a")

@woody77
Copy link
Author

woody77 commented Oct 21, 2022

What I haven't figure out with the 9 repos (out of 115) that I'm seeing this with, is what makes them different.

All (marked dirty and not) are:

  • using ignore=all in the submodule definition
  • in detached HEAD at the same revision as the origin/main branch (that I have locally):
commit 51515f54192d0dcfb528db6060567f6d7eaf8457 (HEAD, origin/master, origin/main, origin/HEAD)

But aside from that, the use of GIT_SUBMODULE_IGNORE_DIRTY is overriding the submodule settings in .gitignore.

@romkatv
Copy link
Owner

romkatv commented Oct 21, 2022

Do you have a workaround that you can employ on your end? If so, that would be the best way to go. If not, your next alternative is to send a PR. If it's clean and safe, I'll merge it and do a release. If it requires non-trivial amount of work from me, I'm unlikely to do it.

@orn688 orn688 linked a pull request Oct 21, 2022 that will close this issue
@orn688
Copy link

orn688 commented Oct 21, 2022

Thanks! Here's a fix: #357

I built and tested locally and confirmed that it works.

@romkatv
Copy link
Owner

romkatv commented Oct 23, 2022

Do you have a workaround that you can employ on your end?

@orn688
Copy link

orn688 commented Oct 24, 2022

I can't find any workaround other than patching #357 into my local powerlevel10k checkout and building gitstatus manually, which is not too difficult, and probably good enough.

However, this does seem like a genuine bug in gitstatusd - it's very surprising that gitstatusd would produce a different "is dirty" result than a git status, and there doesn't seem to be a reason to keep that behavior besides backward compatibility.

I won't insist on you merging the PR though, I trust your judgment (and thank you for your work on powerlevel10k, it's fantastic!).

@romkatv
Copy link
Owner

romkatv commented Oct 24, 2022

At this point, given that we have no theory of why the line of code was added in the first place, I would need to go, read the code and run a bunch of tests. I cannot just merge the PR without doing this. The chance that I'll get around to this is low. Perhaps 10% or so that I'll do it in 2022.

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 a pull request may close this issue.

3 participants