Re: [RFC/PATCH] Add interpret-trailers builtin
On Wed, Nov 6, 2013 at 7:43 AM, Christian Couder chrisc...@tuxfamily.org wrote: Of course in the latter case, a command should probably be specified to tell which value should be used with the key. For example: [trailer signoff] key = Signed-off-by: if_missing = append command = echo $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL' would append a s-o-b line only if there is no s-o-b already. Sorry, I realize that I was wrong above. As the default for if_exist is dont_repeat, the above would append a s-o-b if there is no s-o-b or if there is one but with a different value. To append a s-o-b only if there is no s-o-b already, one would need to use: [trailer signoff] key = Signed-off-by: if_exist = dont_append if_missing = append command = echo $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL' Thanks, Christian. -- 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: [PATCH] Catch more exceptions in compat_log_entry()
On 22 October 2013 15:31, Pavel Roskin pro...@gnu.org wrote: Catch exceptions in default_repo(). Catch git.RepositoryException. This suppresses stack trace in stg pull on detached head and outside the repository. Signed-off-by: Pavel Roskin pro...@gnu.org Thanks. Applied. Catalin -- 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: git svn clone with funky tags layout
Jim Garrison venit, vidit, dixit 05.11.2013 18:16: I'm doing a one-time migration of an svn project. For historical reasons our repo layout is weird: trunk/reporting/reporting_app tags/something_else tags/reporting_app-2.3.45 tags/reporting_app-2.4.46 tags/reporting_app-2.4.0 tags/reporting_app-2.4.1 tags/more_stuff I want to migrate the trunk plus only the 2.4.* tags. I tried giving a wildcard in the config [svn-remote svn] noMetadata = 1 url = http://subversion.tld.com/svn/DevJava fetch = trunk/Reporting/reporting_app:refs/remotes/trunk tags = tags/reporting_app-2.4.*:refs/remotes/tags/* but this does not work: Invalid pattern in 'tags/reporting_app-2.4.*': reporting_app-2.4.* On the off chance that it might work I also tried tags = tags/reporting_app-2.4.*:refs/remotes/tags/reporting_app-2.4.* but that produces the same error message. Is there a way to accomplish this or should I just move the tags I want to import into a separate directory in subversion first? I'd rather not disturb svn but can do that if it's the only way. Depending on whether there are more tags to skip or more to include, you can A) use the standard refspec (--tags=tags/) and remove the superfluous tag branches once git-svn is finished or B) use multiple tag refspecs without wildcard: tags = tags/reporting_app-2.4.0:refs/remotes/tags/2.4.0 tags = tags/reporting_app-2.4.1:refs/remotes/tags/2.4.1 I may have mixed up A and B ;) Michael -- 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
[PATCH 1/2] receive-pack: advertise thin-pack
upload-pack has long advertised thin-pack, letting the clients request these smaller packs. The client however unconditionally assumes that a server is able to fix thin packs and there is no way of telling the client that this is in fact not the case. Make receive-pack advertise 'thin-pack' in anticipation of the client toggling the assumption and document this capability when used by receive-pack. Signed-off-by: Carlos Martín Nieto c...@elego.de --- Documentation/technical/protocol-capabilities.txt | 20 +++- builtin/receive-pack.c| 2 +- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index fd8ffa5..4e96d51 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -72,15 +72,25 @@ interleaved with S-R-Q. thin-pack - -This capability means that the server can send a 'thin' pack, a pack -which does not contain base objects; if those base objects are available -on client side. Client requests 'thin-pack' capability when it -understands how to thicken it by adding required delta bases making -it self-contained. +A thin pack is one with deltas which reference base objects not +contained within the pack (but are known to exist at the receiving +end). This can reduce the network traffic significantly, but it +requires the receiving end to know how to thicken these packs by +adding the missing bases to the pack. + +The upload-pack server advertises 'thin-pack' when it can generate and +send a thin pack. The receive-pack server advertises 'thin-pack' when +it knows how to thicken the pack it receives. + +Likewise, the client requests the 'thin-pack' capability when it +understands how to thicken it. Client MUST NOT request 'thin-pack' capability if it cannot turn a thin pack into a self-contained pack. +Client MUST NOT send a thin pack if the server does not advertise this +capability. + side-band, side-band-64k diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index e3eb5fc..0e35c02 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -132,7 +132,7 @@ static void show_ref(const char *path, const unsigned char *sha1) else packet_write(1, %s %s%c%s%s agent=%s\n, sha1_to_hex(sha1), path, 0, - report-status delete-refs side-band-64k quiet, + report-status delete-refs side-band-64k quiet thin-pack, prefer_ofs_delta ? ofs-delta : , git_user_agent_sanitized()); sent_capabilities = 1; -- 1.8.4.652.g0d6e0ce -- 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
[PATCH 2/2] send-pack: only send a thin pack if the server supports it
In combination a the previous patch making receive-pack advertise the thin-pack capability, this allows git to push to a server in a constrained environment which is not able to fix thin packs while taking advantage of the feature for servers which can. Signed-off-by: Carlos Martín Nieto c...@elego.de --- send-pack.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/send-pack.c b/send-pack.c index 7d172ef..7b88ac8 100644 --- a/send-pack.c +++ b/send-pack.c @@ -205,6 +205,8 @@ int send_pack(struct send_pack_args *args, quiet_supported = 1; if (server_supports(agent)) agent_supported = 1; + if (!server_supports(thin-pack)) + args-use_thin_pack = 0; if (!remote_refs) { fprintf(stderr, No refs in common and none specified; doing nothing.\n -- 1.8.4.652.g0d6e0ce -- 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
[PATCH 0/2] thin-pack capability for send-pack/receive-pack
Hi all, This comes as a result of the discussion starting at [0] about git-push assuming that a server will always support thin packs. Most out there in fact do, but this isn't necessarily the case. Some implementations may not have support for it yet, or the server might be running in an environment where it is not feasible for it to try to fill in the missing objects. Jonathan and Duy mentioned that separate patches for receive-pack and send-pack could let us work around adding this at such a late stage, so here they are. The second patch can maybe lie in waiting for a while. [0] http://thread.gmane.org/gmane.comp.version-control.git/235766/focus=236402 Carlos Martín Nieto (2): receive-pack: advertise thin-pack send-pack: only send a thin pack if the server supports it Documentation/technical/protocol-capabilities.txt | 20 +++- builtin/receive-pack.c| 2 +- send-pack.c | 2 ++ 3 files changed, 18 insertions(+), 6 deletions(-) -- 1.8.4.652.g0d6e0ce -- 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: [PATCH 2/2] git_connect: factor out discovery of the protocol and its parts
On 2013-11-05 22.22, Johannes Sixt wrote: Am 05.11.2013 21:45, schrieb Torsten Bögershausen: On 2013-11-05 20.39, Johannes Sixt wrote: Thanks for picking this up, please see some minor nits inline, and git_connect() is at the end -struct child_process *git_connect(int fd[2], const char *url_orig, - const char *prog, int flags) +static enum protocol parse_connect_url(const char *url_orig, char **ret_host, + char **ret_port, char **ret_path) { char *url; char *host, *path; char *end; Can we put all the char * into one single line? The idea here was to keep the diff minimal, and that further slight cleanups should be combined with subsequent rewrites that should happen to this function. int c; @@ -645,6 +628,49 @@ struct child_process *git_connect(int fd[2], const char *url_orig, if (protocol == PROTO_SSH host != url) port = get_port(end); + *ret_host = xstrdup(host); + if (port) + *ret_port = xstrdup(port); + else + *ret_port = NULL; + if (free_path) + *ret_path = path; + else + *ret_path = xstrdup(path); + free(url); + return protocol; +} + +static struct child_process no_fork; + +/* + * This returns a dummy child_process if the transport protocol does not + * need fork(2), or a struct child_process object if it does. Once done, + * finish the connection with finish_connect() with the value returned from + * this function (it is safe to call finish_connect() with NULL to support + * the former case). + * + * If it returns, the connect is successful; it just dies on errors (this + * will hopefully be changed in a libification effort, to return NULL when + * the connection failed). + */ +struct child_process *git_connect(int fd[2], const char *url, + const char *prog, int flags) +{ + char *host, *path; + struct child_process *conn = no_fork; + enum protocol protocol; + char *port; + const char **arg; + struct strbuf cmd = STRBUF_INIT; + + /* Without this we cannot rely on waitpid() to tell +* what happened to our children. +*/ + signal(SIGCHLD, SIG_DFL); + + protocol = parse_connect_url(url, host, port, path); + if (protocol == PROTO_GIT) { /* These underlying connection commands die() if they * cannot connect. @@ -666,9 +692,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig, prog, path, 0, target_host, 0); free(target_host); This is hard to see in the diff, I think we don't need target_host any more. I though that as well first, but no, we still need it. Further rewrites are needed that move the port discovery from git_proxy_connect() and git_tcp_connect() to the new parse_connect_url() before target_host can go away. And even then it is questionable because target_host is used in an error message and is intended to reflect the original combined host+port portion of the URL, if I read the code correctly. - free(url); - if (free_path) - free(path); + free(host); + free(port); + free(path); return conn; } @@ -709,9 +735,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig, fd[0] = conn-out; /* read from child's stdout */ fd[1] = conn-in; /* write to child's stdin */ strbuf_release(cmd); - free(url); - if (free_path) - free(path); This end of function, free everything and return conn, could we re-arange so that it is in the code only once ? That would be quite simple now; just place the part after the first return into the else branch. That opens opportunities to move variable declarations from the top of the function into the else branch. But all of these changes should go into a separate commit, IMO, so that the function splitting that happens here can be verified more easily. -- Hannes Agreed on all points, (some re-reading was needed) I will first focus on the test cases, since having god test cases eases us the re-factoring later on. Thanks /Torsten -- 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: htonll, ntohll
On 2013-11-05 01.00, Ramsay Jones wrote: On 31/10/13 13:24, Torsten Bögershausen wrote: On 2013-10-30 22.07, Ramsay Jones wrote: [ ... ] Yep, this was the first thing I did as well! ;-) (*late* last night) I haven't had time today to look into fixing up the msvc build (or a complete re-write), so I look forward to seeing your solution. (do you have msvc available? - or do you want me to look at fixing it? maybe in a day or two?) Ramsay, I don't have msvc, so feel free to go ahead, as much as you can. I'll send a patch for the test code I have made, and put bswap.h on hold for a week (to be able to continue with t5601/connect.c) Unfortunately, I haven't had much time to look into this. I do have a patch (given below) that works on Linux, cygwin, MinGW and msvc. However, the msvc build is still broken (as a result of _other_ commits in this 'jk/pack-bitmap' branch; as well as the use of a VLA in another commit). So, I still have work to do! :( Anyway, I thought I would send what I have, so you can take a look. Note, that I don't have an big-endian machine to test this on, so YMMV. Indeed, the *only* testing I have done is to run the test added by this branch (t5310-pack-bitmaps.sh), which works on Linux, cygwin and MinGW. [Note: I have never particularly liked htons, htonl et.al., so adding these htonll/ntohll functions doesn't thrill me! :-D For example see this post[1], which echo's my sentiments exactly.] HTH ATB, Ramsay Jones [1] http://commandcenter.blogspot.co.uk/2012/04/byte-order-fallacy.html -- 8 -- Subject: [PATCH] compat/bswap.h: Fix build on cygwin, MinGW and msvc --- compat/bswap.h | 97 -- 1 file changed, 68 insertions(+), 29 deletions(-) diff --git a/compat/bswap.h b/compat/bswap.h index ea1a9ed..c18a78e 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -17,7 +17,20 @@ static inline uint32_t default_swab32(uint32_t val) ((val 0x00ff) 24)); } +static inline uint64_t default_bswap64(uint64_t val) +{ + return (((val (uint64_t)0x00ffULL) 56) | + ((val (uint64_t)0xff00ULL) 40) | + ((val (uint64_t)0x00ffULL) 24) | + ((val (uint64_t)0xff00ULL) 8) | + ((val (uint64_t)0x00ffULL) 8) | + ((val (uint64_t)0xff00ULL) 24) | + ((val (uint64_t)0x00ffULL) 40) | + ((val (uint64_t)0xff00ULL) 56)); +} + #undef bswap32 +#undef bswap64 #if defined(__GNUC__) (defined(__i386__) || defined(__x86_64__)) @@ -32,54 +45,80 @@ static inline uint32_t git_bswap32(uint32_t x) return result; } +#define bswap64 git_bswap64 +#if defined(__x86_64__) +static inline uint64_t git_bswap64(uint64_t x) +{ + uint64_t result; + if (__builtin_constant_p(x)) + result = default_bswap64(x); + else + __asm__(bswap %q0 : =r (result) : 0 (x)); + return result; +} +#else +static inline uint64_t git_bswap64(uint64_t x) +{ + union { uint64_t i64; uint32_t i32[2]; } tmp, result; + if (__builtin_constant_p(x)) + result.i64 = default_bswap64(x); + else { + tmp.i64 = x; + result.i32[0] = git_bswap32(tmp.i32[1]); + result.i32[1] = git_bswap32(tmp.i32[0]); + } + return result.i64; +} +#endif + #elif defined(_MSC_VER) (defined(_M_IX86) || defined(_M_X64)) #include stdlib.h #define bswap32(x) _byteswap_ulong(x) +#define bswap64(x) _byteswap_uint64(x) #endif -#ifdef bswap32 +#if defined(bswap32) #undef ntohl #undef htonl #define ntohl(x) bswap32(x) #define htonl(x) bswap32(x) -#ifndef __BYTE_ORDER -#if defined(BYTE_ORDER) defined(LITTLE_ENDIAN) defined(BIG_ENDIAN) -#define __BYTE_ORDER BYTE_ORDER -#define __LITTLE_ENDIAN LITTLE_ENDIAN -#define __BIG_ENDIAN BIG_ENDIAN -#else -#error Cannot determine endianness -#endif +#endif + +#if defined(bswap64) + +#undef ntohll +#undef htonll +#define ntohll(x) bswap64(x) +#define htonll(x) bswap64(x) + +#else + +#undef ntohll +#undef htonll + +#if !defined(__BYTE_ORDER) +# if defined(BYTE_ORDER) defined(LITTLE_ENDIAN) defined(BIG_ENDIAN) +# define __BYTE_ORDER BYTE_ORDER +# define __LITTLE_ENDIAN LITTLE_ENDIAN +# define __BIG_ENDIAN BIG_ENDIAN +# endif +#endif + +#if !defined(__BYTE_ORDER) +# error Cannot determine endianness #endif #if __BYTE_ORDER == __BIG_ENDIAN # define ntohll(n) (n) # define htonll(n) (n) -#elif __BYTE_ORDER == __LITTLE_ENDIAN -#if defined(__GNUC__) defined(__GLIBC__) -#include byteswap.h -#else /* GNUC GLIBC */ -static inline uint64_t bswap_64(uint64_t val) -{ - return ((val
RE: git svn clone with funky tags layout
-Original Message- From: Michael J Gruber [mailto:g...@drmicha.warpmail.net] Sent: Wednesday, November 06, 2013 6:03 AM To: Jim Garrison; git@vger.kernel.org Subject: Re: git svn clone with funky tags layout Jim Garrison venit, vidit, dixit 05.11.2013 18:16: I'm doing a one-time migration of an svn project. For historical reasons our repo layout is weird: trunk/reporting/reporting_app tags/something_else tags/reporting_app-2.3.45 tags/reporting_app-2.4.46 tags/reporting_app-2.4.0 tags/reporting_app-2.4.1 tags/more_stuff I want to migrate the trunk plus only the 2.4.* tags. I tried giving a wildcard in the config [svn-remote svn] noMetadata = 1 url = http://subversion.tld.com/svn/DevJava fetch = trunk/Reporting/reporting_app:refs/remotes/trunk tags = tags/reporting_app-2.4.*:refs/remotes/tags/* but this does not work: Invalid pattern in 'tags/reporting_app-2.4.*': reporting_app-2.4.* On the off chance that it might work I also tried tags = tags/reporting_app-2.4.*:refs/remotes/tags/reporting_app-2.4.* but that produces the same error message. Is there a way to accomplish this or should I just move the tags I want to import into a separate directory in subversion first? I'd rather not disturb svn but can do that if it's the only way. Depending on whether there are more tags to skip or more to include, you can A) use the standard refspec (--tags=tags/) and remove the superfluous tag branches once git-svn is finished or B) use multiple tag refspecs without wildcard: tags = tags/reporting_app-2.4.0:refs/remotes/tags/2.4.0 tags = tags/reporting_app-2.4.1:refs/remotes/tags/2.4.1 I may have mixed up A and B ;) Thanks for the response. Since there was a lot of reorganization to be done I decided to just copy all the disparate directories into a new correctly structured tree in svn first, and import from there. Worked great. -- 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: [RFC/PATCH] Add interpret-trailers builtin
Christian Couder christian.cou...@gmail.com writes: To append a s-o-b only if there is no s-o-b already, one would need to use: [trailer signoff] key = Signed-off-by: if_exist = dont_append if_missing = append command = echo $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL' But that is insufficient to emulate what we do, no? I.e. append unless the last one is from the same person we are about to add. -- 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: rcs
Jakub Narębski jna...@gmail.com writes: Finnerty, James M Mr CTR USA USASOC-SOAR wrote Jeff King [mailto:p...@peff.net]wrote: On Tue, Oct 29, 2013 at 11:35:21AM -0500, Finnerty, James M Mr CTR USA USASOC-SOAR wrote: Hi. I'm going to attempt to import a git database into Razor which is linux rcs based. Does the linux version of git use rcs ? No, the formats are completely different, and you will have to translate. We don't usually get requests to go from git to rcs; it usually goes the other way. :) Thanks. We have several systems using Razor right now. So we are trying to get all the systems into one CM system. Razor is just a gui that uses rcs commands. Once we get everything synced we will explore our options for a complete development CM system. The problem with using RCS as sync (as base) is that it is least powerfull of VCS, and as far as I know do not offer place to store extra information, so conversion from Git to RSS will lose some information (committership, signed commits and signed merges, signed tags, etc.). You forgot to mention another important one: atomicity of commits across the entire tree. The best you could do would be to assume that changes in such a collection of RCS ,v files from a Git export to different files with the same timestamp and by the same author are likely to have come from the same Git commit, and that the timestamp monotonically increases, in order to stitch the history back together. -- 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: [PATCH] git-cat-file: fix output when format string contains no variables
Sven Brauch svenbra...@googlemail.com writes: From 2e7b5aed771faeff654a447346bb0b57570d9569 Mon Sep 17 00:00:00 2001 From: Sven Brauch svenbra...@googlemail.com Date: Tue, 5 Nov 2013 20:06:21 +0100 Subject: [PATCH] git-cat-file: fix output when format string contains no variables Thanks; first some procedural issues: - Omit the first line From 2e7b...; that does not belong to any patch submission (it is a separator used between messages in the mailbox formatted file format-patch --stdout produces). - The second line From: Sven... records exactly the same address as the e-mail you are sending out, so it should be omitted as well. - The third line Date: ... is not the time we the general public sees your fix for the first time, which is what Date: of your e-mail header already records, so we do not need it either. - And the last one Subject: ... is redundant; we can see it in your e-mail header. In general, the latter three lines are produced by format-patch to help you fill header fields in the MUA of your choice by cutting (not copying) and pasting. Unless there is a valid reason to have values different from what recipients would see in the e-mail header (and there often isn't, unless you are forwarding somebody else's patch, in which case you may want to use From: , or you are responding to an ongoing discussion with a patch, in which case you may want to use Subject: ), please remove them after copying them out to your e-mail header. When the format string for git-cat-object --batch-check contained no variables, the function would not actually look for the object on disk, but just verify that the hash is correct. Thus it would report no error if asking for objects which did not actually exist on disk if the SHA hash looked ok. Example of buggy behaviour: echo XYZ | git hash-object --stdin | git cat-file --batch-check=found would report found although the manpage claims it would report an error. An excellent log message. It would have been even better to add a new test to t1006 based on this reproduction recipe. Signed-off-by: Sven Brauch svenbra...@googlemail.com --- Notes: This fixes a bug where git-cat-file --batch-check would erroneously tell that objects exist while they did in fact not in case the argument to --batch-check was just a constant strig (i.e. no %(...) variables). The reason was that calling sha1_object_info_extended while requesting no properties of the object would not even verify this object existed, or more exactly, sha1_loose_object_info would not do that. I'm entirely unfamiliar with the git codebase; the suggested fix ensures that always at least one property is requested. If there's a better way to fix this issue, please let me know. I think the real problem is that sha1_loose_object_info() is called by sha1_object_info_extended(), when it does not find a cached or a packed object, and the callee assumes that it is asked to fill in only the requested pieces of information while the caller does not even bother to check if such an object actually exists. How about doing it like the attached instead? -- 8 -- Subject: sha1_loose_object_info(): do not return success on missing object Since 052fe5ea (sha1_loose_object_info: make type lookup optional, 2013-07-12), sha1_loose_object_info() returns happily without checking if the object in question exists, which is not what the the caller sha1_object_info_extended() expects; the caller does not even bother checking the existence of the object itself. Noticed-by: Sven Brauch svenbra...@googlemail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- Oh, by the way, there is this one iffy bit in batch_one_object(): if (get_sha1(obj_name, data-sha1)) { printf(%s missing\n, obj_name); fflush(stdout); return 0; } At this point, the object _may_ be missing, but the obj_name may be malformed, so saying missing is not strictly correct. If, for example, you misspelled the name of the master branch, you would get this: $ echo mastre | git cat-file --batch-check=foo mastre missing I however doubt that it is a good idea to reword this message by adding a logic to tell misspelled object name and missing object name apart. The users of cat-file --batch-check are not expecting to be able to distinguish these two classes of errors anyway. sha1_file.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 7dadd04..00220a4 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2486,12 +2486,11 @@ static int sha1_loose_object_info(const unsigned char *sha1, * need to look inside the object at all. */ if (!oi-typep !oi-sizep) { - if (oi-disk_sizep) { - struct stat st; - if
Re: [PATCH] git-cat-file: fix output when format string contains no variables
On Wed, Nov 06, 2013 at 10:00:57AM -0800, Junio C Hamano wrote: I think the real problem is that sha1_loose_object_info() is called by sha1_object_info_extended(), when it does not find a cached or a packed object, and the callee assumes that it is asked to fill in only the requested pieces of information while the caller does not even bother to check if such an object actually exists. Yes, exactly. This is over-optimization on the part of my 052fe5ea. The caller asked for nothing, so we happily optimize the request to do nothing. But I forgot there is always an implicit does it even exist query in the call. I do not see any point in adding an exists query field to struct object_info. :) -- 8 -- Subject: sha1_loose_object_info(): do not return success on missing object Since 052fe5ea (sha1_loose_object_info: make type lookup optional, 2013-07-12), sha1_loose_object_info() returns happily without checking if the object in question exists, which is not what the the caller sha1_object_info_extended() expects; the caller does not even bother checking the existence of the object itself. Noticed-by: Sven Brauch svenbra...@googlemail.com Signed-off-by: Junio C Hamano gits...@pobox.com This is definitely the right fix. Commit message and patch look good to me. A few extra bits are in the patch below. Oh, by the way, there is this one iffy bit in batch_one_object(): if (get_sha1(obj_name, data-sha1)) { printf(%s missing\n, obj_name); fflush(stdout); return 0; } At this point, the object _may_ be missing, but the obj_name may be malformed, so saying missing is not strictly correct. If, for example, you misspelled the name of the master branch, you would get this: $ echo mastre | git cat-file --batch-check=foo mastre missing I however doubt that it is a good idea to reword this message by adding a logic to tell misspelled object name and missing object name apart. The users of cat-file --batch-check are not expecting to be able to distinguish these two classes of errors anyway. Yes, I noticed that while doing the original series, but left it intact for the exact reason you mention. Note that it is going to stdout and is part of the actual data stream (so there is a good chance scripts are parsing it). diff --git a/sha1_file.c b/sha1_file.c index 7dadd04..00220a4 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2486,12 +2486,11 @@ static int sha1_loose_object_info(const unsigned char *sha1, * need to look inside the object at all. */ if (!oi-typep !oi-sizep) { - if (oi-disk_sizep) { - struct stat st; - if (stat_sha1_file(sha1, st) 0) - return -1; + struct stat st; + if (stat_sha1_file(sha1, st) 0) + return -1; + if (oi-disk_sizep) *oi-disk_sizep = st.st_size; - } return 0; Here's a squashable patch which expands the comment above the code you're tweaking, and adds a test to t1006. I notice that t1006 could use some modernization, whitespace-fixing, and typo-fixing, but I left that out of the patch for clarity. Thanks both of you for working on this. -Peff --- diff --git a/sha1_file.c b/sha1_file.c index 3474dca..10676ba 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2489,7 +2489,11 @@ static int sha1_loose_object_info(const unsigned char *sha1, /* * If we don't care about type or size, then we don't -* need to look inside the object at all. +* need to look inside the object at all. Note that we +* do not optimize out the stat call, even if the +* caller doesn't care about the disk-size, since our +* return value implicitly indicates whether the +* object even exists. */ if (!oi-typep !oi-sizep) { struct stat st; diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index a420742..8a1bc5c 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -194,6 +194,12 @@ test_expect_success --batch-check for an emtpy line ' test missing = $(echo | git cat-file --batch-check) ' +test_expect_success 'empty --batch-check notices missing object' ' + echo $_z40 missing expect + echo $_z40 | git cat-file --batch-check= actual + test_cmp expect actual +' + batch_input=$hello_sha1 $commit_sha1 $tag_sha1 -- 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: [PATCH v3] push: Enhance unspecified push default warning
Matthieu Moy matthieu@grenoble-inp.fr writes: I don't remember all the discussions on the patch which introduced the warning, but I don't think it's relevant to digg them before applying the patch: If we apply the patch then it is too late to dig them ;-) * The assumption was that users would read the docs, but as I already mentioned: Judging by the question asked on stackoverflow ( http://stackoverflow.com/questions/13148066/warning-push-default-is-unset-its-implicit-value-is-changing-in-git-2-0 ) and its popularity, telling the users to read the docs did not work very well. That is true, but does it justify giving a misleading information in the advice message? Specifically: + When push.default is set to 'matching', git will push all local branches\n + to the remote branches with the same (matching) name. invites those who do not read documentation to mistake it with using an explicit refs/heads/*:refs/heads/* refspec. And this one + In Git 2.0 the new push.default of 'simple' will push only the current\n + branch to the same remote branch used by git pull. A push will\n + only succeed if the remote and local branches have the same name.\n while you can see that it is not telling a lie if you read it twice, will only succeed if feels somewhat roundabout. ... push only the current branch back to the branch of the same name, but only if 'git pull' is set to pull from that branch. Otherwise the push will fail. might be an improvement, but I dunno. * The warning has been there for a while now. Advanced users have already set push.default. We shouldn't be worried about eating a bit of screen real estate for users who didn't yet. This part I can agree with. -- 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
[PATCH] Add the commit.gpgsign option to sign all commits
If you want to GPG sign all your commits, you have to add the -S option all the time. The commit.gpgsign config option allows to sign all commits automatically. Signed-off-by: Nicolas Vigier bo...@mars-attacks.org --- The option description now suggests using an agent. Documentation/config.txt | 7 +++ builtin/commit-tree.c| 7 ++- builtin/commit.c | 4 builtin/merge.c | 3 +++ 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ab26963d6187..ffaa37752a39 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -988,6 +988,13 @@ commit.cleanup:: have to remove the help lines that begin with `#` in the commit log template yourself, if you do this). +commit.gpgsign:: + A boolean to specify whether all commits should be GPG signed. + Use of this option when doing operations such as rebase can + result in a large number of commits being signed. It is therefore + convenient to use an agent to avoid typing your gpg passphrase + several times. + commit.status:: A boolean to enable/disable inclusion of status information in the commit message template when using an editor to prepare the commit diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c index f641ff2a898c..1646d5b25e4f 100644 --- a/builtin/commit-tree.c +++ b/builtin/commit-tree.c @@ -12,6 +12,8 @@ static const char commit_tree_usage[] = git commit-tree [(-p sha1)...] [-S[keyid]] [-m message] [-F file] sha1 changelog; +static const char *sign_commit; + static void new_parent(struct commit *parent, struct commit_list **parents_p) { unsigned char *sha1 = parent-object.sha1; @@ -31,6 +33,10 @@ static int commit_tree_config(const char *var, const char *value, void *cb) int status = git_gpg_config(var, value, NULL); if (status) return status; + if (!strcmp(var, commit.gpgsign)) { + sign_commit = git_config_bool(var, value) ? : NULL; + return 0; + } return git_default_config(var, value, cb); } @@ -41,7 +47,6 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix) unsigned char tree_sha1[20]; unsigned char commit_sha1[20]; struct strbuf buffer = STRBUF_INIT; - const char *sign_commit = NULL; git_config(commit_tree_config, NULL); diff --git a/builtin/commit.c b/builtin/commit.c index 6ab4605cf5c2..cffddf210807 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1406,6 +1406,10 @@ static int git_commit_config(const char *k, const char *v, void *cb) } if (!strcmp(k, commit.cleanup)) return git_config_string(cleanup_arg, k, v); + if (!strcmp(k, commit.gpgsign)) { + sign_commit = git_config_bool(k, v) ? : NULL; + return 0; + } status = git_gpg_config(k, v, NULL); if (status) diff --git a/builtin/merge.c b/builtin/merge.c index 02a69c14e6ab..fea27244557d 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -604,6 +604,9 @@ static int git_merge_config(const char *k, const char *v, void *cb) } else if (!strcmp(k, merge.defaulttoupstream)) { default_to_upstream = git_config_bool(k, v); return 0; + } else if (!strcmp(k, commit.gpgsign)) { + sign_commit = git_config_bool(k, v) ? : NULL; + return 0; } status = fmt_merge_msg_config(k, v, cb); -- 1.8.4.2 -- 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: [PATCH] git-cat-file: fix output when format string contains no variables
On Wednesday 06 November 2013 10:00:57 Junio C Hamano wrote: Thanks; first some procedural issues: Thanks, I will take care of the mentioned points for future submissions. I think the real problem is that sha1_loose_object_info() is called by sha1_object_info_extended(), when it does not find a cached or a packed object, and the callee assumes that it is asked to fill in only the requested pieces of information while the caller does not even bother to check if such an object actually exists. How about doing it like the attached instead? Yes; this seems more like a proper fix. I would prefer it over my suggestion. It is illogical that sha1_loose_object_info sometimes returns an error if the object does not exist and sometimes not, depending on which properties are requested. Sven -- 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: [PATCH v3] push: Enhance unspecified push default warning
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: I don't remember all the discussions on the patch which introduced the warning, but I don't think it's relevant to digg them before applying the patch: If we apply the patch then it is too late to dig them ;-) * The assumption was that users would read the docs, but as I already mentioned: Judging by the question asked on stackoverflow ( http://stackoverflow.com/questions/13148066/warning-push-default-is-unset-its-implicit-value-is-changing-in-git-2-0 ) and its popularity, telling the users to read the docs did not work very well. That is true, but does it justify giving a misleading information in the advice message? Also applying this will have an unpleasant fallout to merging the endgame patch b2ed944a (push: switch default from matching to simple, 2013-01-04). The added text needs to be corrected with an evil merge. I'd prefer to having worry about such a fallout only once. Which arguably we already did when we came up with the current message, so I am fairly annoyed by this patch coming loong after we concluded the original discussion. Sigh X-. I'll worry about this later, as b2ed944a is in 'next' during the feature freeze, and I'd prefer not having to rebase it on top of the final version of this patch. -- 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: [RFC/PATCH] Add interpret-trailers builtin
From: Junio C Hamano gits...@pobox.com Christian Couder christian.cou...@gmail.com writes: To append a s-o-b only if there is no s-o-b already, one would need to use: [trailer signoff] key = Signed-off-by: if_exist = dont_append if_missing = append command = echo $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL' But that is insufficient to emulate what we do, no? I.e. append unless the last one is from the same person we are about to add. Yeah, but, with DONT_REPEAT_PREVIOUS, it would be possible using: [trailer signoff] key = Signed-off-by: if_exist = dont_repeat_previous if_missing = append command = echo $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL' Cheers, Christian. -- 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: [PATCH 0/2] thin-pack capability for send-pack/receive-pack
Carlos Martín Nieto c...@elego.de writes: Hi all, This comes as a result of the discussion starting at [0] about git-push assuming that a server will always support thin packs. Most out there in fact do, but this isn't necessarily the case. Some implementations may not have support for it yet, or the server might be running in an environment where it is not feasible for it to try to fill in the missing objects. Jonathan and Duy mentioned that separate patches for receive-pack and send-pack could let us work around adding this at such a late stage, so here they are. The second patch can maybe lie in waiting for a while. I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? -- 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: [RFC/PATCH] Add interpret-trailers builtin
Christian Couder chrisc...@tuxfamily.org writes: From: Junio C Hamano gits...@pobox.com Christian Couder christian.cou...@gmail.com writes: To append a s-o-b only if there is no s-o-b already, one would need to use: [trailer signoff] key = Signed-off-by: if_exist = dont_append if_missing = append command = echo $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL' But that is insufficient to emulate what we do, no? I.e. append unless the last one is from the same person we are about to add. Yeah, but, with DONT_REPEAT_PREVIOUS, it would be possible using: [trailer signoff] key = Signed-off-by: if_exist = dont_repeat_previous if_missing = append command = echo $GIT_COMMITTER_NAME $GIT_COMMITTER_EMAIL' Anything is possible, but possible does not justify it is better way than other possible ways. What are the plausible values for if_missing? If if_missing needs prepend, for example, in addition to append, does it mean if_exist also needs corresponding prepend variant for the value dont_repeat_previous you would give to if_exist? Having two that are seemingly independent configuration does not seem to be helping in reducing complexity (by keeping settings that can be independently set orthogonal, by saying if the other rule decides to add, do we append, prepend, insert at the middle?, for example). How would one differentiate between there is a field with that key and there is a field with that key, value combination with a single if_exist? Add another variable if_exist_exact? -- 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: [PATCH v5 06/10] fast-export: add new --refspec option
Felipe Contreras felipe.contre...@gmail.com writes: On Thu, Oct 31, 2013 at 12:26 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: +test_expect_success 'use refspec' ' + git fast-export --refspec refs/heads/master:refs/heads/foobar master | \ + grep ^commit | sort | uniq actual It feels somewhat redundant that you have to twice say that you are pushing your master, once with --refspec and then the branch name. Is this the best we can do? As this has been discussed before and no other solution came forward, yes. We need to take that no other solution came forward with a grain of salt. After all, this is your itch, and if nobody was interested in helping you (which I think that we both understand entirely plausible, given the recent history), it only means you didn't think of any other solution. I didn't think things through, but at the external UI level, I see a possibility of a nicer way to express the above. In our push refspec (and export is about pushing what we have), a colonless refspec A is a short-hand for A:A, so the traditional git fast-export master can be thought of, in a new world order with a patch that lets you do a ref mapping, a short-hand for an identical mapping: git fast-export master:master It follows that the syntax naturally support git fast-export refs/heads/master:refs/heads/foobar I would think. That approach lets you express ref mapping without a new option --refspec, which goes contrary to existing UI for any commands in Git (I think nobody takes refspec as a value to a dashed-option in the transport; both in fetch and push, they are proper operands, i.e. command line arguments to the command), no? -- 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
(broken ?) output of git diff --color-word
I just commented out few lines, git diff is fine : @@ -144,10 +145,10 @@ StartUML || exit 2 SHARES= if [[ $VICTIMS -eq 1 ]]; then -# SHARES=/tmp -# SHARES=$SHARES /mnt/hostfs -# SHARES=$SHARES /mnt/nfsv2 -# SHARES=$SHARES /mnt/nfsv3 + SHARES=/tmp + SHARES=$SHARES /mnt/hostfs + SHARES=$SHARES /mnt/nfsv2 + SHARES=$SHARES /mnt/nfsv3 # SHARES=$SHARES /mnt/nfsv4 echo $SHARES | grep -q hostfs but git diff --color-words places the # somehow obscure : @@ -144,10 +145,10 @@ StartUML || exit 2 SHARES= if [[ $VICTIMS -eq 1 ]]; then # SHARES=/tmp# SHARES=$SHARES /mnt/hostfs# SHARES=$SHARES /mnt/nfsv2# SHARES=$SHARES /mnt/nfsv3 # SHARES=$SHARES /mnt/nfsv4 echo $SHARES | grep -q hostfs -- MfG/Sincerely Toralf Förster pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3 -- 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: [PATCH 0/2] thin-pack capability for send-pack/receive-pack
On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote: I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. Cheers, cmn -- 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: [PATCH v3] push: Enhance unspecified push default warning
Junio C Hamano gits...@pobox.com writes: That is true, but does it justify giving a misleading information in the advice message? Clearly, yes. Trying to be exhaustive here is not a good idea, we'd end up rewritting the man page, and then users won't read the message because it's too long. Specifically: + When push.default is set to 'matching', git will push all local branches\n + to the remote branches with the same (matching) name. invites those who do not read documentation to mistake it with using an explicit refs/heads/*:refs/heads/* refspec. Yes, but those who want to know the exact behavior should read the doc. That's life. + In Git 2.0 the new push.default of 'simple' will push only the current\n + branch to the same remote branch used by git pull. A push will\n + only succeed if the remote and local branches have the same name.\n while you can see that it is not telling a lie if you read it twice, will only succeed if feels somewhat roundabout. ... push only the current branch back to the branch of the same name, but only if 'git pull' is set to pull from that branch. Otherwise the push will fail. might be an improvement, but I dunno. I do not see much difference actually. I tend to prefer the original version: to me the expected behavior is to make push and pull essentially symetrical, and the fact that it fails if the branch is named differently is a safety feature comming on top of that. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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: [PATCH v5 06/10] fast-export: add new --refspec option
On Wed, Nov 06, 2013 at 01:00:42PM -0800, Junio C Hamano wrote: I didn't think things through, but at the external UI level, I see a possibility of a nicer way to express the above. In our push refspec (and export is about pushing what we have), a colonless refspec A is a short-hand for A:A, so the traditional git fast-export master can be thought of, in a new world order with a patch that lets you do a ref mapping, a short-hand for an identical mapping: git fast-export master:master It follows that the syntax naturally support git fast-export refs/heads/master:refs/heads/foobar I would think. That approach lets you express ref mapping without a new option --refspec, which goes contrary to existing UI for any commands in Git (I think nobody takes refspec as a value to a dashed-option in the transport; both in fetch and push, they are proper operands, i.e. command line arguments to the command), no? I think that is much nicer for the simple cases, but how do we handle more complex rev expressions? One can say: git fast-export master ^origin or even: git fast-export origin..master The ^origin is not a refspec, and finding the refspec in the dot-expression would involve parsing it into two components. I think you can come up with a workable system by parsing the arguments as revision specifiers and then applying some rules. E.g., a positive ref foo is a refspec foo:foo, but negative ^bar does not impact refspecs at all, and the same rules are applied for bar..foo. There is a syntactic conflict where foo:bar would be interpreted as a tree:path by the revision code, though, whereas in this context it means a refspec. So I think it is possible to go that route (and perhaps preferable, even, because it keeps the simple and common cases easy for the user), but the implementation is not as simple as just treating the arguments as refspecs. -Peff -- 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: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Hi, Christian Couder wrote: Now has_suffix() returns 1 when the suffix is present and 0 otherwise. Ok. My only worry is that the function is less discoverable since its name is so different from prefixcmp(), which might cause someone to invent yet another postfixcmp. The old name followed the pattern anything-cmp(), which suggests a general comparison function suitable for e.g. sorting objects. But this was not the case for suffixcmp(). It's not clear to me that prefixcmp() is usable for sorting objects, either. Shouldn't it get the same treatment? Except for that concern, the patch looks good. If some day we invent a type for 4-byte-aligned object names, it might make sense to do something similar to hashcmp, distinguishing between hashcmp for use where ordering is important and something like hash_eq when checking for equality (since I suspect the latter can be made faster). Thanks, Jonathan -- 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: [PATCH 0/2] thin-pack capability for send-pack/receive-pack
Carlos Martín Nieto c...@elego.de writes: On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote: I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. OK, I agree that it sounds quite niche-y, but it still is sensible. If a receiving end does not want to (this includes it is incapable of doing so, but does not have to be limited to) complete a thin pack, the series will give it such an option in the longer term. 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: [PATCH 0/2] thin-pack capability for send-pack/receive-pack
On Wed, Nov 06, 2013 at 02:25:50PM -0800, Junio C Hamano wrote: Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. OK, I agree that it sounds quite niche-y, but it still is sensible. If a receiving end does not want to (this includes it is incapable of doing so, but does not have to be limited to) complete a thin pack, the series will give it such an option in the longer term. I wonder if we want to make the flag go in the opposite direction, then. Right now we have no flag, and we assume the other side can handle a thin pack. If we add a thin flag, then the timeline is roughly: 1. Receive-pack starts advertising thin. 2. Send-pack cannot assume lack of thin means the other side cannot handle thin (it might just be an older receive-pack), and keeps sending thin packs. [time passes] 3. Send-pack can safely assume that every server has learned thin and can assume that lack of thin means the server does not want a thin pack. In other words, the benefit happens at step 3, and we do not get any effect until some long assumption time passes. If we instead introduced no-thin, it is more like: 1. Receive-pack starts advertising no-thin (as dictated by circumstances, as Carlos describes). 2. Send-pack which does not understand no-thin will ignore it and send a thin pack. This is the same as now, and the same as step 2 above. 3. An upgraded send-pack will understand no-thin and do as the server asks. So an upgraded client and server can start cooperating immediately, and we do not have to wait for the long assumption time to pass before applying the second half. It is tempting to think about a thin flag because that would be the natural way to have implemented it from the very beginning. But it is not the beginning, and the negative flag is the only way at this point to say if you understand this, please behave differently than we used to (because the status quo is send a thin pack, whether I said it was OK or not). -Peff -- 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: [PATCH v3] push: Enhance unspecified push default warning
Junio C Hamano gits...@pobox.com writes: Also applying this will have an unpleasant fallout to merging the endgame patch b2ed944a (push: switch default from matching to simple, 2013-01-04). The added text needs to be corrected with an evil merge. I'd prefer to having worry about such a fallout only once. Which arguably we already did when we came up with the current message, so I am fairly annoyed by this patch coming loong after we concluded the original discussion. Sigh X-. I'll worry about this later, as b2ed944a is in 'next' during the feature freeze, and I'd prefer not having to rebase it on top of the final version of this patch. Here is a rebase of the endgame patch, on top of the result of applying Greg's patch (you have to fix the line-wrapping in the message, though). The only change from the version we have been cooking since January is the message in builtin/push.c. I haven't checked if the result merges cleanly to other topics in flight though. It will be quite messy to merge this and Greg's patch to anything past 3153a9e8 (Merge branch 'jc/push-2.0-default-to-simple' into next, 2013-10-28), which already has the original endgame patch, so I'll postpone it until later (I still need to tag 1.8.5-rc1 today). Thanks. -- 8 -- Subject: [PATCH] push: switch default from matching to simple We promised to change the behaviour of lazy git push [there] that does not say what to push on the command line from matching to simple in Git 2.0. This finally flips that bit. Helped-by: Matthieu Moy matthieu@imag.fr Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/config.txt | 14 -- Documentation/git-push.txt | 10 ++ advice.c | 2 -- advice.h | 1 - builtin/push.c | 37 ++--- 5 files changed, 20 insertions(+), 44 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index ab26963..bb45969 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -142,19 +142,13 @@ advice.*:: -- pushUpdateRejected:: Set this variable to 'false' if you want to disable - 'pushNonFFCurrent', 'pushNonFFDefault', + 'pushNonFFCurrent', 'pushNonFFMatching', 'pushAlreadyExists', 'pushFetchFirst', and 'pushNeedsForce' simultaneously. pushNonFFCurrent:: Advice shown when linkgit:git-push[1] fails due to a non-fast-forward update to the current branch. - pushNonFFDefault:: - Advice to set 'push.default' to 'upstream' or 'current' - when you ran linkgit:git-push[1] and pushed 'matching - refs' by default (i.e. you did not provide an explicit - refspec, and no 'push.default' configuration was set) - and it resulted in a non-fast-forward error. pushNonFFMatching:: Advice shown when you ran linkgit:git-push[1] and pushed 'matching refs' explicitly (i.e. you used ':', or @@ -1929,7 +1923,7 @@ When pushing to a remote that is different from the remote you normally pull from, work as `current`. This is the safest option and is suited for beginners. + -This mode will become the default in Git 2.0. +This mode has become the default in Git 2.0. * `matching` - push all branches having the same name on both ends. This makes the repository you are pushing to remember the set of @@ -1948,8 +1942,8 @@ suitable for pushing into a shared central repository, as other people may add new branches there, or update the tip of existing branches outside your control. + -This is currently the default, but Git 2.0 will change the default -to `simple`. +This used to be the default, but not since Git 2.0 (`simple` is the +new default). -- diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 9eec740..5553f99 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -78,8 +78,8 @@ the local side, the remote side is updated if a branch of the same name already exists on the remote side. --all:: - Instead of naming each ref to push, specifies that all - refs under `refs/heads/` be pushed. + Push all branches (i.e. refs under `refs/heads/`); cannot be + used with other refspec. --prune:: Remove remote branches that don't have a local counterpart. For example @@ -437,8 +437,10 @@ Examples configured for the current branch). `git push origin`:: - Without additional configuration, works like - `git push origin :`. + Without additional configuration, pushes the current branch to + the configured upstream (`remote.origin.merge` configuration + variable) if it has the same name as the current branch, and + errors out without pushing otherwise. + The default behavior of this command when
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
On 06.11.2013, at 23:17, Jonathan Nieder jrnie...@gmail.com wrote: Hi, Christian Couder wrote: Now has_suffix() returns 1 when the suffix is present and 0 otherwise. Ok. My only worry is that the function is less discoverable since its name is so different from prefixcmp(), which might cause someone to invent yet another postfixcmp. Well, that can always happen, no matter what, can't it? Though personally I wouldn't mind if there was an has_prefix instead or in parallel to prefixcmp. The old name followed the pattern anything-cmp(), which suggests a general comparison function suitable for e.g. sorting objects. But this was not the case for suffixcmp(). It's not clear to me that prefixcmp() is usable for sorting objects, either. Shouldn't it get the same treatment? Well, unlike suffixcmp, it is transitive, so it could be used for sorting. Whether doing that would be sensible is another question, though :-). For clarity, it might indeed be better to also change prefixcmp to has_prefix(), and if some code pops up in the future that needs something like the current prefixcmp, it can still be added back. signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Max Horn wrote: Well, unlike suffixcmp, it is transitive, so it could be used for sorting. It is not antisymmetric. prefixcmp(foo, foobar) 0 prefixcmp(foobar, foo) == 0 I can see how it's possible to care about the sign of the return value, but it's equally possible to care about the sign from suffixcmp. Neither is suitable as a function for passing to qsort. Hoping that clarifies, Jonathan -- 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
прекраснейший тренинг для глаз
мыслите являться Зрячим? мыслите являться Зрячим? Зрячим являться бессменно прекрасно. http://goo.gl/sGzLoe -- 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: [PATCH 0/2] thin-pack capability for send-pack/receive-pack
On Wed, Nov 6, 2013 at 1:41 PM, Carlos Martín Nieto c...@elego.de wrote: On Wed, 2013-11-06 at 12:32 -0800, Junio C Hamano wrote: I'll queue these for now, but I doubt the wisdom of this series, given that the ship has already sailed long time ago. Currently, no third-party implementation of a receiving end can accept thin push, because thin push is not a capability that needs to be checked by the current clients. People will have to wait until the clients with 2/2 patch are widely deployed before starting to use such a receiving end that is incapable of thin push. Wouldn't the world be a better place if instead they used that time waiting to help such a third-party receiving end to implement thin push support? Support in the code isn't always enough. The particular case that brought this on is one where the index-pack implementation can deal with thin packs just fine. This particular service takes the pack which the client sent and does post-processing on it to store it elsewhere. During the receive-pack equivalent, there is no git object db that it can query for the missing base objects. I realise this is pretty a unusual situation. How... odd? At Google we have made effort to ensure servers can accept thin packs, even though its clearly easier to accept non-thin, because clients in the wild already send thin packs and changing the deployed clients is harder than implementing the existing protocol. If the server can't complete the pack, I guess this also means the client cannot immediately fetch from the server it just pushed to? -- 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: [PATCH v3] push: Enhance unspecified push default warning
Matthieu Moy wrote: Junio C Hamano gits...@pobox.com writes: Specifically: + When push.default is set to 'matching', git will push all local branches\n + to the remote branches with the same (matching) name. invites those who do not read documentation to mistake it with using an explicit refs/heads/*:refs/heads/* refspec. Yes, but those who want to know the exact behavior should read the doc. That's life. Surely we can do better? For example: When push.default is set to 'matching', git will push local branches to remote branches that already exist with the same (matching) name. + In Git 2.0 the new push.default of 'simple' will push only the current\n + branch to the same remote branch used by git pull. A push will\n + only succeed if the remote and local branches have the same name.\n while you can see that it is not telling a lie if you read it twice, will only succeed if feels somewhat roundabout. ... push only the current branch back to the branch of the same name, but only if 'git pull' is set to pull from that branch. Otherwise the push will fail. might be an improvement, but I dunno. I do not see much difference actually. I tend to prefer the original version: to me the expected behavior is to make push and pull essentially symetrical, and the fact that it fails if the branch is named differently is a safety feature comming on top of that. Perhaps: In Git 2.0 (or now, if push.default is set to 'simple'), git will behave more conservatively by pushing only the current branch to the corresponding remote branch used by git pull, and only if the remote and local branches have the same name. Except that forgets the exception having to do with triangular workflows. So maybe: In Git 2.0, git will default to a more conservative 'simple' behavior that only pushes the current branch. Hope that helps, Jonathan -- 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: [PATCH 0/2] thin-pack capability for send-pack/receive-pack
On Wed, Nov 6, 2013 at 2:54 PM, Jeff King p...@peff.net wrote: If we instead introduced no-thin, it is more like: 1. Receive-pack starts advertising no-thin (as dictated by circumstances, as Carlos describes). 2. Send-pack which does not understand no-thin will ignore it and send a thin pack. This is the same as now, and the same as step 2 above. 3. An upgraded send-pack will understand no-thin and do as the server asks. So an upgraded client and server can start cooperating immediately, and we do not have to wait for the long assumption time to pass before applying the second half. It is tempting to think about a thin flag because that would be the natural way to have implemented it from the very beginning. But it is not the beginning, and the negative flag is the only way at this point to say if you understand this, please behave differently than we used to (because the status quo is send a thin pack, whether I said it was OK or not). I think the only sane option at this point is a no-thin flag, or just require servers that want to be wire compatible to accept thin packs. -- 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: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Am 07.11.2013 um 00:28 schrieb Jonathan Nieder jrnie...@gmail.com: Max Horn wrote: Well, unlike suffixcmp, it is transitive, so it could be used for sorting. It is not antisymmetric. prefixcmp(foo, foobar) 0 prefixcmp(foobar, foo) == 0 Right! I wasn't thinkinh :-( I can see how it's possible to care about the sign of the return value, but it's equally possible to care about the sign from suffixcmp. Neither is suitable as a function for passing to qsort. Yeah, so then I'd be for changing rhis one to has_prefix. Hoping that clarifies, Jonathan -- 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
[ANNOUNCE] Git v1.8.5-rc1
A release candidate Git v1.8.5-rc1 is now available for testing at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: 005ed6e0e91043d1d9da9a47f0235fdac240269a git-1.8.5.rc1.tar.gz 542586fc562acb88aec807cd27ae0dc0d4d507f7 git-htmldocs-1.8.5.rc1.tar.gz 5d7f057c9b5bc4d1b610ce18b986b28320751774 git-manpages-1.8.5.rc1.tar.gz The following public repositories all have a copy of the v1.8.5-rc1 tag and the master branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Git v1.8.5 Release Notes (draft) Backward compatibility notes (for Git 2.0) -- When git push [$there] does not say what to push, we have used the traditional matching semantics so far (all your branches were sent to the remote as long as there already are branches of the same name over there). In Git 2.0, the default will change to the simple semantics that pushes: - only the current branch to the branch with the same name, and only when the current branch is set to integrate with that remote branch, if you are pushing to the same remote as you fetch from; or - only the current branch to the branch with the same name, if you are pushing to a remote that is not where you usually fetch from. Use the user preference configuration variable push.default to change this. If you are an old-timer who is used to the matching semantics, you can set the variable to matching to keep the traditional behaviour. If you want to live in the future early, you can set it to simple today without waiting for Git 2.0. When git add -u (and git add -A) is run inside a subdirectory and does not specify which paths to add on the command line, it will operate on the entire tree in Git 2.0 for consistency with git commit -a and other commands. There will be no mechanism to make plain git add -u behave like git add -u .. Current users of git add -u (without a pathspec) should start training their fingers to explicitly say git add -u . before Git 2.0 comes. A warning is issued when these commands are run without a pathspec and when you have local changes outside the current directory, because the behaviour in Git 2.0 will be different from today's version in such a situation. In Git 2.0, git add path will behave as git add -A path, so that git add dir/ will notice paths you removed from the directory and record the removal. Versions before Git 2.0, including this release, will keep ignoring removals, but the users who rely on this behaviour are encouraged to start using git add --ignore-removal path now before 2.0 is released. The default prefix for git svn will change in Git 2.0. For a long time, git svn created its remote-tracking branches directly under refs/remotes, but it will place them under refs/remotes/origin/ unless it is told otherwise with its --prefix option. Updates since v1.8.4 Foreign interfaces, subsystems and ports. * git-svn used with SVN 1.8.0 when talking over https:// connection dumped core due to a bug in the serf library that SVN uses. Work it around on our side, even though the SVN side is being fixed. * On MacOS X, we detected if the filesystem needs the pre-composed unicode strings workaround, but did not automatically enable it. Now we do. * remote-hg remote helper misbehaved when interacting with a local Hg repository relative to the home directory, e.g. clone hg::~/there. * imap-send ported to OS X uses Apple's security framework instead of OpenSSL one. * Subversion 1.8.0 that was recently released breaks older subversion clients coming over http/https in various ways. * git fast-import treats an empty path given to ls as the root of the tree. UI, Workflows Features * xdg-open can be used as a browser backend for git web-browse (hence to show git help -w output), when available. * git grep and git show pays attention to --textconv option when these commands are told to operate on blob objects (e.g. git grep -e pattern HEAD:Makefile). * git replace helper no longer allows an object to be replaced with another object of a different type to avoid confusion (you can still manually craft such replacement using git update-ref, as an escape hatch). * git status no longer prints dirty status information for submodules for which submodule.$name.ignore is set to all. * git rebase -i honours core.abbrev when preparing the insn sheet for editing. * git status during a cherry-pick shows what original commit is being picked. * Instead of typing four capital letters HEAD, you can say @ now, e.g. git log
What's cooking in git.git (Nov 2013, #02; Wed, 6)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The first release candidate 1.8.5-rc1 has been tagged. As promised/requested, the final steps for 2.0 are in 'next'; they, together with a handful topics that have been merged to 'next' fairly recently, will _not_ be part of the upcoming 1.8.5 release, but will be carried over in 'next' to the next cycle. There is a proposed rewording of advice message from git push patch, which is tentatively queued near the tip of 'pu' for now; it would be nice to get a few more sets of eyeballs. I am not sure if we should merge it before the 1.8.5 final, yet (we have i18n to worry about, among other things). You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to master] * ak/cvsserver-stabilize-use-of-hash-keys (2013-10-30) 1 commit (merged to 'next' on 2013-11-01 at cc3b890) + cvsserver: Determinize output to combat Perl 5.18 hash randomization Perl portability fix. * bw/solaris-sed-tr-test-portability (2013-11-04) 3 commits (merged to 'next' on 2013-11-04 at c2c09e28) + t4015: simplify sed command that is not even seen by sed (merged to 'next' on 2013-11-01 at f04be9a) + Avoid difference in tr semantics between System V and BSD + Change sed i\ usage to something Solaris' sed can handle Shell script portability fix. * fc/trivial (2013-10-31) 13 commits (merged to 'next' on 2013-11-04 at c608324) + setup: trivial style fixes + run-command: trivial style fixes + diff: trivial style fix + revision: trivial style fixes + pretty: trivial style fix + describe: trivial style fixes + transport-helper: trivial style fix + sha1-name: trivial style cleanup + branch: trivial style fix + revision: add missing include + doc/pull: clarify the illustrations + t: replace pulls with merges + merge: simplify ff-only option A random collection of style fixes and minor doc updates. * jk/duplicate-objects-in-packs (2013-10-31) 1 commit (merged to 'next' on 2013-11-01 at 8951339) + Fix '\%o' for printf from coreutils Test fixup to a topic recently graduated. * jk/subtree-install-fix (2013-10-30) 1 commit (merged to 'next' on 2013-11-01 at 531bd79) + subtree: add makefile target for html docs We did not generate HTML version of documentation to git subtree in contrib/. * jk/wrap-perl-used-in-tests (2013-10-29) 2 commits (merged to 'next' on 2013-11-01 at 73444c5) + t: use perl instead of $PERL_PATH where applicable + t: provide a perl() function which uses $PERL_PATH Perl portability fix. * jn/test-prereq-perl-doc (2013-10-28) 1 commit (merged to 'next' on 2013-11-01 at 4d4a8b4) + t/README: tests can use perl even with NO_PERL The interaction between use of Perl in our test suite and NO_PERL has been clarified a bit. * sc/doc-howto-dumb-http (2013-10-28) 1 commit (merged to 'next' on 2013-11-01 at a734b6e) + doc/howto: warn about (dumb)http server document being too old An ancient How-To on serving Git repositories on an HTTP server lacked a warning that it has been mostly superseded with more modern way. * vd/doc-unpack-objects (2013-11-01) 2 commits (merged to 'next' on 2013-11-01 at 443d0f4) + Documentation: pack-file is not literal in unpack-objects + Documentation: restore a space in unpack-objects usage The synopsis section of git unpack-objects documentation has been clarified a bit. -- [New Topics] * cc/remote-remove-redundant-postfixcmp (2013-11-06) 2 commits (merged to 'next' on 2013-11-06 at 7b45219) + Rename suffixcmp() to has_suffix() and invert its result (merged to 'next' on 2013-11-04 at 6408502) + builtin/remote: remove postfixcmp() and use suffixcmp() instead Minor code clean-up. Can wait in 'next'. * tb/clone-ssh-with-colon-for-port (2013-11-04) 1 commit . git clone: is an URL local or ssh Still being reworked. * cn/thin-push-capability (2013-11-06) 2 commits - send-pack: only send a thin pack if the server supports it - receive-pack: advertise thin-pack Peff had a good suggestion to control this by expressing what the receiving end wants in a more direct way, namely to advertise a 'no-thin' trait in the capability list. * nd/wt-status-align-i18n (2013-11-06) 1 commit - wt-status: take the alignment burden off translators An attempt to automatically align the names in the git status output, taking the display width of (translated) section labels into account. * nv/commit-gpgsign-config (2013-11-06) 1 commit - Add the commit.gpgsign option to sign all commits Introduce commit.gpgsign configuration variable to force every commit to be GPG signed. Needs tests, perhaps? * sb/sha1-loose-object-info-check-existence (2013-11-06) 1
Re: [PATCH v3 2/2] Rename suffixcmp() to has_suffix() and invert its result
Jonathan Nieder jrnie...@gmail.com writes: The old name followed the pattern anything-cmp(), which suggests a general comparison function suitable for e.g. sorting objects. But this was not the case for suffixcmp(). It's not clear to me that prefixcmp() is usable for sorting objects, either. Shouldn't it get the same treatment? Sounds like a plan for a good follow-up series. If some day we invent a type for 4-byte-aligned object names, it might make sense to do something similar to hashcmp, distinguishing between hashcmp for use where ordering is important and something like hash_eq when checking for equality (since I suspect the latter can be made faster). Interesting. -- 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: [PATCH v5 06/10] fast-export: add new --refspec option
On Wed, Nov 6, 2013 at 3:00 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Thu, Oct 31, 2013 at 12:26 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: +test_expect_success 'use refspec' ' + git fast-export --refspec refs/heads/master:refs/heads/foobar master | \ + grep ^commit | sort | uniq actual It feels somewhat redundant that you have to twice say that you are pushing your master, once with --refspec and then the branch name. Is this the best we can do? As this has been discussed before and no other solution came forward, yes. We need to take that no other solution came forward with a grain of salt. After all, this is your itch, and if nobody was interested in helping you (which I think that we both understand entirely plausible, given the recent history), it only means you didn't think of any other solution. Thinking is easy, the problem is not thinking of a solution, the problem is actually doing it. I think we should end world hunger. What a great idea! I didn't think things through, but at the external UI level, I see a possibility of a nicer way to express the above. You already said that before. Talk is cheap. Show me the code. -- Felipe Contreras -- 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: [PATCH 1/3] for-each-ref: introduce %C(...) for color
Junio C Hamano wrote: ... users of for-each-ref format will be _more_ familiar with formats used by for-each-ref, and it would make a lot more sense to keep the syntactic resemblance between existing features to show magic things in for-each-ref and the new feature to show color (which is merely one new magic to the vocabulary in the context of for-each-ref), no? Okay, so what do you suggest in place of %C(...)? -- 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