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

don't force checking for dirty submodules #357

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

orn688
Copy link

@orn688 orn688 commented Oct 21, 2022

opts.ignore_submodules = GIT_SUBMODULE_IGNORE_DIRTY is equivalent to setting the git status flag --ignore-submodules=dirty, which takes precedence per-submodule ignore settings in .gitmodules. This means that gitstatus returns a different result than a default git status call with no flags if there are any dirty submodules that are marked ignore = all in .gitsubmodules.

This is confusing because git status and gitstatusd will be out of sync and there's no obvious reason why.

This should only result in a behavior change for projects with submodules that set the .gitmodules config value
submodule.<name>.ignore to something besides none or dirty, but in all cases it will ensure git status and gitstatusd are in agreement.

Fixes #356.

`opts.ignore_submodules = GIT_SUBMODULE_IGNORE_DIRTY` is equivalent to
setting the `git status` flag `--ignore-submodules=dirty`, which takes
precedence per-submodule `ignore` settings in .gitmodules. This means
that `gitstatus` returns a different result than a default `git status`
call with no flags if there are any dirty submodules that are marked
`ignore = all` in .gitsubmodules.

This is confusing because `git status` and `gitstatusd` will be out of
sync and there's no obvious reason why.

This should only result in a behavior change for  projects with
submodules that set the .gitmodules config value
`submodule.<name>.ignore` to something besides `none` or `dirty`, but in
all cases it will ensure `git status` and `gitstatusd` are in agreement.
@romkatv
Copy link
Owner

romkatv commented Oct 21, 2022

If you remove that line, are you sure it'll respect the per-submodule ignore settings in .gitmodules? Could you test it?

Also, I vaguely remember that there was a setting that makes git status recurse submodules. Does this really exist or am I misremembering? If it does exist, could you test what happens if you set it?

@orn688
Copy link
Author

orn688 commented Oct 21, 2022

Good idea, I just tested it out. Removing ignore = all from an entry in .gitmodules corresponding to a dirty submodule and committing the change causes gitstatusd to indicate that the repo is dirty, consistent with git status. So it seems to be working as intended.

I don't see any recurse-submodules option for git status, but there is git submodule status, which takes an optional --recursive flag – is that what you mean?

@romkatv
Copy link
Owner

romkatv commented Oct 21, 2022

I don't see any recurse-submodules option for git status, but there is git submodule status, which takes an optional --recursive flag – is that what you mean?

I don't know. What I do know is that the line this PR is deleting was put there for a reason and presently we don't know what this reason was. If you could find out, and if this reason is no longer relevant, I'll merge it.

@orn688
Copy link
Author

orn688 commented Oct 21, 2022

I don't know. What I do know is that the line this PR is deleting was put there for a reason and presently we don't know what this reason was. If you could find out, and if this reason is no longer relevant, I'll merge it.

You added it in version 0.1 (code), but unfortunately I don't see any comment or anything to explain why you put it there in the first place.

Based on a Github Code Search, I also don't see any other major projects that are using the GIT_SUBMODULE_IGNORE_DIRTY option, so I have no idea why you might have chosen to use it in this project.

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.

gitstatus ignores the ignore field of .gitmodules
2 participants