Message ID | 1326147171-13752-1-git-send-email-arnd@arndb.de |
---|---|
State | New |
Headers | show |
On Mon, Jan 9, 2012 at 2:12 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > 58 files changed, 1967 insertions(+), 818 deletions(-) Hmm. You don't seem to use the "detect renames" flag -M when you create these, so the end result doesn't match mine. With -M, it looks like this: 54 files changed, 1399 insertions(+), 250 deletions(-) because it sees these: rename arch/arm/mach-tegra/{board-dt.c => board-dt-tegra20.c} (91%) rename arch/arm/mach-tegra/include/mach/{pinmux-t2.h => pinmux-tegra20.h} (96%) rename arch/arm/mach-tegra/{pinmux-t2-tables.c => pinmux-tegra20-tables.c} (95%) rename arch/arm/plat-omap/include/plat/{ti816x.h => ti81xx.h} (60%) Please do use -M, because it ends up making diffstats *so* much more readable when there are renames (instead of big delete-create diffs, you get a small diffstat that actually is much more relevant to what happened). Linus
On Mon, Jan 9, 2012 at 2:12 PM, Arnd Bergmann <arnd@arndb.de> wrote: > > For each branch "next/foo", there is also a signed tag "foo" on the > arm-soc git. Please pull whichever of these you prefer. So I pulled the tags, because not only do I then get the signature, but the merge message will automatically also end up containing your notes that you put in the tags. Which is a nice feature worth pointing out to people: when you send me pull-requests, you can add commentary that actually gets saved in the tag. So to give an example from Arnd's series, look at commit e8cbce976050 in the current -git tree (just pushed out, since I did the merges just moments ago): [torvalds@i5 linux]$ git show e8cbce976050 commit e8cbce976050a9f874a8b07012ddeb9b9eb59603 Merge: b3c37522928b 27fdb577435e Author: Linus Torvalds <torvalds@linux-foundation.org> Date: Mon Jan 9 14:40:48 2012 -0800 Merge tag 'timer' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc timer changes for msm A very simple series. We used to have more churn in the timer area, so this is kept separate. Will probably put this into the drivers series next time. * tag 'timer' of git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc: msm: timer: Use clockevents_config_and_register() msm: timer: Setup interrupt after registering clockevent msm: timer: Remove SoC specific #ifdefs msm: timer: Remove msm_clocks[] and simplify code msm: timer: Fix ONESHOT mode interrupts msm: timer: Use GPT for clockevents and DGT for clocksource msm: timer: Cleanup #includes and #defines msm: timer: Tighten #ifdef for local timer support where those "timer changes for msm" and the comment about a simple series were all from Arnd's tag. Now, you don't *have* to do these kinds of descriptions, and I do end up editing them if they look odd (eg Arnd saying "I" when the commit then ends up being mine), but I wanted to point it out because some submaintainers may end up enjoying the capability to add notes to the merge to explain things better. And Arnd, please do double-check that I didn't mess up any of the (pretty trivial) merge conflicts.. Linus
On Mon, Jan 09, 2012 at 02:37:17PM -0800, Linus Torvalds wrote: > Please do use -M, because it ends up making diffstats *so* much more > readable when there are renames (instead of big delete-create diffs, > you get a small diffstat that actually is much more relevant to what > happened). What about using -C instead (which implies -M, but also detects copies) ?
On Tue, Jan 10, 2012 at 12:39 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > What about using -C instead (which implies -M, but also detects copies) ? I don't mind -C. It sometimes causes differences to what I see by default, but those differences are often interesting. But while it's interesting and relevant (unlike the non-rename patch that is just noisy), it also can hide lots of lines. With -C, you can get a diffstat that is actually fairly small, but that adds a lot of lines to the kernel (because somebody just copied large files with small changes), and if that happens I do want to see it as a "big change". So for me, the plain "just show renames" is a good default. So there is absolutely nothing wrong with -C. It's not what I use, but when I see that the diffstats don't match, it's easy to notice why, and that information is often fairly interesting, so I don't mind. Linus
On Wednesday 11 January 2012, Junio C Hamano wrote: > > Linus Torvalds <torvalds@linux-foundation.org> writes: > > > On Tue, Jan 10, 2012 at 12:39 AM, Russell King - ARM Linux > > <linux@arm.linux.org.uk> wrote: > >> > >> What about using -C instead (which implies -M, but also detects copies) ? > > > > I don't mind -C. It sometimes causes differences to what I see by > > default, but those differences are often interesting. > > > > But while it's interesting and relevant (unlike the non-rename patch > > that is just noisy), it also can hide lots of lines. With -C, you can > > get a diffstat that is actually fairly small, but that adds a lot of > > lines to the kernel (because somebody just copied large files with > > small changes), and if that happens I do want to see it as a "big > > change". So for me, the plain "just show renames" is a good default. > > > > So there is absolutely nothing wrong with -C. It's not what I use, but > > when I see that the diffstats don't match, it's easy to notice why, > > and that information is often fairly interesting, so I don't mind. > > How about encouraging people to use stock "git request-pull" instead? > > Then best/better practices can be captured as improvement patches to it, > instead of being spread as updates to many people's homebrew scripts, no? I keep having trouble in multiple ways with git request-pull, so I ended up running it as part of a script on fictional commits, removing the diffstat output, adding a new diffstat output that actually refered to the contents I was sending (though forgetting to use -M, as Linus noted) and adding the necessary headers that let me send out the series of pull requests using git send-email. I think that request-pull is fixable, but I'm not completely sure if I'm misunderstanding something myself. To give a realistic example, lets assume I have three branches I want to send out as individual pull requests. Each of these branch is the result of merging multiple of my downstream branches: * Branch A (fixes) contains a bunch of bug fixes collected from various people that have sent me a pull request. Some of these are based on v3.3-rc2, others are based on v3.3-rc3 or v3.3-rc4, so the resulting tree is based on v3.3-rc4. * Branch B (cleanups) contains cleanup patches from multiple sources. Some of the branches I pulled in here also contain commits that I have on the fixes branches from the same people, but most don't. The latest branch I have pulled in is based on v3.3-rc5, so this has a newer base than Branch A, but is not a complete superset of it. * Branch C (features) contains new stuff from various people, some of whom are basing them on top of branches that went into A and/or B. It also contains v3.3-rc4 but no later upstream code. However, one of the branches has a dependency on an external tree (typically Russell's devel-stable branch), so that gets pulled into C and I would wait for the dependency to get upstream before submitting C. Now let's assume that all dependencies are merged upstream already and I just want to send out three pull requests. The first pull request generally works fine, though it could be that some version of git in the past would include the diff between v3.3-rc2 and v3.3-rc4 in the diffstat. For the second pull request, I merge v3.3 with A and send do 'git request-pull B origin tmp-merge-of-upstream-and-A'. This seems to generate the correct list of patches, but the wrong diffstat (diffstat also contains the diff between v3.3-rc4 and v3.3-rc5, although that is indeed part of tmp-merge-of-upstream-and-A. For submitting branch C, I have to merge upstream, A, B and the dependencies together and then send a pull request against that. This typically also includes the external dependencies in the diffstat. Arnd
On Wed, Jan 11, 2012 at 19:12, Arnd Bergmann <arnd@arndb.de> wrote: [...] > Now let's assume that all dependencies are merged upstream already > and I just want to send out three pull requests. The first > pull request generally works fine, though it could be that some > version of git in the past would include the diff between v3.3-rc2 > and v3.3-rc4 in the diffstat. For the second pull request, I merge > v3.3 with A and send do 'git request-pull B origin > tmp-merge-of-upstream-and-A'. This seems to generate the correct > list of patches, but the wrong diffstat (diffstat also contains > the diff between v3.3-rc4 and v3.3-rc5, although that is indeed > part of tmp-merge-of-upstream-and-A. For submitting branch C, > I have to merge upstream, A, B and the dependencies together > and then send a pull request against that. This typically also > includes the external dependencies in the diffstat. <throwing the bat in the hen house, no idea if this expression exists in other languages than Dutch> And all of this would look nice if you would have done a rebase on top of the latest tagged version of Linus' tree that contains all prerequisites, right? </throwing...> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, 11 Jan 2012, Geert Uytterhoeven wrote: > On Wed, Jan 11, 2012 at 19:12, Arnd Bergmann <arnd@arndb.de> wrote: > > [...] > > > Now let's assume that all dependencies are merged upstream already > > and I just want to send out three pull requests. The first > > pull request generally works fine, though it could be that some > > version of git in the past would include the diff between v3.3-rc2 > > and v3.3-rc4 in the diffstat. For the second pull request, I merge > > v3.3 with A and send do 'git request-pull B origin > > tmp-merge-of-upstream-and-A'. This seems to generate the correct > > list of patches, but the wrong diffstat (diffstat also contains > > the diff between v3.3-rc4 and v3.3-rc5, although that is indeed > > part of tmp-merge-of-upstream-and-A. For submitting branch C, > > I have to merge upstream, A, B and the dependencies together > > and then send a pull request against that. This typically also > > includes the external dependencies in the diffstat. > > <throwing the bat in the hen house, no idea if this expression exists > in other languages than Dutch> > And all of this would look nice if you would have done a rebase on top of the > latest tagged version of Linus' tree that contains all prerequisites, right? > </throwing...> No, because as a maintainer merging other people's branches, you're not supposed to rebase anything. Linus did flame maintainers doing that in the past. The reason is that by rebasing you modify the environment in which the changes are applied which voids the testing that the original author did. Nicolas
On Wed, Jan 11, 2012 at 12:29 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > And all of this would look nice if you would have done a rebase on top of the > latest tagged version of Linus' tree that contains all prerequisites, right? Rebasing means that nobody else can depend on or work with that tree, so it's a no-no. Sure, it works if you are the only person touching it, but then you had better not export it at all, so what's the point? We have had independent problems in another branch exactly because it was rebased and people merged it, so bringing up rebasing as a "solution" is wrong-headed. It just causes *more* problems of other kinds, even if it may make git request-pull trivial. Linus
On Thu, Jan 12, 2012 at 00:21, Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Wed, Jan 11, 2012 at 12:29 PM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: >> >> And all of this would look nice if you would have done a rebase on top of the >> latest tagged version of Linus' tree that contains all prerequisites, right? > > Rebasing means that nobody else can depend on or work with that tree, > so it's a no-no. > > Sure, it works if you are the only person touching it, but then you > had better not export it at all, so what's the point? That's why you need two branches: 1. a non-rebasing one for development, 2. a rebasing one containing cherry-picked (possibly folded) commits for preparing for upstream delivery. Both branches contain an identical source tree at all times, but they contain different commits. If you make a merge error in the first, or a rebase error in the second, you will notice as they will differ. The non-rebasing branch should be used by your (sub)lieutenants to base their work on. The rebasing branch is used for deliveries upstream. `for-next` and `for-linus` are subsets of it. Patches emailed out for review can/should come from this branch (appying patches is also a form of rebasing). > We have had independent problems in another branch exactly because it > was rebased and people merged it, so bringing up rebasing as a > "solution" is wrong-headed. It just causes *more* problems of other > kinds, even if it may make git request-pull trivial. Other people are supposed to merge the non-rebasing branch only. Just like you (as in "everyone except Linus") only merge in your non-rebasing branch. What I like (the most?) about git is that it tracks automatically what commits in my rebasing branch have been applied upstream. If you pull from your sub-lieutenants instead of applying patches, or if you have multiple upstreams, it becomes more complicated, but I think git rebase can still handle it. Examples: http://git.kernel.org/?p=linux/kernel/git/geert/linux-m68k.git;a=shortlog;h=refs/heads/master (non-rebasing) http://git.kernel.org/?p=linux/kernel/git/geert/linux-m68k.git;a=shortlog;h=refs/heads/m68k-queue (rebasing) Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Jan 11, 2012 at 10:32 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > That's why you need two branches: > 1. a non-rebasing one for development, > 2. a rebasing one containing cherry-picked (possibly folded) commits for > preparing for upstream delivery. Stop being a moron. Go back and read the "nobody can work with you". If you rebase for upstream delivery, then that means that everybody that works with you are workign with a tree that isn't ready for delivery, and that they cannot rely on. Just don't do it. If your tree is so ugly that you can't deliver it upstream, then don't deliver it sideways or downstream either. Keep it in your own pants, and don't make it public at all. Since nobody can trust it anyway, and since it isn't the final end result, why even bother? They can't rely on it, they can't work with it. So here's the *real* solution: - make sure your development tree is in good enough shape that you can make it public, and can ask me to pull it. It really is that simple. If you don't think it's in good enough shape, don't make it public. Linus