Git issues with submodules #37528

New Issue

Brecht Van Lommel · 2013-11-18T23:50:52+01:00

Brecht Van Lommel commented

2013-11-18 23:50:52 +01:00

Somehow a master branch seems to have no changes but "arc patch" or "arc diff" can give this error message:

You have unstaged changes in this working copy.

  Working copy: /Users/brecht/dev/git-blender/blender/

  Unstaged changes in working copy:
    release/scripts/addons
Usage Exception: Stage and commit (or revert) them before proceeding.

@mont29 found out that this command is used by arc and somehow lists the change:

git ls-files -m

a workaround was:

git update-index --refresh -- release/scripts/addons

Somehow a master branch seems to have no changes but "arc patch" or "arc diff" can give this error message: ``` You have unstaged changes in this working copy. Working copy: /Users/brecht/dev/git-blender/blender/ Unstaged changes in working copy: release/scripts/addons Usage Exception: Stage and commit (or revert) them before proceeding. ``` @mont29 found out that this command is used by arc and somehow lists the change: ``` git ls-files -m ``` a workaround was: ``` git update-index --refresh -- release/scripts/addons

Brecht Van Lommel commented

2013-11-18 23:50:52 +01:00

Changed status to: 'Open'

Brecht Van Lommel commented

2013-11-18 23:50:52 +01:00

Added subscribers: @brecht, @mont29

Brecht Van Lommel commented

2013-11-18 23:56:50 +01:00

So I did a simple checkout with the basic commands on the Git Usage page and immediately git ls-files -m shows the addons folder as modified, but none of the other submodules.

git clone git://git.blender.org/blender.git
cd blender
git submodule update --init --recursive --remote

I don't know if that is because they are configured different in some way or because it's just the first one alphabetically.

So I did a simple checkout with the basic commands on the Git Usage page and immediately `git ls-files -m` shows the addons folder as modified, but none of the other submodules. ``` git clone git://git.blender.org/blender.git cd blender git submodule update --init --recursive --remote ``` I don't know if that is because they are configured different in some way or because it's just the first one alphabetically.

Brecht Van Lommel commented

2013-11-19 16:03:19 +01:00

Some strange stuff with submodules happened in this commit as well blender/blender@000312ab51.

Brecht Van Lommel changed title from ~~Arcanist gives message about unstaged changes in addons when it shouldn't~~ to Arcanist issues with submodules

2013-11-19 16:03:47 +01:00

Bastien Montagne commented

2013-11-19 16:38:30 +01:00

Mmmmh… I don’t think arcanist is the issue here, it’s not arcanist which generates that "ghost change" over /release/scripts/addons/ dir, I think. I would rather incriminate git and our (rather complex) sub-module setup?

Anyway, I got again a detached head in addons, which I fixed with (within addons dir):


 git checkout master
 git reset --hard origin/master

… and git ls-files -m listed again /release/scripts/addons, which I fixed again with above command…

I still have to figure how those detached heads happen, too.

Mmmmh… I don’t think arcanist is the issue here, it’s not arcanist which generates that "ghost change" over /release/scripts/addons/ dir, I think. I would rather incriminate git and our (rather complex) sub-module setup? Anyway, I got again a detached head in addons, which I fixed with (within addons dir): ``` git checkout master git reset --hard origin/master ``` … and git ls-files -m listed again /release/scripts/addons, which I fixed again with above command… I still have to figure how those detached heads happen, too.

Brecht Van Lommel commented

2013-11-19 17:01:54 +01:00

Yes, I don't think it's a bug in Arcanist, just that it's where we are seeing the end result of these problems.

I actually looked into the Git code to see where it happens, in read_cache.c this is what makes it skip checking if the folder was actually modified and relying on the index instead (which is outdated?):

/*

   * Immediately after read-tree or update-index --cacheinfo,
   * the length field is zero, as we have never even read the
   * lstat(2) information once, and we cannot trust DATA_CHANGED
   * returned by ie_match_stat() which in turn was returned by
   * ce_match_stat_basic() to signal that the filesize of the
   * blob changed.  We have to actually go to the filesystem to
   * see if the contents match, and if so, should answer "unchanged".
   *
   * The logic does not apply to gitlinks, as ce_match_stat_basic()
   * already has checked the actual HEAD from the filesystem in the
   * subproject.  If ie_match_stat() already said it is different,
   * then we know it is.
   */

    if ((changed & DATA_CHANGED) &&
        (S_ISGITLINK(ce->ce_mode) || ce->ce_stat_data.sd_size != 0))
        return changed;

I'm not sure what to do with that information at the moment, but anyway.

Yes, I don't think it's a bug in Arcanist, just that it's where we are seeing the end result of these problems. I actually looked into the Git code to see where it happens, in `read_cache.c` this is what makes it skip checking if the folder was actually modified and relying on the index instead (which is outdated?): ``` /* ``` * Immediately after read-tree or update-index --cacheinfo, * the length field is zero, as we have never even read the * lstat(2) information once, and we cannot trust DATA_CHANGED * returned by ie_match_stat() which in turn was returned by * ce_match_stat_basic() to signal that the filesize of the * blob changed. We have to actually go to the filesystem to * see if the contents match, and if so, should answer "unchanged". * * The logic does not apply to gitlinks, as ce_match_stat_basic() * already has checked the actual HEAD from the filesystem in the * subproject. If ie_match_stat() already said it is different, * then we know it is. */ ``` if ((changed & DATA_CHANGED) && (S_ISGITLINK(ce->ce_mode) || ce->ce_stat_data.sd_size != 0)) return changed; ``` I'm not sure what to do with that information at the moment, but anyway.

Bastien Montagne commented

2013-11-21 14:53:28 +01:00

Current mess with git submodules prevented me to use 'arc land', so I had to mimic it by hand - for the records, here are the commands executed by a default 'arc land':

git checkout master
git pull --rebase
git checkout your_diff_branch
git rebase master
git checkout master
git merge --squash --ff-only your_diff_branch
git commit

Current mess with git submodules prevented me to use 'arc land', so I had to mimic it by hand - for the records, here are the commands executed by a default 'arc land': ``` git checkout master git pull --rebase git checkout your_diff_branch git rebase master git checkout master git merge --squash --ff-only your_diff_branch git commit ```

Brecht Van Lommel changed title from ~~Arcanist issues with submodules~~ to Git issues with submodules

2013-11-21 17:40:55 +01:00

Brecht Van Lommel commented

2013-11-21 17:42:19 +01:00

I just did git submodule deinit for all of them now, changes to submodules keep sneaking into commits even when they don't show as changed with git status or during git commit message type, only afterwards with git show can you see it.

I just did `git submodule deinit` for all of them now, changes to submodules keep sneaking into commits even when they don't show as changed with git status or during git commit message type, only afterwards with git show can you see it.

Jonathan Williamson commented

2013-11-21 18:08:04 +01:00

Added subscriber: @JonathanWilliamson

Julien Rivaud commented

2013-11-21 18:11:34 +01:00

Added subscriber: @FrnchFrgg

Julien Rivaud commented

2013-11-21 18:11:34 +01:00

brecht, did you submit the bug to the Git Mailing list ? They are very friendly, competent and to the point; I'm often amazed at the quality of the anwers even to seemingly stupid questions from newbies. Very high signal-to-noise ratio overall there.
If you did, let's put a gmane link here.

brecht, did you submit the bug to the Git Mailing list ? They are very friendly, competent and to the point; I'm often amazed at the quality of the anwers even to seemingly stupid questions from newbies. Very high signal-to-noise ratio overall there. If you did, let's put a gmane link here.

Brecht Van Lommel commented

2013-11-21 18:21:40 +01:00

Added subscribers: @Sergey, @ideasman42

Brecht Van Lommel commented

2013-11-21 18:21:40 +01:00

We will contact git devs, there's kind of a parallel discussion over email going on with @Sergey and @ideasman42, we just need to summarize the issues we have in a nice reproducable way.

Dalai Felinto commented

2013-11-22 06:47:31 +01:00

Added subscriber: @dfelinto

Sergey Sharybin commented

2013-11-22 08:13:35 +01:00

Some notes:

All the submodules are configured in the same way. You've got modified addons only because other repos weren't updated and not pulled any changes when you update submodules to the latest version. But if you make changes on purpose there, they'll be listed in git ls-files -m a well.
It is not bug in Arcanist. Arcanist just uses git ls-files -m to see whether there're uncommited unstaged changes. Workaround could be to replace it with git diff-index --name-only HEAD --. This seems to ignore submodules changes as it should be.
Detached HEAD is totally fine. It is written in git submodule documentation then unless you use --rebase argument to git submodule update you'll have detached HEAD. This is kinda logical in fact from, just a bit annoying if you want to push from submodule. I've tweaked commands in documentation which makes it so submodules stays in master branch.
I have still no idea how @ideasman42 manages to commit changes to submodules hash. We're using ignore=all for all the submodules, and i could not commit changes to hash on purpose even. Could be a bug in some particular Git versions?

@FrnchFrgg yes, i had experience with Git ML in the past and guys there are really great. I'm collecting some more details to fire up question/possible report to their mailing list.

Some notes: - All the submodules are configured in the same way. You've got modified addons only because other repos weren't updated and not pulled any changes when you update submodules to the latest version. But if you make changes on purpose there, they'll be listed in `git ls-files -m` a well. - It is not bug in Arcanist. Arcanist just uses `git ls-files -m` to see whether there're uncommited unstaged changes. Workaround could be to replace it with `git diff-index --name-only HEAD --`. This seems to ignore submodules changes as it should be. - Detached `HEAD` is totally fine. It is written in git submodule documentation then unless you use `--rebase` argument to `git submodule update` you'll have detached `HEAD`. This is kinda logical in fact from, just a bit annoying if you want to push from submodule. I've tweaked commands in documentation which makes it so submodules stays in master branch. - I have still no idea how @ideasman42 manages to commit changes to submodules hash. We're using ignore=all for all the submodules, and i could not commit changes to hash on purpose even. Could be a bug in some particular Git versions? @FrnchFrgg yes, i had experience with Git ML in the past and guys there are really great. I'm collecting some more details to fire up question/possible report to their mailing list.

Sergey Sharybin commented

2013-11-22 11:14:59 +01:00

Sent mail to git mailing list. Here's link to thread: http://article.gmane.org/gmane.comp.version-control.git/238173

Be welcome to provide more info there as we've got more :)

Sent mail to git mailing list. Here's link to thread: http://article.gmane.org/gmane.comp.version-control.git/238173 Be welcome to provide more info there as we've got more :)

Brecht Van Lommel commented

2013-11-22 14:50:54 +01:00

Nice coincidence, Arcanist just got a fix for this to use git diff-index --name-only HEAD instead of git ls-files -m:
https://secure.phabricator.com/rARCe62b23e67deacc24469525cc5dea2b297a5073fb

Nice coincidence, Arcanist just got a fix for this to use `git diff-index --name-only HEAD` instead of `git ls-files -m`: https://secure.phabricator.com/rARCe62b23e67deacc24469525cc5dea2b297a5073fb

Sergey Sharybin commented

2013-11-22 16:30:15 +01:00

Maybe they follow git ML discussion? :)

Well, that nice but still doesn't solve issues with @ideasman42's commits to addons SHA. But since can not reproduce it by self would ask Cambo to investigate this a big and report to Git's ML.

Maybe they follow git ML discussion? :) Well, that nice but still doesn't solve issues with @ideasman42's commits to addons SHA. But since can not reproduce it by self would ask Cambo to investigate this a big and report to Git's ML.

Sergey Sharybin commented

2013-11-22 16:35:37 +01:00

Hold on, i only see changes to the comments. Command itself is still the same. Am i missing something?

Edit:

They only documented the issue with ls-files, no functional changes were made in fact.

Hold on, i only see changes to the comments. Command itself is still the same. Am i missing something? **Edit:** They only documented the issue with ls-files, no functional changes were made in fact.

Brecht Van Lommel commented

2013-11-22 17:19:04 +01:00

Oh sorry, got excited too early.

Dalai Felinto commented

2013-11-22 17:25:15 +01:00

That's not a coincidence :) The 'upstream bug' link is @Sergey 's email to git list.
And yes, no real code change yet . . . but at least the workaround works here too (OSX 10.8.5)

That's not a coincidence :) The 'upstream bug' link is @Sergey 's email to git list. And yes, no real code change yet . . . but at least the workaround works here too (OSX 10.8.5)

Sergey Sharybin commented

2013-11-22 22:31:30 +01:00

Seems it is in upstream already: https://secure.phabricator.com/rARC6033f1422144db95dee9003b58ed5198c2f5e372

Git pull and use new arcanist :) Workaround for remained issues i've posted to the ML.

Seems it is in upstream already: https://secure.phabricator.com/rARC6033f1422144db95dee9003b58ed5198c2f5e372 Git pull and use new arcanist :) Workaround for remained issues i've posted to the ML.

Dalai Felinto commented

2013-11-26 00:29:01 +01:00

can this be closed? things are working for me with latests arcanist

Brecht Van Lommel commented

2013-11-26 00:57:38 +01:00

Changed status from 'Open' to: 'Resolved'

Brecht Van Lommel closed this issue

2013-11-26 00:57:38 +01:00

Brecht Van Lommel self-assigned this 2013-11-26 00:57:38 +01:00

Brecht Van Lommel commented

2013-11-26 00:57:38 +01:00

I think so. There's still a bug in git that the git developers are solving, but we now know how to avoid the problem by not doing "git add ." or "git commit release/scripts" for example, since those silently include submodule changes.

Sv. Lockal commented

2013-12-15 14:57:38 +01:00

Added subscriber: @Lockal

Sign in to join this conversation.

No Milestone

No project

No Assignees

7 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: infrastructure/blender-org#37528

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Git issues with submodules #37528