Re: co-authoring commits
On Fri, Jun 19, 2015 at 8:18 PM, Jakub Narębski jna...@gmail.com wrote: On 2015-06-18 at 23:25, Tuncer Ayaz wrote: On Thu, Jun 18, 2015 at 12:52 AM, Theodore Ts'o wrote: On Wed, Jun 17, 2015 at 10:26:32PM +0200, Tuncer Ayaz wrote: [...] One could imagine some frankly, quite rare example where there is a team of people who votes on each commit before it gets sent out and where everyone is equal and there is no hierarchy. In that case, perhaps you could set the from field to a mailing list address. But honestly, how often is that *all* of the authors are completely equal[1]? For that case something like patchwork, phabricator, or gerrit seems to be the logical tool to use, and should ideally leave a trace of approvals and such in the resulting commit message(s). If the patch management tool takes care of merging the commit(s), it can be harder to misattribute signed-off/reviewed-by/etc, which is a good thing. Doesn't Gerrit (at least) use trailer-like structured *notes* in the 'reviews' category (i.e. refs/notes/reviews ref) to store information about review process? Don't remember if it does specifically, but I'm sure it can be configured to. I know Phabricator appends a lot upon doing the commit. I'll have to check it the next time I happen to use Gerrit. You could of course use multiple (everybody makes their own) commits, where you risk breaking bisectability and avoid the need for equal co-authorship support. In pair programming such intermediate commits will quite often be fixups, and when you attempt to squash the fixups for bisectability's sake, you may get a desire for co-authorship of the resulting commit. Hmmm... I didn't think about the problem of attributing authorship for squashed commits. Though here multiple 'author' headers, or Strictly speaking, in a live pair programming session usually the one currently typing will be corrected by the other dev, and the roles will switch when the typist changes. multiline 'author' header would be a better match than 'coauthor' header (which itself doesn't need, I think, the date filed, or does it?) What makes sense to me is that the date is decoupled from the list of authors and there is one date only. I think of it as an expanded version of the previously single-entry author field, where the limit has been lifted. With my purely git-user hat on, that is. So, yes, a multiline author header sounds like a solution to me. Maybe it should be sorted alphabetically upon commit regardless of the order of --author ONE --author TWO. [This is sent from Thunderbird news, so it should be all right] This is fine, the other one was broken. Out of curiosity what's the difference between Thunderbird email and news? -- To unsubscribe from this list: send the line unsubscribe git in
Re: co-authoring commits
On Thu, Jun 18, 2015 at 12:52 AM, Theodore Ts'o wrote: On Wed, Jun 17, 2015 at 10:26:32PM +0200, Tuncer Ayaz wrote: By allowing multiple authors, you don't have to decide who's the primary author, as in such situations usually there is no primary at all. I sometimes deliberately override the author when committing and add myself just as another co-author in the commit message, but as others have noted it would be really great if we can just specify multiple authors. Just recently, there a major thread on the IETF mailing list where IETF working group had drafts where people were listed as co-authors without their permission, and were upset that the fact that their name was added made it seem as if they agreed with the end product. (i.e., that they were endorsing the I-D). So while adding formal coauthor might solves (a few) problems, it can also introduce others. You can misuse signed-off/reviewed-by/etc the same way. Ultimately there is one person who can decide which parts of the changes to put in the commit that gets sent to the maintainer. So there *is* someone who is the primary author; the person who takes the final pass on the patch and then hits the send key. If you (do it in isolation and) want to take full responsibility, yes, but I consider reviewed-by/signed-off as taking partial responsibility because it's a vetting process. One could imagine some frankly, quite rare example where there is a team of people who votes on each commit before it gets sent out and where everyone is equal and there is no hierarchy. In that case, perhaps you could set the from field to a mailing list address. But honestly, how often is that *all* of the authors are completely equal[1]? For that case something like patchwork, phabricator, or gerrit seems to be the logical tool to use, and should ideally leave a trace of approvals and such in the resulting commit message(s). If the patch management tool takes care of merging the commit(s), it can be harder to misattribute signed-off/reviewed-by/etc, which is a good thing. In my personal practice, if I make significant changes to a patch, I will indeed simply change the submitter, and then give credit the original author. This is the case where I'm essentially saying, Bob did a lot of work, but I made a bunch of changes, so if things break horribly, blame *me*, not Bob. Alternatively, if I just need to make a few cosmetic changes to Alice's patch (i.e., fix white spaces, correct spelling, change the commit description so it's validly parsable and understandable English, etc.), I'll just add a comment in square brackets indicating what changes I made before I committed the change. This seems to work just fine, and I don't think we should try to fix something that isn't broken. Perfectly valid use cases, but different from the scenarios Josh mentioned. You could of course use multiple (everybody makes their own) commits, where you risk breaking bisectability and avoid the need for equal co-authorship support. In pair programming such intermediate commits will quite often be fixups, and when you attempt to squash the fixups for bisectability's sake, you may get a desire for co-authorship of the resulting commit. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
co-authoring commits
Even though I don't have time to work on a feature like this, like others before me, I've been in situations where I would have liked to set more than one GIT_AUTHOR_NAME (etc.) for a single commit due to the involvement of multiple developers in authoring a change. Is this something that breaks the design and would never be implemented, or can it be integrated such that one can specify co-authors when committing a change? I'm thinking: $ git commit --add-author Tony Zwei elsegu...@example.com -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: co-authoring commits
On Wed, Jun 17, 2015 at 9:58 PM, Junio C Hamano wrote: Tuncer Ayaz tuncer.a...@gmail.com writes: Is this something that breaks the design and would never be implemented, Yes. Junio, thanks for the quick response. I suppose things have changed since Jonathan Nieder's response in [1] (2010), or I've read too much into the mini-thread between Jonathan and Josh. I was under the impression that this is generally possible without shaking up all underpinnings. For what it's worth, here's why I would use the feature: By allowing multiple authors, you don't have to decide who's the primary author, as in such situations usually there is no primary at all. I sometimes deliberately override the author when committing and add myself just as another co-author in the commit message, but as others have noted it would be really great if we can just specify multiple authors. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=451880 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: co-authoring commits
On Wed, Jun 17, 2015 at 11:51 PM, Junio C Hamano wrote: j...@joshtriplett.org writes: Having more than one author field in a commit would likely break things, but having a coauthor field seems plausible these days. Git added support for signed commits, and the world didn't end, so it's possible to extend the commit format. Something being possible and something being sensible are two different things, though. I agree coauthor field that is not understood by anybody would unlikely break existing implementations, but it is not a useful way to add this information to commit objects. For one thing, until you teach git log or its equivalents in everybody's (re)implementation of Git, the field will not be shown, you cannot easily edit it while amending or rebasing, git log --grep= would not know about it, and you would need git cat-file commit to view it. A footer Co-authored-by: does not have any such issue. We left commit headers extensible long before we introduced commit signing, and we used it to add the encoding header. In general, we invent new headers only when structurely necessary. When you declare that the log message for this indiviaul commit is done in one encoding, that is not something you would want to _edit_ with your editor while you are editing your message. Similarly you would not want to risk touching the GPG signature of a signed commit or a signed merge while editing your message. The _only_ reason I would imagine why somebody may be tempted to think that coauthor as part of the object header makes sense is because author is already there. You can argue that author did not have to be part of the object header, and that is right. I would agree with you 100% that author did not have to be there. But that is too late to change. And being consistent with a past mistake is not a good reason to repeat that same mistake. Makes sense. Without intimate knowledge of current internals, what about the following potentially crazy plan? 1. demote/deprecate GIT_AUTHOR_* 2. implement a new author-ship model that supports both and treats the old entries as supported-but-deprecated 3. maybe auto-migrate entries in the repo, or add a switch to do that as part of git-gc or another process 4. extend tooling to support 'commit --add-author' or similar 5. teach git.git tools to properly display additional authors as equals in commit ownership 6. let other tools catch up, but rest assured nothing was broken 7. consider other use cases and different implementations (flexibility), to not have to repeat this 5 years down the road for another field -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 745224e0 gcc-4.9 emmintrin.h build error
On Sat, Jul 12, 2014 at 6:56 AM, Jeff King wrote: On Fri, Jul 11, 2014 at 12:12:55AM +0200, Tuncer Ayaz wrote: Sorry, didn't test properly when I tried with/without config.mak, and PROFILE=BUILD was the problem. I had that in config.mak based on information gathered from INSTALL and Makefile. To be clear, is PROFILE=BUILD (still) supported? I think none of the regular devs uses PROFILE, and it bit-rotted somewhat. Andi Kleen recently posted a series to fix it[1]. I can reproduce your problem without that series, but compiling with ak/profile-feedback-build merged in seems to work OK. [1] http://thread.gmane.org/gmane.comp.version-control.git/253005 Yes, Andi's patch works. Any idea when ak/profile-feedback-build will land in master? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 745224e0 gcc-4.9 emmintrin.h build error
On Sat, Jul 12, 2014 at 6:56 AM, Jeff King wrote: I think none of the regular devs uses PROFILE, and it bit-rotted By the way, is there no build (CI) server for git.git to regularly test branches on different platforms or at least different build configs on the same platform? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
745224e0 gcc-4.9 emmintrin.h build error
The changes in 745224e0 don't seem to build here with gcc-4.9 on linux x64_64. Any idea what's wrong? CC credential-store.o In file included from /usr/lib/.../xmmintrin.h:31:0, from /usr/lib/.../emmintrin.h:31, from git-compat-util.h:708, from cache.h:4, from credential-store.c:1: /usr/lib/.../mmintrin.h: In function '_mm_cvtsi32_si64': /usr/lib/.../mmintrin.h:64:3: error: can't convert between vector values of different size return (__m64) __builtin_ia32_vec_init_v2si (__i, 0); ^ /usr/lib/.../mmintrin.h: In function '_mm_cvtsi64_si32': /usr/lib/.../mmintrin.h:107:10: error: incompatible type for argument 1 of '__builtin_ia32_vec_ext_v2si' return __builtin_ia32_vec_ext_v2si ((__v2si)__i, 0); ^ [...] In file included from /usr/lib/.../emmintrin.h:31:0, from git-compat-util.h:708, from cache.h:4, from credential-store.c:1: /usr/lib/.../xmmintrin.h: In function '_mm_add_ss': /usr/lib/.../xmmintrin.h:127:19: error: incompatible type for argument 1 of '__builtin_ia32_addss' return (__m128) __builtin_ia32_addss ((__v4sf)__A, (__v4sf)__B); ^ /usr/lib/.../xmmintrin.h:127:3: note: expected '__vector(4) float' but argument is of type '__m128' return (__m128) __builtin_ia32_addss ((__v4sf)__A, (__v4sf)__B); ^ /usr/lib/.../xmmintrin.h:127:19: error: incompatible type for argument 2 of '__builtin_ia32_addss' return (__m128) __builtin_ia32_addss ((__v4sf)__A, (__v4sf)__B); ^ [...] /usr/lib/.../emmintrin.h:1455:3: error: incompatible type for argument 2 of '__builtin_ia32_movntpd' __builtin_ia32_movntpd (__A, (__v2df)__B); ^ /usr/lib/.../emmintrin.h:1455:3: note: expected '__vector(2) double' but argument is of type '__m128d' Makefile:1983: recipe for target 'credential-store.o' failed make: *** [credential-store.o] Error 1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 745224e0 gcc-4.9 emmintrin.h build error
On Thu, Jul 10, 2014 at 9:59 PM, Tuncer Ayaz wrote: The changes in 745224e0 don't seem to build here with gcc-4.9 on linux x64_64. Any idea what's wrong? s/x64_64/x86_64/ Should have written amd64 to avoid the typo :). CC credential-store.o In file included from /usr/lib/.../xmmintrin.h:31:0, from /usr/lib/.../emmintrin.h:31, from git-compat-util.h:708, from cache.h:4, from credential-store.c:1: /usr/lib/.../mmintrin.h: In function '_mm_cvtsi32_si64': /usr/lib/.../mmintrin.h:64:3: error: can't convert between vector values of different size return (__m64) __builtin_ia32_vec_init_v2si (__i, 0); ^ /usr/lib/.../mmintrin.h: In function '_mm_cvtsi64_si32': /usr/lib/.../mmintrin.h:107:10: error: incompatible type for argument 1 of '__builtin_ia32_vec_ext_v2si' return __builtin_ia32_vec_ext_v2si ((__v2si)__i, 0); ^ [...] In file included from /usr/lib/.../emmintrin.h:31:0, from git-compat-util.h:708, from cache.h:4, from credential-store.c:1: /usr/lib/.../xmmintrin.h: In function '_mm_add_ss': /usr/lib/.../xmmintrin.h:127:19: error: incompatible type for argument 1 of '__builtin_ia32_addss' return (__m128) __builtin_ia32_addss ((__v4sf)__A, (__v4sf)__B); ^ /usr/lib/.../xmmintrin.h:127:3: note: expected '__vector(4) float' but argument is of type '__m128' return (__m128) __builtin_ia32_addss ((__v4sf)__A, (__v4sf)__B); ^ /usr/lib/.../xmmintrin.h:127:19: error: incompatible type for argument 2 of '__builtin_ia32_addss' return (__m128) __builtin_ia32_addss ((__v4sf)__A, (__v4sf)__B); ^ [...] /usr/lib/.../emmintrin.h:1455:3: error: incompatible type for argument 2 of '__builtin_ia32_movntpd' __builtin_ia32_movntpd (__A, (__v2df)__B); ^ /usr/lib/.../emmintrin.h:1455:3: note: expected '__vector(2) double' but argument is of type '__m128d' Makefile:1983: recipe for target 'credential-store.o' failed make: *** [credential-store.o] Error 1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 745224e0 gcc-4.9 emmintrin.h build error
On Thu, Jul 10, 2014 at 10:23 PM, Jeff King wrote: On Thu, Jul 10, 2014 at 09:59:37PM +0200, Tuncer Ayaz wrote: The changes in 745224e0 don't seem to build here with gcc-4.9 on linux x64_64. Any idea what's wrong? Weird. It compiles fine on my x86_64 box (Debian unstable, gcc 4.9.0-10). Can you tell us more about your environment? gcc version 4.9.0 20140604 (prerelease) I normally use a custom config.mak, but I get the error without it too, so it has no direct effect. Let me know if there's anything to try out for finding the difference. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 745224e0 gcc-4.9 emmintrin.h build error
On Thu, Jul 10, 2014 at 10:33 PM, David Turner wrote: On Thu, 2014-07-10 at 21:59 +0200, Tuncer Ayaz wrote: The changes in 745224e0 don't seem to build here with gcc-4.9 on linux x64_64. Any idea what's wrong? CC credential-store.o In file included from /usr/lib/.../xmmintrin.h:31:0, What's in the ...? Because if you're using headers from a different version of gcc, that might explain it. /usr/lib/gcc/x86_64-unknown-linux-gnu/4.9.0/include/emmintrin.h -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 745224e0 gcc-4.9 emmintrin.h build error
On Thu, Jul 10, 2014 at 10:53 PM, David Turner wrote:
On Thu, 2014-07-10 at 22:44 +0200, Tuncer Ayaz wrote:
On Thu, Jul 10, 2014 at 10:33 PM, David Turner wrote:
On Thu, 2014-07-10 at 21:59 +0200, Tuncer Ayaz wrote:
The changes in 745224e0 don't seem to build here with gcc-4.9 on
linux x64_64. Any idea what's wrong?
CC credential-store.o
In file included from /usr/lib/.../xmmintrin.h:31:0,
What's in the ...?
Because if you're using headers from a different version of gcc, that
might explain it.
/usr/lib/gcc/x86_64-unknown-linux-gnu/4.9.0/include/emmintrin.h
That seems fine to me.
It looks like the error messages are coming from inside the system's
header files (but this is sometimes misleading). If you just try to
compile
#include emmintrin.h
int main() { }
with whatever options you use for git, does that work? If not, I would
say that you have a compiler setup problem.
The above test works on the same machine, so I'll
investigate what's going on when building git. Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 745224e0 gcc-4.9 emmintrin.h build error
Sorry, didn't test properly when I tried with/without config.mak, and PROFILE=BUILD was the problem. I had that in config.mak based on information gathered from INSTALL and Makefile. To be clear, is PROFILE=BUILD (still) supported? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
