diff options
author | Thomas Schwinge <tschwinge@gnu.org> | 2009-06-23 17:15:59 +0200 |
---|---|---|
committer | Thomas Schwinge <tschwinge@gnu.org> | 2009-06-23 19:09:52 +0200 |
commit | 181d1bab7b75b070918c65dfc05fbb3c3c5c1344 (patch) | |
tree | 822eb60d784c6111bd61780573e84eea5e9d2758 /community/weblogs | |
parent | 99224d3e476fb975bfbc2ff92a4167d7a52ae9b9 (diff) |
community/weblogs/tschwinge/2009-06-23_split_patch_and_git_rebase_--interactive: New article about splitting a patch into three, and then some ``git rebase --interactive''.
Diffstat (limited to 'community/weblogs')
2 files changed, 545 insertions, 0 deletions
diff --git a/community/weblogs/tschwinge/2009-06-23_split_patch_and_git_rebase_--interactive.mdwn b/community/weblogs/tschwinge/2009-06-23_split_patch_and_git_rebase_--interactive.mdwn new file mode 100644 index 00000000..bd08060e --- /dev/null +++ b/community/weblogs/tschwinge/2009-06-23_split_patch_and_git_rebase_--interactive.mdwn @@ -0,0 +1,545 @@ +[[!meta copyright="Copyright © 2009 Free Software Foundation, Inc."]] + +[[!meta license="""[[!toggle id="license" text="GFDL 1.2+"]][[!toggleable +id="license" text="Permission is granted to copy, distribute and/or modify this +document under the terms of the GNU Free Documentation License, Version 1.2 or +any later version published by the Free Software Foundation; with no Invariant +Sections, no Front-Cover Texts, and no Back-Cover Texts. A copy of the license +is included in the section entitled [[GNU Free Documentation +License|/fdl]]."]]"""]] + +[[!meta title="splitting a patch into three, and then some git rebase +--interactive"]] + +I was revisiting the issue of getting the Hurd's code base compiled with recent +versions of GCC. Specifically, there were a lot of duplicate symbols shown at +linking time, and all these were related to `inline` functions. Originally, in +2007, we had solved this problem already (or rather, shifted it) by using GCC's +`-fgnu89-inline` option, but as we [[!GNU_Savannah_patch 6840 desc="saw now"]], +that one obviously doesn't help anymore if third-party code is using the Hurd's +unfixed header files. + +So I was revisiting this issue. I was already prepared that this would take +some hours, with lots of editing, compiling cycles, plus some analyzing of the +binaries. So I made up a fresh repository for this work. + + $ mkdir hurd-ei + $ cd hurd-ei/ + $ git init + [...] + $ git remote add savannah git://git.savannah.gnu.org/hurd/hurd.git + $ git fetch + [...] + +Switch to a new topic-branch. + + $ git checkout -b master-ei savannah/master + Branch master-ei set up to track remote branch master from savannah. + Switched to a new branch 'master-ei' + +(*`ei`* is short for `extern inline`.) + +The first thing to do was to disable that `-fgnu89-inline` option, so I edited +`Makeconf` where it was added to `CFLAGS`. + +I started editing, compiling, editing, compiling, and so on. + +Finally, the tree was in a shape where everything was building fine and the +resulting libraries contained the symbols they should, etc. + +I committed the whole junk as one *big blob* commit, to store it in a safe +place (you never know with these Hurd machines...), and to continue working on +it the next day. + + $ git commit -a + +For the commit message, I already mostly assembled a `ChangeLog`-style log. +Then: + + $ git format-patch savannah/master.. + 0001-Bla.patch + +... and here is [[0001-Bla.patch.bz2]] (compressed). + + +The next day, a.k.a. today, in a different Git repository. + + $ git checkout -b master-fix_inline savannah/master + Branch master-fix_inline set up to track remote branch master from savannah. + Switched to a new branch 'master-fix_inline' + $ bunzip2 < ../some/where/0001-Bla.patch.bz2 | git am + Applying: Bla. + +The *big blob* is now on top of savannah/master (which was +`2772f5c6a6a51cf946fd95bf6ffe254273157a21`, by the way -- in case that you want +to reproduce this tutorial later, simply substitute `savannah/master` with +`2772...`). + +By then, I had come to the conclusion that the commit essentially was fine, but +should be split into two, and the `configure` hunk shouldn't be in there. So +be it. + +So, the `HEAD` of the active branch is our *big blob* commit that we want to +work on. Check with `git show HEAD`: + + $ git show HEAD + commit 93e97f3351337c349e2926f4041e61bc487ef9e6 + Author: Thomas Schwinge <tschwinge@gnu.org> + Date: Tue Jun 23 00:27:28 2009 +0200 + + Bla. + + * console-client/timer.h (fetch_jiffies): Use static inline instead of extern + inline. + * ext2fs/ext2fs.h (test_bit, set_bit, clear_bit, dino, global_block_modified) + (record_global_poke, sync_global_ptr, record_indir_poke, sync_global) + (alloc_sync): Likewise. + * libftpconn/priv.h (unexpected_reply): Likewise. + * term/term.h (qsize, qavail, clear_queue, dequeue_quote, dequeue) + (enqueue_internal, enqueue, enqueue_quote, unquote_char, char_quoted_p) + (queue_erase): Likewise. + * ufs/ufs.h (dino, indir_block, cg_locate, sync_disk_blocks, sync_dinode) + (swab_short, swab_long, swab_long_long): Likewise. + * term/munge.c (poutput): Use static inline instead of inline. + + * libdiskfs/diskfs.h: Apply inline optimization only ifdef + [__USE_EXTERN_INLINES]. Use __extern_inline instead of extern inline. + * libftpconn/ftpconn.h: Likewise. + * libpipe/pipe.h: Likewise. + * libpipe/pq.h: Likewise. + * libshouldbeinlibc/idvec.h: Likewise. + * libshouldbeinlibc/maptime.h: Likewise. + * libshouldbeinlibc/ugids.h: Likewise. + * libstore/store.h: Likewise. + * libthreads/rwlock.h: Likewise. + * libdiskfs/extern-inline.c: Adapt to these changes. + * libftpconn/xinl.c: Likewise. And don't #include "priv.h". + * libpipe/pipe-funcs.c: Likewise. + * libpipe/pq-funcs.c: Likewise. + * libshouldbeinlibc/maptime-funcs.c: Likewise. And remove superfluous + includes. + * libstore/xinl.c: Likewise. + * libthreads/rwlock.c: Likewise. + + * Makeconf (CFLAGS): Don't append $(gnu89-inline-CFLAGS). + * pfinet/Makefile (CFLAGS): Append $(gnu89-inline-CFLAGS). + + diff --git a/Makeconf b/Makeconf + index e9b2045..236f1ec 100644 + --- a/Makeconf + +++ b/Makeconf + @@ -65,7 +65,7 @@ INCLUDES += -I$(..)include -I$(top_srcdir)/include + CPPFLAGS += $(INCLUDES) \ + -D_GNU_SOURCE -D_IO_MTSAFE_IO -D_FILE_OFFSET_BITS=64 \ + $($*-CPPFLAGS) + -CFLAGS += -std=gnu99 $(gnu89-inline-CFLAGS) -Wall -g -O3 \ + +CFLAGS += -std=gnu99 -Wall -g -O3 \ + [...] + +We want to undo this one commit, but preserve its changes in the working +directory. + + $ git reset HEAD^ + Makeconf: locally modified + configure: locally modified + console-client/timer.h: locally modified + ext2fs/ext2fs.h: locally modified + libdiskfs/diskfs.h: locally modified + libdiskfs/extern-inline.c: locally modified + libftpconn/ftpconn.h: locally modified + libftpconn/priv.h: locally modified + libftpconn/xinl.c: locally modified + libpipe/pipe-funcs.c: locally modified + libpipe/pipe.h: locally modified + libpipe/pq-funcs.c: locally modified + libpipe/pq.h: locally modified + libshouldbeinlibc/idvec.h: locally modified + libshouldbeinlibc/maptime-funcs.c: locally modified + libshouldbeinlibc/maptime.h: locally modified + libshouldbeinlibc/ugids.h: locally modified + libstore/store.h: locally modified + libstore/xinl.c: locally modified + libthreads/rwlock.c: locally modified + libthreads/rwlock.h: locally modified + pfinet/Makefile: locally modified + term/munge.c: locally modified + term/term.h: locally modified + ufs/ufs.h: locally modified + +Now, `HEAD` points to the commit before the previous `HEAD`, i.e. `HEAD^`. +Again, check with `git show HEAD`: + + $ git show HEAD + commit 2772f5c6a6a51cf946fd95bf6ffe254273157a21 + Author: Samuel Thibault <samuel.thibault@ens-lyon.org> + Date: Thu Apr 2 23:06:37 2009 +0000 + + 2009-04-03 Samuel Thibault <samuel.thibault@ens-lyon.org> + + * exec.c (prepare): Call PREPARE_STREAM earlier to permit calling + finish_mapping on E even after errors, as is already done in do_exec. + + diff --git a/exec/ChangeLog b/exec/ChangeLog + index 5a0ad1d..a9300bf 100644 + --- a/exec/ChangeLog + +++ b/exec/ChangeLog + @@ -1,3 +1,8 @@ + +2009-04-03 Samuel Thibault <samuel.thibault@ens-lyon.org> + + + + * exec.c (prepare): Call PREPARE_STREAM earlier to permit calling + + finish_mapping on E even after errors, as is already done in do_exec. + + + 2008-06-10 Samuel Thibault <samuel.thibault@ens-lyon.org> + + * elfcore.c (TIME_VALUE_TO_TIMESPEC): Completely implement instead of + diff --git a/exec/exec.c b/exec/exec.c + index 05dc883..cb3d741 100644 + --- a/exec/exec.c + +++ b/exec/exec.c + @@ -726,6 +726,9 @@ prepare (file_t file, struct execdata *e) + + e->interp.section = NULL; + + + /* Initialize E's stdio stream. */ + + prepare_stream (e); + [...] + +Luckily, Git saves the previous (i.e. before the `git reset`) `HEAD` reference +as `ORIG_HEAD`. Have a look at it with `git show ORIG_HEAD` -- it contains the +*big blob* commit, including the preliminary commit message -- just what HEAD +was before: + + $ git show ORIG_HEAD + commit 93e97f3351337c349e2926f4041e61bc487ef9e6 + Author: Thomas Schwinge <tschwinge@gnu.org> + Date: Tue Jun 23 00:27:28 2009 +0200 + + Bla. + + * console-client/timer.h (fetch_jiffies): Use static inline instead of extern + inline. + [...] + + diff --git a/Makeconf b/Makeconf + index e9b2045..236f1ec 100644 + --- a/Makeconf + +++ b/Makeconf + @@ -65,7 +65,7 @@ INCLUDES += -I$(..)include -I$(top_srcdir)/include + CPPFLAGS += $(INCLUDES) \ + -D_GNU_SOURCE -D_IO_MTSAFE_IO -D_FILE_OFFSET_BITS=64 \ + $($*-CPPFLAGS) + -CFLAGS += -std=gnu99 $(gnu89-inline-CFLAGS) -Wall -g -O3 \ + +CFLAGS += -std=gnu99 -Wall -g -O3 \ + [...] + +OK, now let's pick the files that we want to have in the first of the +envisioned two commits: these are the *static inline instead of extern inline* +and *apply inline optimization only...* sections. + + $ git add console-client/timer.h ext2fs/ext2fs.h [...] libthreads/rwlock.c + +Oh, we forgot something: now that we're preparing this stuff to go into the +*master* repository, update the copyright years. Edit, edit, edit, and then, +again: + + $ git add console-client/timer.h ext2fs/ext2fs.h [...] libthreads/rwlock.c + +Now Git's staging area contains the changes that we want to commit (and the +working directory contains the rest of the *big blob*). Commit these `add`ed +files, and use *big blob*'s commit message as a template for the new one, as it +already contains most of what we want (don't forget to chop off the unneeded +parts). + + $ git commit -c ORIG_HEAD + Waiting for Emacs... + [master-fix_inline 51c15bc] Use static inline where appropriate. + 6 files changed, 50 insertions(+), 51 deletions(-) + $ git show HEAD + commit c6c9d7a69dea26e04bba7010582e7bcd612e710c + Author: Thomas Schwinge <tschwinge@gnu.org> + Date: Tue Jun 23 00:27:28 2009 +0200 + + Use static inline where appropriate and use glibc's __extern_inline machinery. + + * console-client/timer.h (fetch_jiffies): Use static inline instead of extern + inline. + * ext2fs/ext2fs.h (test_bit, set_bit, clear_bit, dino, global_block_modified) + (record_global_poke, sync_global_ptr, record_indir_poke, sync_global) + (alloc_sync): Likewise. + * libftpconn/priv.h (unexpected_reply): Likewise. + * term/term.h (qsize, qavail, clear_queue, dequeue_quote, dequeue) + (enqueue_internal, enqueue, enqueue_quote, unquote_char, char_quoted_p) + (queue_erase): Likewise. + * ufs/ufs.h (dino, indir_block, cg_locate, sync_disk_blocks, sync_dinode) + (swab_short, swab_long, swab_long_long): Likewise. + * term/munge.c (poutput): Use static inline instead of inline. + + * libdiskfs/diskfs.h: Apply inline optimization only ifdef + [__USE_EXTERN_INLINES]. Use __extern_inline instead of extern inline. + * libftpconn/ftpconn.h: Likewise. + * libpipe/pipe.h: Likewise. + * libpipe/pq.h: Likewise. + * libshouldbeinlibc/idvec.h: Likewise. + * libshouldbeinlibc/maptime.h: Likewise. + * libshouldbeinlibc/ugids.h: Likewise. + * libstore/store.h: Likewise. + * libthreads/rwlock.h: Likewise. + * libdiskfs/extern-inline.c: Adapt to these changes. + * libftpconn/xinl.c: Likewise. And don't #include "priv.h". + * libpipe/pipe-funcs.c: Likewise. + * libpipe/pq-funcs.c: Likewise. + * libshouldbeinlibc/maptime-funcs.c: Likewise. And remove superfluous + includes. + * libstore/xinl.c: Likewise. + * libthreads/rwlock.c: Likewise. + + diff --git a/console-client/timer.h b/console-client/timer.h + index 4204192..5e64e97 100644 + --- a/console-client/timer.h + +++ b/console-client/timer.h + @@ -1,5 +1,7 @@ + /* timer.h - Interface to a timer module for Mach. + - Copyright (C) 1995,96,2000,02 Free Software Foundation, Inc. + + + + Copyright (C) 1995, 1996, 2000, 2002, 2009 Free Software Foundation, Inc. + + + Written by Michael I. Bushnell, p/BSG and Marcus Brinkmann. + + This file is part of the GNU Hurd. + @@ -54,7 +56,7 @@ int timer_remove (struct timer_list *timer); + /* Change the expiration time of the timer TIMER to EXPIRES. */ + void timer_change (struct timer_list *timer, long long expires); + + -extern inline long long + +static inline long long + [...] + +As you can see, `HEAD` now points to the new commit on top of the current +branch. (`ORIG_HEAD` doesn't change.) + +On to the next, and last one, only two changes should be left: the `Makeconf` +and `pfinet/Makefile` ones. + + $ git status + # On branch master-fix_inline + # Your branch is ahead of 'savannah/master' by 1 commit. + # + # Changed but not updated: + # (use "git add <file>..." to update what will be committed) + # (use "git checkout -- <file>..." to discard changes in working directory) + # + # modified: Makeconf + # modified: configure + # modified: pfinet/Makefile + # + # Untracked files: + # (use "git add <file>..." to include in what will be committed) + # + # 0001-Bla.patch + # autom4te.cache/ + # hurd_extern_inline_fix.patch?file_id=18191 + no changes added to commit (use "git add" and/or "git commit -a") + +Alright, there is as well still the `configure` hunk that we want to get rid +of. But first for the real second commit, after editing for again adding the +copyright year update: + + $ git add Makeconf pfinet/Makefile + $ git commit -c ORIG_HEAD + Waiting for Emacs... + [master-fix_inline 6a967d1] We're now C99 inline safe -- apart from the Linux code in pfinet. + 2 files changed, 6 insertions(+), 3 deletions(-) + +Check that we're in a clean state now: + + $ git status + # On branch master-fix_inline + # Your branch is ahead of 'savannah/master' by 2 commits. + # + # Changed but not updated: + # (use "git add <file>..." to update what will be committed) + # (use "git checkout -- <file>..." to discard changes in working directory) + # + # modified: configure + # + # Untracked files: + # (use "git add <file>..." to include in what will be committed) + # + # 0001-Bla.patch + # autom4te.cache/ + # hurd_extern_inline_fix.patch?file_id=18191 + no changes added to commit (use "git add" and/or "git commit -a") + +Oops, we forgot something... + + $ git checkout -- configure + +Now, our tree is clean again. (Check with `git status`.) + +By now, we came to the conclusion that the first of the two commits should have +been further split into two separate ones. Of course, essentially we would do +the same splitting again that we've done just now -- but how to easily modify +the first commit, now that we have another one on top of it? + +Alright, `git rebase --interactive` to the rescue -- let's interactively +*`rebase`* the last two commits, to modify them as wanted. + + $ git rebase --interactive HEAD~2 + Waiting for Emacs... + +Emacs wants us to tell which commits we want to keep as they are (`pick`), +which should be merged into others (`squash`), and which we want to `edit`. In +our scenario, we want to `edit` the first one and `pick` the second one. +Change the file thusly and close it. + + Stopped at 5becbb5... Use static inline where appropriate and use... + You can amend the commit now, with + + git commit --amend + + Once you are satisfied with your changes, run + + git rebase --continue + +We want to undo this first commit to split it into two. Again, use `git reset` +for that, while preserving the commit's changes in the working directory. + + $ git reset HEAD^ + console-client/timer.h: locally modified + [...] + +Pick the set of files that we want to have in the first of the envisioned two +commits: the *static inline instead of extern inline* section, and commit them, +again using the previous commit message as a template for the new one: + + $ git add console-client/timer.h ext2fs/ext2fs.h [...] term/munge.c + $ git commit -c ORIG_HEAD + Waiting for Emacs... + [detached HEAD 51c15bc] Use static inline where appropriate. + 6 files changed, 50 insertions(+), 51 deletions(-) + +Next part: *apply inline optimization only...*. Again, `git add` those files +that shall be part of the next commit, i.e. all remaining ones. As before, use +the previous commit message as a template. + + $ git add libdiskfs/diskfs.h [...] libthreads/rwlock.c + $ git commit -c ORIG_HEAD + Waiting for Emacs... + [detached HEAD 8ac30ea] [__USE_EXTERN_INLINES]: Use glibc's __extern_inline machinery. + 16 files changed, 508 insertions(+), 356 deletions(-) + +Now we're done with splitting that commit into two. (Check with `git status` +that we didn't forget anything.) What's missing is getting back the other +commit on top of the two now-split ones: + + $ git rebase --continue + Successfully rebased and updated refs/heads/master-fix_inline. + +Here we go. The other commit has been applied on top of the two new ones. + +Due to time-honored tradition, I always double-check what I have just +committed, before distributing it to the world: + + $ git log --reverse -p -C --cc savannah/master.. + +... and promptly, I recognize some changes that shouldn't be in there: when +using it on some files, Emacs' `copyright-fix-years`, aside from indeed fixing +the list of copyright years, and adding the current year, also changed *GPL +... version 2* into *version 3*, which would be nice, but which we can't do for +the moment. The error is present only in the first and second commit. If it +were in only in the third (the last) one, simply editing the files, and then +using `git commit --amend` would be the solution. But again there is the +problem about how to modify the first (`HEAD~2`) and second (`HEAD~1`, or +`HEAD^`) commit now that there is another one on top of it. By now, we know +the solution: + + $ git rebase --interactive HEAD~3 + Waiting for Emacs... + +This time, we need to `edit` the first and second commits, and `pick` the third +one. + + Stopped at ffd215b... Use static inline where appropriate. + You can amend the commit now, with + + git commit --amend + + Once you are satisfied with your changes, run + + git rebase --continue + +`git show` (which defaults to showing `HEAD`, by the way) can again be used to +have a look at the current `HEAD` (which is the first of the three commits), +and then we revert the unwanted changes in the editor, resulting with the +following changed files: + + $ git status + # Not currently on any branch. + # Changed but not updated: + # (use "git add <file>..." to update what will be committed) + # (use "git checkout -- <file>..." to discard changes in working directory) + # + # modified: ext2fs/ext2fs.h + # modified: libftpconn/priv.h + # modified: term/munge.c + # modified: term/term.h + # modified: ufs/ufs.h + # + # Untracked files: + # (use "git add <file>..." to include in what will be committed) + # + # 0001-Bla.patch + # autom4te.cache/ + # hurd_extern_inline_fix.patch?file_id=18191 + no changes added to commit (use "git add" and/or "git commit -a") + +Then, we can -- as `git rebase` suggested above -- *amend* the existing `HEAD` +commit with these changes (`--amend` and `--all`), reusing `HEAD`'s commit +message without spawning an editor (`-C HEAD`): + + $ git commit --amend -C HEAD --all + [detached HEAD c6c9d7a] Use static inline where appropriate. + 6 files changed, 45 insertions(+), 46 deletions(-) + +Continue with the next commit: + + $ git rebase --continue + Stopped at 8ac30ea... [__USE_EXTERN_INLINES]: Use glibc's __extern_inline machinery. + You can amend the commit now, with + + git commit --amend + + Once you are satisfied with your changes, run + + git rebase --continue + +Again, have a look at the commit (`git show`), revert the unwanted changes, +*amend* `HEAD`, and continue to the next commit: + + $ git commit --amend -C HEAD --all + [detached HEAD 9990dc6] [__USE_EXTERN_INLINES]: Use glibc's __extern_inline machinery. + 16 files changed, 500 insertions(+), 348 deletions(-) + $ git rebase --continue + Stopped at 6a967d1... We're now C99 inline safe -- apart from the Linux code in pfinet. + You can amend the commit now, with + + git commit --amend + + Once you are satisfied with your changes, run + + git rebase --continue + +Two files are left to be edited (`git show`, etc., again), and finally: + + $ git commit --amend -C HEAD --all + [detached HEAD 241c605] We're now C99 inline safe -- apart from the Linux code in pfinet. + 2 files changed, 5 insertions(+), 2 deletions(-) + $ git rebase --continue + Successfully rebased and updated refs/heads/master-fix_inline. + +That's it. `git log --reverse -p -C --cc savannah/master..` now looks as nice +as can be. + + +Of course, this is only a small insight of what is possible to do with `git +rebase` and friends -- see the manual for further explanations. diff --git a/community/weblogs/tschwinge/2009-06-23_split_patch_and_git_rebase_--interactive/0001-Bla.patch.bz2 b/community/weblogs/tschwinge/2009-06-23_split_patch_and_git_rebase_--interactive/0001-Bla.patch.bz2 Binary files differnew file mode 100644 index 00000000..4a682c86 --- /dev/null +++ b/community/weblogs/tschwinge/2009-06-23_split_patch_and_git_rebase_--interactive/0001-Bla.patch.bz2 |