git stash deletes/drops changes of "assume-unchanged" files

Hello (and please CC me on replies).

I was unpleasantly surprised to discover yesterday that doing `git stash` on a repository where I had previously run `git update-index --assume-unchanged FOO` completely lost all changes I had in file FOO.

I understand the behavior is logical from the way git-stash is implemented, but it strikes as very undesirable behavior to me. Does somebody think something could be done to change it? In my opinion, both including in the stash changes in FOO, or just leaving them in the working tree, would be better alternatives.

Thoughts?

-8<-

% git init % echo file1 contents >file1; echo file2 contents >file2 % git add file1 file2; git commit -m Initial.

% echo changes for file2 >>file2 % echo some other changes to file 1 >>file1 % git update-index --assume-unchanged file1

% git stash % cat file1 file1 contents % git stash show -p

Adeodato Simó <dato <at> net.com.org.es> writes:

I just ran into this today.

Was a decision about this behavior reached in the intervening time?

Jim Greenleaf <james.a.greenleaf@gmail.com> writes:

When you mark a file assume-unchanged, git internally sets a flag that this file should not be considered when doing cache refreshes -- the file is always assumed to be up-to-date.

So while I haven't actually looked into all of the code, I imagine it goes something like this:

* git-stash uses git update-index --all on all modified files.  But it
  doesn't show up as modified, because you promised it isn't.

* Later it calls git reset --hard, which blows away the existing state.
  This would seem to ignore the assume-unchanged flag in this case, as
  otherwise it wouldn't overwrite it.

Whether the last behavior is a bug is in the eye of the beholder. In your case you apparently lost work. However, 'git reset --hard' in itself should discard all uncommitted work without asking any further questions (because it's --hard). So the bug is then in the sequence

  ask about uncommitted work
  save it elsewhere
  git reset --hard

assuming that this actually makes sure nothing gets lost. But the only thing that was lost was *files that you promised would not be changed*.

What's really unfortunate is that we caused this in the first place by Pasky's 6259ac6 (Documentation: How to ignore local changes in tracked files, 2008-07-18). It recommends exactly the --assume-unchanged strategy to ignore changes to tracked files.

And it's hard to disagree with its commit message:

    This is currently probably one of the top FAQs at #git and the
    --assume-unchanged switch is not widely known

Except that now the corresponding FAQ is that we have to actively dissuade people from using --assume-unchanged precisely because it keeps biting people.

So maybe it would be time to first make up our minds as to what --assume-unchanged should actually mean:

* Ignore changes to a tracked file, but treat them as valuable.  In
  this case we'd have to make sure that failures like git-stash's are
  handled properly.

* Ignore changes to a tracked file, as in "who cares if it was changed".

* A very specific optimization for users who know what they are doing.

Thomas Rast <trast@inf.ethz.ch> writes:

It has always been a promise the user makes to Git that the working tree files that are marked as such will be kept identical to what is in the index (hence there is no need for Git to check if they were modified). And by extension, Git is now free to choose reading from the working tree file when asked to read from blob object recorded in the index for that path, or vice versa, because of that promise.

It is not --ignore-changes bit, and has never been. What are the workflows that are helped if we had such a bit? If we need to support them, I think you need a real --ignore-changes bit, not an abuse of --assume-unchanged.

Junio C Hamano <gitster@pobox.com> writes:

I gather -- from #git -- that it's mostly used for config files, which have an annoying habit of being different from the repository.

Which is wrong, really. But we still claim that --assume-unchanged is a solution to it in git-update-index(1).

Thomas Rast <trast@inf.ethz.ch> writes:

That is doubly wrong, then ;-)

How would we want to proceed from here? The obvious first step would be to fix the documentation, but then what is next?

Thinking aloud, ignoring that "Which is wrong, really" part in your message and assuming that we do want to support --ignore-changes....

Can the way we handle "--ignore-changes" files be a strict superset (or is it subset?) of what we currently do for "--assume-unchanged"? That is, if we "fix"^Wchange the behaviour of "--assume-unchanged" to be less aggressive in assuming that the user kept his promise, can we get "--ignore-changes" without losing much of the performance benefit of "--assume-unchanged" the people who originally wanted to have that feature have enjoyed for all these years?

If you are working on a project with a large working tree, by marking paths in one directory you do not care about (and do not use) with the --assume-unchanged bit, checking out another branch can be done without inspecting if there are uncommitted changes in the part of the working tree that may be clobbered with the different version of the file in the other branch. That has to go for "--ignore-changes", for example. Are there others that need to suffer?

If so, these two have to be done as totally independent options, but if -ignore-changes can be just a slightly less agressive -assume-unchanged, we could "fix" "--assume-unchanged", introduce "--ignore-changes" as a synonym and be done with it. I highly doubt that is doable.

The only sensible way forward, it seems to me, is introduce a proper "--ignore-changes" that is independent from "--assume-unchanged". What does "--ignore-changes" really mean?

The end user does not want to see changes to a config file when he runs "git status" and "git diff". I think "git commit -a" would ignore the local changes to the configuration file as a natural consequence if we teach "git status" to ignore paths marked with the "--ignore-changes" bit. But the same "git diff" (between the index and the working tree) logic is internally used to decide if a path has local changes when running "git checkout" to check out another branch, "git rebase" to see if there are local changes, etc. and the user do want to view the paths as modified.

I am not so sure if there is a clear semantics other than an unactionable blanket statement "ignore local changes".

On Thu, May 23, 2013 at 7:20 PM, Junio C Hamano <gitster@pobox.com> wrote:

The wording of --ignore-changes suffers the same lack of clarity that --assume-unchanged does.

  --assume-unchanged : These changes are ephemeral

  --ignore-changes : These changes are precious but not to be committed

What's better? --sequester is probably too obscure. Maybe --hold. Or --silence. Or --shut-up.

Does this mean a new class of files for git-status? Added, changed, untracked, ignored and held?

I wonder if there is a use case for such a switch to be applied to content rather than files. Suppose I want to --hold these changes, but further changes to the same files should be fair game? That's probably an insane situation for some future itch and not this one. Anyway, it sounds like it would involve the index or maybe a 2nd index.

Phil

Phil Hord <phil.hord <at> gmail.com> writes:

How about --freeze?

On Fri, May 24, 2013 at 03:34:26PM +0000, Jim Greenleaf wrote:

I wonder if this would be better as a file rather than another option to git-update-index. We already have .git/info/exclude so we could add .git/info/freeze or .git/info/local with the same syntax as the normal .gitignore file.

John Keeping <john <at> keeping.me.uk> writes:

.git/info/freeze would be a good solution. It would avoid the need to add a new class of files for git-status, while keeping a simple, familiar record of all frozen files in a single location.

On Fri, May 24, 2013 at 03:42:37PM +0000, Jim Greenleaf wrote:

Now I've thought about it a bit more, I'm not sure this does work.

If an entry in the freeze list means "ignore local changes in this file", we really want to be talking about local changes relative to some base. Otherwise, what happens if the upstream file is radically altered? A user probably doesn't want to keep their file unchanged when this happens.

So we don't just want to store the filename, we want to store the version of the file that the user chose to ignore. One way to do this might be to mark the file as a conflict whenever a change to it comes in and ignore the freeze file when there is a conflict in the index. But then we either need to introduce a new command to manage this state or some way for the user to perform Git operations ignoring the freeze file, otherwise how can the user pull down updates?

Perhaps a more user-friendly way to handle this would be to introduce auto-stash around any operation that will modify a frozen file. So we stash the user's (frozen) changes and then apply them after changing the file. If there are conflicts then these are marked in the index and must be resolved, then the unstaged changes in the file are ignored again.

  Hi!

On Fri, May 24, 2013 at 12:56:50AM +0200, Thomas Rast wrote:

  Indeed, it has been my lack of imagination regarding what can go
wrong. I am fine with the changes not being shown in `git diff` and even
not so worried about them being overwritten by a merge/checkout
(touching that file for other purposes), but `git stash` dropping the
changes is rather vicious. ;-)

  An emergency fix would be to add a warning to the documentation that
under various circumstances, your changes may get overwritten and keep a
backup copy. It's a bit silly, I'm not sure how long it may take to
flesh out a proper solution; if we just stop recommending anything (or
recommend something unhelpful like "you don't want that"), people will
just refer to the old advice and I think it's better to warn them.

  The main workflow for me is when you don't get to pick the workflow.
Most recently, I found myself tackling this scenario:

  (i) https://github.com/huceke/omxplayer carries file Makefile.include

  (ii) I'm paid to make some modifications to the omxplayer software
on short notice.

  (iii) Makefile.include hardcodes some crosscompiling tool paths and
other things (like CFLAGS) that are different in my setup.

  For the first few commits, I have avoided using -a, then I went ahead
and marked Makefile.include with --assume-unchanged. It felt like
something dangerous, so I also made a backup of the file for good
measure; that turned out to be a good idea after the first `git stash`
issued. (Unfortunately, I forgot about the problem before I would have
time to think about fixing that.)

  Yeah, omxplayer's setup is not ideal. But in this scenario, I'm not
really in the position to easily start poking into other people's
toolchain setup, I'd like git just to help me get my work done and move
on and ideally keep my pull requests clean of unrelated commits.

  Just to clear up on what the best practice is, I'd imagine the setup
to be something like:

	(a) Makefile contains inclusion of Makefile.include.

	(b) There is a file like Makefile.include.template containing
	a template to be copied over and filled by the user.

	(c) Makefile contains code that makes sure all variables that
	are supposed to be set are set and obsolete variables are not,
	since there is no mechanism to cause e.g. a merge conflict
	on change of Makefile.include.template.

Is there a better way to solve this?

  There are a couple of things to notice here:

  (i) The solution is highly specific for the particular file format
and usage, universal recommendations are difficult especially if we
are to cover (c).

  (ii) The solution is certainly not the simplest one to occur to
the original author, who will probably initially just commit
Makefile.include with the values suitable for them.

  (iii) A corrolary to (ii), the person who will find tackling this
problem first will probably be a newcoming developer to the project
who is likely not to be familiar with it and its toolchain / config
mechanisms, and this will be a huge hassle.

  Therefore the demand for Git to just solve their problem on its level.
Of course Git would be simpler and more elegant if it didn't have to do
this and cover all the annoying corner cases. But is this simplification
worth the extra workflow hassle for its users?

On Fri, May 24, 2013 at 01:57:12AM +0200, Petr Baudis wrote:

I think the best practice would be what Git itself does ;-)

The Makefile sets default values for all parameters, some of which are inferred based on the system. It then includes config.mak, which allows the user to override any of these values.

On Fri, May 24, 2013 at 09:22:53AM +0100, John Keeping wrote:

So that's pretty similar to what I described, modulo the filenames. I'd say it's more friendly if you don't need to tweak any of the defaults in the common case, but less friendly if you always need to tweak something/everything (you really want a template file then and not covering (c) is a problem).

On Fri, May 24, 2013 at 11:40:07AM +0200, Petr Baudis wrote:

I don't see anything wrong with having a template file documenting the parameters, but I think it's important that there are sensible defaults in place when the user's configuration file does not specify a value for a parameter. It wasn't clear to me from your definition that there were defaults to be overridden by the user's configuration file, as opposed to forcing the user to define certain values and causing an error if those are not defined.

On Fri, May 24, 2013 at 11:06:12AM +0100, John Keeping wrote:

That's the case in plenty of situations - when specifying usernames and passwords and server hostnames, paths to cross-compiling environments that pretty much everyone has at a different place, and so on.

On Fri, May 24, 2013 at 12:14:16PM +0200, Petr Baudis wrote:

Yeah, I didn't mean to say that everything can have a sensible default.

Going back to where this started, in the omxplayer Makefile, I would map my suggestion to a change like this:

    * Change most of the ":=" in Makefile.include to "=" so that the
      order of variable definition matters less
    * Move Makefile.include to Makefile.defaults
    * Change the "include Makefile.include" at the top of Makefile to:

        include Makefile.defaults
        -include Makefile.config
    
    * Add Makefile.config to .gitignore

So that it continues to Just Work for people using buildroot but you can create Makefile.config to override those defaults.

I agree that this isn't possible in all cases, and your template approach is certainly useful for configuration files - particularly because those templates can be included in end-user documentation or the installation as they are likely to be needed in the installed application and not just development.

On Fri, May 24, 2013 at 11:40:18AM +0100, John Keeping wrote:

  Indeed, that doesn't cover some corner cases of (c), but that's not a
big deal in practice I guess.

  My point still stands - this is extra hassle, done just for the sake
of the tool; I think the tool should not get in the way. Moreover, it's
not the default solution for your typical original author and therefore
you will still often find yourself in a situation where you have to deal
with a setup that's broken already.

On Fri, May 24, 2013 at 01:03:22PM +0200, Petr Baudis wrote:

I think we're in violent agreement here.

I can see that there are cases where an --ignore-changes option that behaves like --assume-unchanged but without ever overwriting the local file is a useful feature. I was simply trying to point at what I consider best practices for makefiles, which was relevant for the example you gave. Sorry if that was unclear.

----- Original Message -----

The web team at my $dayjob has the same problem, and I believe they are also using --assume-unchanged.

This may be slightly too tangential, but a different workflow we experimented with is marking the config file(s) merge=ours in gitattributes on each branch.  Ideally then devs can check in their local settings on their local branches.  Unfortunately, as is probably well known here, the merge attribute is only checked by the low level merge algorithm, so too often settings got bashed incorrectly (only one merge parent changed the file).  Perhaps there are some options in that direction?

Thanks, Stephen