Re: [git patches] libata updates
On Wed, Jul 25, 2012 at 7:10 PM, Jeff Garzik wrote: > > Thanks, so noted. I guess if the merge gets more complex than something > easily described in an email, that implies that maintainers should do more > cross-coordination and maybe a merge tree. It's fairly rare. It happens mostly with the arch trees for some reason - the ARM tree used to be an unholy mess. And then we have the small "oops, we didn't even notice" things when some driver (or FS) interface changes, and we have a new driver/fs or just extended an old one, and there's a subtle conflict. And people miss those, and quite frankly, it's not a huge deal. We can fix it up later. It's the "oh, I knew about this" cases where it's fixed up as a separate commit I dislike. And quite frankly, I really do a lot of merges, and over the history of git we have not had all that many really complicated ones. I commonly send people email saying "ok, this clashed, you need to double-check my merge", but it's not common that they are really problems. So to a first approximation, I'd actually prefer that submaintainers not care *at*all* about whether something clashes upstream or not. If there are consistent and problematic clashes, that implies some deeper problem, and the solution to that is not "let's pre-merge", but rather more along the lines of "we're doing something wrong, maybe our interfaces are crap, or our modularity is wrong, let's think about it". And for subsystems where we have had problems, it's actually really nice if the maintainer sends me the unmerged "this is the work I've done" tree, and then perhaps has a separate "xyzzy-merged" branch that has a pre-merged version. For cases like that, I will do the merge myself, but I'll actually double-check my merge against the maintainer merge. And it's happened more than once that my merge has differed, and _my_ merge is the correct one. The maintainer may know his code better, but I know my merging. I do a ton of them. For example, this week I've done 66 merges, and 15 of them had conflicts. Of the 15, only two were interesting iirc, but even those weren't really "problematic", they were just enough to trigger me to send out emails to the maintainers about them. And I don't think your libata merge would really have merited even that, apart from the small semantic thing (which would also have been trivial with a oneliner "heads up, check out the semantic change in xyz.c:abcdef()". > What's the best way for libata to move forward, now that this hideous merge > has been pushed out to the Well Known libata branches? The > pre-jgarzik-merge commit you would have pulled is > dc7f71f486f4f5fa96f6dcf86833da020cde8a11 had my pull request been proper. I'll take your merge, it's not like it's a huge problem. What I care most about is the "flow", and I am making a stink just so that this doesn't become a common issue. We have tons of ugly history, and I'm not black-and-white - problems happen. Big deal. I just want to make sure that they don't become systemic. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On Thu, 26 Jul 2012, Aaron Lu wrote: > On 07/26/2012 01:05 PM, James Bottomley wrote: > > On Thu, 2012-07-26 at 12:47 +0800, Aaron Lu wrote: > >> On 07/26/2012 05:38 AM, Jeff Garzik wrote: > >>> On 07/25/2012 04:35 PM, Jeff Garzik wrote: > * Updating libata to directly bind with ACPI / runtime power mgmt. > This is a pre-req for SATA ZPODD (CD-ROM power management). > > Touches ACPI (exports++) and SCSI in minor ways. Has been in linux-next > for weeks. > > The rest of [ZPODD] will probably come via SCSI tree, as it involves > a lot of updates to the 'sr' driver etc. > >>> > >>> BTW Lin and Aaron, note that this did not include these changes: > >>> > >>> sr: check support for device busy class events > >>> sr: support zero power ODD > >>> sr: make sure ODD is in resumed state in block ioctl > >>> > >>> as in the end I wanted to put the brakes on SCSI-touching patches. These > >>> should be able to go into James' scsi-misc tree with the other SCSI-area > >>> ZPODD changes. > >>> > >>> For those three 'sr' changes listed above, you may add > >>> > >>> Acked-by: Jeff Garzik > >>> > >>> when moving them over. > >> > >> Thanks Jeff. > >> > >> Hi James, > >> I'll prepare these dropped patches plus some other fixes for ZPODD which > >> I've sent v2 recently and merge them into v3 for you to review. > > > > They weren't exactly dropped ... I've been waiting for you to address > > Alan Stern's comments, since he's our resident expert on suspend/resume. > > Oh, I forgot to mention, that I agree with Alan's comments and have > addressed them in my v2 patches here: > http://marc.info/?l=linux-scsi&m=134312317325650&w=2 > > The 2 patches Alan has comments are: > http://marc.info/?l=linux-scsi&m=134312311025619&w=2 > http://marc.info/?l=linux-scsi&m=134312308225610&w=2 > > Hi Alan, > Are the v2 patches look OK to you? Yes, they are better now. Acked-by: Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
* Linus Torvalds wrote: > I couldn't find an example of that in a quick look, it's > fairly uncommon to have non-conflicting merges that had > semantic - but not contextual - conflicts. [...] This: git log --grep='Semantic merge\|Semantic conflict' gives over a dozen examples of such semantic merges I've done in the past 4 years. I fully agree that they are best done in the merge commit - in hindsight perhaps with a tad more explanation than I did. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On 07/26/2012 01:05 PM, James Bottomley wrote: On Thu, 2012-07-26 at 12:47 +0800, Aaron Lu wrote: On 07/26/2012 05:38 AM, Jeff Garzik wrote: On 07/25/2012 04:35 PM, Jeff Garzik wrote: * Updating libata to directly bind with ACPI / runtime power mgmt. This is a pre-req for SATA ZPODD (CD-ROM power management). Touches ACPI (exports++) and SCSI in minor ways. Has been in linux-next for weeks. The rest of [ZPODD] will probably come via SCSI tree, as it involves a lot of updates to the 'sr' driver etc. BTW Lin and Aaron, note that this did not include these changes: sr: check support for device busy class events sr: support zero power ODD sr: make sure ODD is in resumed state in block ioctl as in the end I wanted to put the brakes on SCSI-touching patches. These should be able to go into James' scsi-misc tree with the other SCSI-area ZPODD changes. For those three 'sr' changes listed above, you may add Acked-by: Jeff Garzik when moving them over. Thanks Jeff. Hi James, I'll prepare these dropped patches plus some other fixes for ZPODD which I've sent v2 recently and merge them into v3 for you to review. They weren't exactly dropped ... I've been waiting for you to address Alan Stern's comments, since he's our resident expert on suspend/resume. Oh, I forgot to mention, that I agree with Alan's comments and have addressed them in my v2 patches here: http://marc.info/?l=linux-scsi&m=134312317325650&w=2 The 2 patches Alan has comments are: http://marc.info/?l=linux-scsi&m=134312311025619&w=2 http://marc.info/?l=linux-scsi&m=134312308225610&w=2 Hi Alan, Are the v2 patches look OK to you? And James, Do you want me to rebase these patches on top of scsi-misc tree? Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On Thu, 2012-07-26 at 12:47 +0800, Aaron Lu wrote: > On 07/26/2012 05:38 AM, Jeff Garzik wrote: > > On 07/25/2012 04:35 PM, Jeff Garzik wrote: > >> * Updating libata to directly bind with ACPI / runtime power mgmt. > >> This is a pre-req for SATA ZPODD (CD-ROM power management). > >> > >> Touches ACPI (exports++) and SCSI in minor ways. Has been in linux-next > >> for weeks. > >> > >> The rest of [ZPODD] will probably come via SCSI tree, as it involves > >> a lot of updates to the 'sr' driver etc. > > > > BTW Lin and Aaron, note that this did not include these changes: > > > > sr: check support for device busy class events > > sr: support zero power ODD > > sr: make sure ODD is in resumed state in block ioctl > > > > as in the end I wanted to put the brakes on SCSI-touching patches. These > > should be able to go into James' scsi-misc tree with the other SCSI-area > > ZPODD changes. > > > > For those three 'sr' changes listed above, you may add > > > > Acked-by: Jeff Garzik > > > > when moving them over. > > Thanks Jeff. > > Hi James, > I'll prepare these dropped patches plus some other fixes for ZPODD which > I've sent v2 recently and merge them into v3 for you to review. They weren't exactly dropped ... I've been waiting for you to address Alan Stern's comments, since he's our resident expert on suspend/resume. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On 07/26/2012 05:38 AM, Jeff Garzik wrote: On 07/25/2012 04:35 PM, Jeff Garzik wrote: * Updating libata to directly bind with ACPI / runtime power mgmt. This is a pre-req for SATA ZPODD (CD-ROM power management). Touches ACPI (exports++) and SCSI in minor ways. Has been in linux-next for weeks. The rest of [ZPODD] will probably come via SCSI tree, as it involves a lot of updates to the 'sr' driver etc. BTW Lin and Aaron, note that this did not include these changes: sr: check support for device busy class events sr: support zero power ODD sr: make sure ODD is in resumed state in block ioctl as in the end I wanted to put the brakes on SCSI-touching patches. These should be able to go into James' scsi-misc tree with the other SCSI-area ZPODD changes. For those three 'sr' changes listed above, you may add Acked-by: Jeff Garzik when moving them over. Thanks Jeff. Hi James, I'll prepare these dropped patches plus some other fixes for ZPODD which I've sent v2 recently and merge them into v3 for you to review. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On 07/25/2012 07:30 PM, Linus Torvalds wrote: On Wed, Jul 25, 2012 at 3:58 PM, Jeff Garzik wrote: What is the right course in when a post-merge change is needed? Just describe the issue and the required change. Than I can just do it as part of the merge, and now the whole series is bisectable, including the merge itself. Here's a (fairly bad) example: http://www.spinics.net/lists/netdev/msg192349.html and the reason I call that a bad example is not because that's a bad pull request, but simply that those are all real data conflicts, not the more subtle kind of "it merges fine, but because new code introduced uses an interface that changed, you need to do xyz". Thanks, so noted. I guess if the merge gets more complex than something easily described in an email, that implies that maintainers should do more cross-coordination and maybe a merge tree. What's the best way for libata to move forward, now that this hideous merge has been pushed out to the Well Known libata branches? The pre-jgarzik-merge commit you would have pulled is dc7f71f486f4f5fa96f6dcf86833da020cde8a11 had my pull request been proper. I can lop off the top 3 commits and force-update the libata-dev.git branches, then send a new pull request -- but you have grumbled at that sort of behavior in maintainer trees before too... Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On Wed, Jul 25, 2012 at 3:58 PM, Jeff Garzik wrote: > > What is the right course in when a post-merge change is needed? Just describe the issue and the required change. Than I can just do it as part of the merge, and now the whole series is bisectable, including the merge itself. Here's a (fairly bad) example: http://www.spinics.net/lists/netdev/msg192349.html and the reason I call that a bad example is not because that's a bad pull request, but simply that those are all real data conflicts, not the more subtle kind of "it merges fine, but because new code introduced uses an interface that changed, you need to do xyz". I couldn't find an example of that in a quick look, it's fairly uncommon to have non-conflicting merges that had semantic - but not contextual - conflicts. (Although it does happen, and sometimes it's actually not the developer, but Stephen Rothwell who notices it in -next and lets me know before the merge). Linus -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On 07/25/2012 06:31 PM, Linus Torvalds wrote: On Wed, Jul 25, 2012 at 3:26 PM, Jeff Garzik wrote: Even so, separately, it still needed that post-merge compile fix. And that's yet another example of how *NOT* to do things. If the merge has errors like that, then they should be fixed up in the merge. Please. Don't do this. Let me merge stuff, and you explain in the pull request why it gets merge problems. Not this mess. That merge itself was *trivial*. I do those kinds of fixups in my sleep and you don't even need to explain those. The non-trivial part you did as a separate commit. But neither of those should have been "I'll pre-merge for Linus so that he doesn't see these problems". What is the right course in when a post-merge change is needed? Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On Wed, Jul 25, 2012 at 3:26 PM, Jeff Garzik wrote: > > Even so, separately, it still needed that post-merge compile fix. And that's yet another example of how *NOT* to do things. If the merge has errors like that, then they should be fixed up in the merge. Please. Don't do this. Let me merge stuff, and you explain in the pull request why it gets merge problems. Not this mess. That merge itself was *trivial*. I do those kinds of fixups in my sleep and you don't even need to explain those. The non-trivial part you did as a separate commit. But neither of those should have been "I'll pre-merge for Linus so that he doesn't see these problems". Linus -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On 07/25/2012 06:06 PM, Linus Torvalds wrote: On Wed, Jul 25, 2012 at 1:43 PM, Jeff Garzik wrote: On Wed, Jul 25, 2012 at 04:35:51PM -0400, Jeff Garzik wrote: Please pull 641589bff714f39b33ef1d7f02eaa009f2993b64 from git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git tags/upstream Oh, I forgot to point out the merge commit, making my HEAD more recent than might be expected. There was a merge conflict and an API change that needed to be dealt with, in order for your pull to be correct. So I'd *much* rather see an explanation of what the conflict is when you ask me to pull, and let me handle it, rather than you pre-merging things for me. I *want* to see conflicts between subsystems. Seriously. Tried to add some explanation to the merge commit itself, giving plenty of detail. Even so, separately, it still needed that post-merge compile fix. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On Wed, Jul 25, 2012 at 1:43 PM, Jeff Garzik wrote: > On Wed, Jul 25, 2012 at 04:35:51PM -0400, Jeff Garzik wrote: >> Please pull 641589bff714f39b33ef1d7f02eaa009f2993b64 from >> git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git >> tags/upstream >> > > Oh, I forgot to point out the merge commit, making my HEAD more recent > than might be expected. There was a merge conflict and an API change > that needed to be dealt with, in order for your pull to be correct. So I'd *much* rather see an explanation of what the conflict is when you ask me to pull, and let me handle it, rather than you pre-merging things for me. I *want* to see conflicts between subsystems. Seriously. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On 07/25/2012 04:35 PM, Jeff Garzik wrote: Please pull 641589bff714f39b33ef1d7f02eaa009f2993b64 from git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git tags/upstream (text copied from the upstream-linus tag) Notable changes: * Updating libata to directly bind with ACPI / runtime power mgmt. This is a pre-req for SATA ZPODD (CD-ROM power management). Touches ACPI (exports++) and SCSI in minor ways. Has been in linux-next for weeks. The rest of [ZPODD] will probably come via SCSI tree, as it involves a lot of updates to the 'sr' driver etc. BTW Lin and Aaron, note that this did not include these changes: sr: check support for device busy class events sr: support zero power ODD sr: make sure ODD is in resumed state in block ioctl as in the end I wanted to put the brakes on SCSI-touching patches. These should be able to go into James' scsi-misc tree with the other SCSI-area ZPODD changes. For those three 'sr' changes listed above, you may add Acked-by: Jeff Garzik when moving them over. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git patches] libata updates
On Wed, Jul 25, 2012 at 04:35:51PM -0400, Jeff Garzik wrote: > Please pull 641589bff714f39b33ef1d7f02eaa009f2993b64 from > git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git > tags/upstream > Oh, I forgot to point out the merge commit, making my HEAD more recent than might be expected. There was a merge conflict and an API change that needed to be dealt with, in order for your pull to be correct. Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
