How to commit incomplete changes?

 Do people have any feelings or conventions for how and when to publish
 a series of commits where the first one(s) break something and the next
 ones clear it up?  I've found some discussion, but with vague results.

 I'm about to commit some small edits which go together with bigger
 generated changes.  It seems both more readable and more cherry-pick-
 friendly to me to keep these in separate commits.

 What I've found is I can use a line in the commit message like
     "Incomplete change, requires next commit (update foo/ dir)."
 and, if there is any point, do a no-ff merge past the breakage.

I'm curiuos, why to you want to commit changes that break something separately from fixup?

Many conventions (as I know) use ideology that every commit must NOT BREAK existing code or tests. Every SHARED commit. Git design (as you must be already know) allows you to make/change/reorder as many commits as you want (before you share them or push to a "central" repository). So, you have not to be afraid to commit every your change, because you can always change/fixup/split your commits.

Usually, you introduce a feature in a branch. Also, your project must have (?) (mine do have) unit-tests, at least. And most changes must be tested. So, breakage must be discovered early, even after some other commits in that feature branch. In that case you can just make a fixup commit and then rebase it on a breakage commit with a "squash".

And only after all features made and all tests passed you can share them (push to another repo).

 On Thu, 15 Dec 2011 10:44:44 +0400, Alexey Shumkin 
 <Alex.Crezoff@gmail.com> wrote:

 I answered that, but maybe too briefly:

 To expand on that: To review the change, review the hand-edited 
 commits,
 which is easier when these do not drown in generated changes.  Review
 the *commands* which generated the rest - I'd put those in the commit
 message - and glance at the actual changes.  Cherry-pick: Possbly you
 need to run the commands instead of cherry-picking the generated
 changes.  That's easier with a commit with only generated changes.

 I know it also can cause problems.  Would you make a single big commit
 anyway, and describe carefully in the commit message which parts are
 hand-edits?  (We don't auto-test commits yet, but I'll sure this issue
 will crop up again later when we do.)

Oh! I got it. I missed "generated changes". Well, unfortunately (or fortunately ;) ), I did not meet such a workflow when changes are "generated" without my hands. In your case it may sound reasonable to make separate fixup commit. But Git allows you to make your own (more flexible than SVN, for instance) workflow, which suits you. It's up to you ;) You decide. If you plan cherry-picking that fixups, do separate fixups. Just publish them together. If you want every commit is "clear" and "workable", squash fixup into a single commit. I do not know what exactly is "generated changes" you're talking about ), so, maybe I'd do separate fixups, maybe not. )) There is no single solution. ))) TIMTOWTDI That is why you hesitate :) Do your own decision. And feel free to change it later. ))

* "Make your own decision", of course :)

On 12/14/2011 5:24 PM, Hallvard B Furuseth wrote:

A main purpose for the squash and fixup options is (as Randall Schwartz put it in his git video http://www.youtube.com/watch?v=8dhZ9BXQgc4) "To make it look like you did it all perfectly without making any mistakes" (or a reasonable facsimile thereof). You insights on the cherry-picking of fixes is interesting, but makes no sense in the context of unpublished work. Why would you need to cherry-pick fixes to mistakes that have not yet been propagated (published)? If the cherry-picks of fixes are for your other already merged local branches then just save the pre-squash/fixup version of the branch to another branch, (ie, git branch mybranch-b4-fixup) and cherry-pick from that unsquashed copy to patch up your other unpublished branches. Keep in mind that cherry-pick is not alway the best way to apply fixes. A merge or rebase to get the fix is the sign of a better workflow in many cases, TBOMK. On the other hand, if the bugs have been published then you have no choice but to commit the fix separately because you can't rearrage/edit published history. Keep in mind that ideally commits should be logical. You can use the rearrage feature of interactive rebase to squash fixes into the feature commit they go to. IOW, I don't think squashing everything into a giant commit just to consolidate bugfixes into a single commit makes sense if that would mean losing the distinct separation between differing feature commits.

I assume by 'generated changes' you mean the automerge in git that is a wonderful default for vast systems like the linux kernel in which code is unlikely to overlap logically, but very dangerous in legacy application systems where changes to the same file can create logical bugs despite not being on the 'exact same line of code'. You are supposed to review all your merged files after a merge regardless. However, we don't trust ourselves that much in our shop so we force conflicts on same-file edits by making "user-date stamp" updates on "line 1" (depends on language-dependent comment line rules) in our pre-commit hook. That way we are forced to manually review the merge of same-file edits "by hand" thus avoiding "generated results". Of course, unique-file edits can still break things and thus a merge review is still in order.

Hope this helps. I'm not a git workflow expert, but my comments are based on experience. I too am still looking for better ways to manage workflow while leveraging the flexibity and agility of git for concurrent development.

v/r, neal

Neal Kreitzinger <nkreitzinger@gmail.com> writes:

My reading of the "need to split" example was not "bulk of work plus fixes to mistakes". Imagine you are working on somebody else's code and for some reason you want to do

	s/setenv/xsetenv/g

all over the code, and also add a wrapper to implement xsetenv() function.

You _could_ do it in one single commit, but what happens when you try to adjust to the updated upstream code, which may have added new callsites to setenv()?

If you keep it as two patches, one is mechanical (i.e. s/setenv/xsetenv/g) and the other is manual (i.e. implementation of xsetenv()), then you can discard the text of the "mechanical" one from the old series and instead run the substitution on the updated code, and then cherry-pick the "manual" one.

If you did the mechanical one first, the resulting code would not compile (lacks xsetenv() implementation), and then the second "manual" one would "fix" it. In this simplified example, it is easy to flip the orders and keep things work, but then you would get a complaint from clever compiler or linker that xsetenv() implementation is defined but nobody uses it, which is another kind of breakage. So it _is_ possible that you cannot avoid breaking the system inside two patches, making them "all-or-none" series.

[Neal Kreitzinger]

No. And to your questions of why I want this with unpublished work: No. Like I wrote, I'm talking about published commits.

[Junio C Hamano]

Yes - except there is no "mistakes" since it's deliberate. I'd do s/setenv/xsetenv/g, which does too little (misses some preprocessor stuff) or is too greedy, then commit anyway and clean up in next commit.

I could make and commit a much more complicated script to do it all, but that's unhelpful when trying to read what the heck the change is doing. And who knows what it'd do when run on a somewhat different codebase.

That example matches a future internal API change. My current issue is changes generated with 'autoreconf' - after cleaning up an utter libtool/automake mess by hand, which will break things if I don't autoreconf in the same commmit.

Indeed. In this case, it'd be when cherry-picking from the devel branch to the release branch. These still differ too much, a legacy of our old CVS workflow.

Yes.  I'd order it in a sequence which never broke anything if I could.

Well, come to think of it: Possibly I could introduce some new code which would only exist for the sake of patching over the temporary breakage, and then delete that code again 2-3 commits later. In this case, I'd among other things create an obsolete libtool.m4 which is currently hiding inside aclocal.m4. Not sure if that makes more sense than just having a few broken commits.

I wrote:

Wait, I see - as-yet unpublished commits, yes. Which seem best to me to publish that way. And which will get cherry-picked later, after commit and testing.

On 12/15/11 12:24 AM, Hallvard B Furuseth wrote:

Why do you store generated code? Usually you only store what is absolutely necessary. That means generated code should be generated as needed (part of the build process for example).

tom