[PATCH] Change strbuf_read_file() to return ssize_t
It is currently declared to return int, which could overflow for large files. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- This patch is against maint, but it also rebases against master without conflict. I couldn't find any way to exploit this bug. Most callers only use this function for locally-generated files in the first place. And the correct length of the file is available in strbuf::len, so most callers only use the return value for a 0 check. And they don't do anything risky on the error path. strbuf.c | 5 +++-- strbuf.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/strbuf.c b/strbuf.c index 88cafd4..b4da9f5 100644 --- a/strbuf.c +++ b/strbuf.c @@ -481,9 +481,10 @@ int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term) return 0; } -int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint) +ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint) { - int fd, len; + int fd; + ssize_t len; fd = open(path, O_RDONLY); if (fd 0) diff --git a/strbuf.h b/strbuf.h index 1883494..1ea9d0b 100644 --- a/strbuf.h +++ b/strbuf.h @@ -364,7 +364,7 @@ extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint); * Read the contents of a file, specified by its path. The third argument * can be used to give a hint about the file size, to avoid reallocs. */ -extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint); +extern ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint); /** * Read the target of a symbolic link, specified by its path. The third -- 2.1.4 -- 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] git-multimail: update to release 1.1.0
The only change is a bugfix: the SMTP mailer was not working with Python 2.4. Signed-off-by: Matthieu Moy matthieu@imag.fr --- contrib/hooks/multimail/CHANGES | 5 + contrib/hooks/multimail/README | 2 +- contrib/hooks/multimail/README.Git | 4 ++-- contrib/hooks/multimail/git_multimail.py | 12 +--- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/contrib/hooks/multimail/CHANGES b/contrib/hooks/multimail/CHANGES index 0b823d8..6bb1230 100644 --- a/contrib/hooks/multimail/CHANGES +++ b/contrib/hooks/multimail/CHANGES @@ -1,3 +1,8 @@ +Release 1.1.1 (bugfix-only release) +=== + +* The SMTP mailer was not working with Python 2.4. + Release 1.1.0 = diff --git a/contrib/hooks/multimail/README b/contrib/hooks/multimail/README index 3a33cb7..e552c90 100644 --- a/contrib/hooks/multimail/README +++ b/contrib/hooks/multimail/README @@ -1,4 +1,4 @@ -git-multimail Version 1.1.0 +git-multimail Version 1.1.1 === .. image:: https://travis-ci.org/git-multimail/git-multimail.svg?branch=master diff --git a/contrib/hooks/multimail/README.Git b/contrib/hooks/multimail/README.Git index 449d36f..f5d59a8 100644 --- a/contrib/hooks/multimail/README.Git +++ b/contrib/hooks/multimail/README.Git @@ -6,10 +6,10 @@ website: https://github.com/git-multimail/git-multimail The version in this directory was obtained from the upstream project -on Jun 18 2015 and consists of the git-multimail subdirectory from +on July 03 2015 and consists of the git-multimail subdirectory from revision -1f0dbb3b60035767889b913df16d9231ecdb8709 refs/tags/1.1.0 +6d6c9eb62a054143322cfaecde3949189c065b46 refs/tags/1.1.1 Please see the README file in this directory for information about how to report bugs or contribute to git-multimail. diff --git a/contrib/hooks/multimail/git_multimail.py b/contrib/hooks/multimail/git_multimail.py index 7cb2b36..c06ce7a 100755 --- a/contrib/hooks/multimail/git_multimail.py +++ b/contrib/hooks/multimail/git_multimail.py @@ -1745,14 +1745,20 @@ class SMTPMailer(Mailer): self.username = smtpuser self.password = smtppass try: +def call(klass, server, timeout): +try: +return klass(server, timeout=timeout) +except TypeError: +# Old Python versions do not have timeout= argument. +return klass(server) if self.security == 'none': -self.smtp = smtplib.SMTP(self.smtpserver, timeout=self.smtpservertimeout) +self.smtp = call(smtplib.SMTP, self.smtpserver, timeout=self.smtpservertimeout) elif self.security == 'ssl': -self.smtp = smtplib.SMTP_SSL(self.smtpserver, timeout=self.smtpservertimeout) +self.smtp = call(smtplib.SMTP_SSL, self.smtpserver, timeout=self.smtpservertimeout) elif self.security == 'tls': if ':' not in self.smtpserver: self.smtpserver += ':587' # default port for TLS -self.smtp = smtplib.SMTP(self.smtpserver, timeout=self.smtpservertimeout) +self.smtp = call(smtplib.SMTP, self.smtpserver, timeout=self.smtpservertimeout) self.smtp.ehlo() self.smtp.starttls() self.smtp.ehlo() -- 2.5.0.rc0.7.ge1edd74.dirty -- 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] index-pack: kill union delta_base to save memory
On Fri, Jul 3, 2015 at 11:51 PM, Junio C Hamano gits...@pobox.com wrote: Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Once we know the number of objects in the input pack, we allocate an array of nr_objects of struct delta_entry. On x86-64, this struct is 32 bytes long. The union delta_base, which is part of struct delta_entry, provides enough space to store either ofs-delta (8 bytes) or ref-delta (20 bytes). Sorry for responding to a patch 7000+ messages ago, but some kind folks at Google were puzzled by this code, and I think they found a small bug. static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) { - struct delta_entry **sorted_by_pos; + struct ref_delta_entry **sorted_by_pos; int i, n = 0; /* @@ -1282,28 +1344,25 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) * resolving deltas in the same order as their position in the pack. */ sorted_by_pos = xmalloc(nr_unresolved * sizeof(*sorted_by_pos)); - for (i = 0; i nr_deltas; i++) { - if (objects[deltas[i].obj_no].real_type != OBJ_REF_DELTA) - continue; - sorted_by_pos[n++] = deltas[i]; - } + for (i = 0; i nr_ref_deltas; i++) + sorted_by_pos[n++] = ref_deltas[i]; qsort(sorted_by_pos, n, sizeof(*sorted_by_pos), delta_pos_compare); You allocate an array of nr_unresolved (which is the sum of nr_ref_deltas and nr_ofs_deltas in the new world order with patch) entries, fill only the first nr_ref_deltas entries of it, and then sort that first n (= nr_ref_deltas) entries. The qsort and the subsequent loop only looks at the first n entries, so nothing is filling or reading these nr_ofs_deltas entres at the end. I do not see any wrong behaviour other than temporary wastage of nr_ofs_deltas pointers that would be caused by this, but this allocation is misleading. Also, the old code before this change had to use 'i' and 'n' because some of the things we see in the (old) deltas[] array we scanned with 'i' would not make it into the sorted_by_pos[] array in the old world order, but now because you have only ref delta in a separate ref_deltas[] array, they increment lockstep. That also puzzled me while re-reading this code. Perhaps something like this is needed? Yeah I can see the confusion when reading the code without checking its history. You probably want to kill the argument nr_unresolved too because it's not needed anymore after your patch. So what's the bug you mentioned? All I got from the above was wastage and confusion, no bug. -- Duy -- 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 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Fri, Jul 3, 2015 at 11:07 AM, Jeff King p...@peff.net wrote: I wondered briefly whether this impacted the keepalives we added to `upload-pack` in 05e9515; those are implemented as 0-byte data packets, which we send during the potentially long counting/delta-compression phase before we send out pack data. It works there because the packets actually contain a single sideband byte, so they are never mistaken for a flush packet. Interesting. I didn't know about 05e9515. This is a great hack. We have similar issues in our server-server system at $DAY_JOB, they use --quiet as no human is watching. So we did a different hack for the same reason. Related, I recently ran into a case where I think we should do the same for pushes. After receiving the pack, `index-pack` may chew on the result for literally minutes (try pushing up the entire linux.git history sometime). We say nothing at all on the wire until we've finished that, run check_everything_connected, and run all hooks. Some clients (or intermediates on the connection) may give up after a few minutes of silence. I think we should have: 1. Some progress eye-candy from the server to tell us that something is happening, and how close we are to finishing (basically index-pack -v). JGit receive-pack has done this for years. We output a progress monitor for the resolving delta phase, and the counting during the graph connectivity check, as JGit being in Java is slow as snot and cannot digest the linux kernel instantly. 2. When progress is disabled, similar keepalive packets saying nope, no output yet. Yea this is a problem so I think JGit ignores the client's request for quiet here and shovels progress messages anyway as a hack to force keep-alive. Never considered the empty side-band message that 05e9515 introduces. For (2), hopefully we can implement it in the same way, and rely on empty sideband-0 packets. I haven't tested it in practice, though (I have some very rough patches for (1) already). sideband-0 is not going to work for JGit clients. JGit clients are strict about the sideband stream being 1,2,3 and fail hard if they get any other stream from the server[1]. [1] https://eclipse.googlesource.com/jgit/jgit/+/master/org.eclipse.jgit/src/org/eclipse/jgit/transport/SideBandInputStream.java#169 -- 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] rebase: return non-zero error code if format-patch fails
Clemens Buchacher clemens.buchac...@intel.com writes: Since e481af06 (rebase: Handle cases where format-patch fails) we notice if format-patch fails and return immediately from git-rebase--am. We save the return value with ret=$?, but then we return $?, which is usually zero in this case. Fix this by returning $ret instead. Sounds sensible. Cc: Andrew Wong andrew.k...@gmail.com Signed-off-by: Clemens Buchacher clemens.buchac...@intel.com Reviewed-by: Jorge Nunes jorge.nu...@intel.com Where was this review made? I may have missed a recent discussion, and that is why I am asking, because Reviewed-by: lines that cannot be validated by going back to the list archive does not add much value. Thanks. --- git-rebase--am.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-rebase--am.sh b/git-rebase--am.sh index f923732..9ae898b 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -78,7 +78,7 @@ else As a result, git cannot rebase them. EOF - return $? + return $ret fi git am $git_am_opt --rebasing --resolvemsg=$resolvemsg \ -- 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 name-rev not accepting abbreviated SHA with --stdin
Sitaram Chamarty sitar...@gmail.com writes: On 06/25/2015 05:41 AM, Junio C Hamano wrote: Sitaram Chamarty sitar...@gmail.com writes: This *is* documented, but I'm curious why this distinction is made. I think it is from mere laziness, and also in a smaller degree coming from an expectation that --stdin would be fed by another script like rev-list where feeding full 40-hex is less work than feeding unique abbreviated prefix. Makes sense; thanks. Maybe if I feel really adventurous I will, one day, look at the code :-) Sorry, but I suspect this is not 100% laziness; it is meant to read text that has object names sprinkled in and output text with object names substituted. I suspect that this was done to prevent a short string that may look like an object name like deadbabe from getting converted into an unrelated commit object name. -- 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] log: add log.follow config option
Matthieu Moy matthieu@grenoble-inp.fr writes: David Turner dtur...@twopensource.com writes: Twitter's history is almost completely linear, so it's useful for us. Since it looks like the patch won't be useful outside of our context, I'll just rewrite it to check the pathspec count, and not upstream it until follow becomes more general. As long as it's an opt-in and that the documentation states the limitations clearly enough, I think it makes sense to me to have this upstream. Perhaps. But at least log -- 1 2 and log -- should not be broken for those that set the configuration. Unless you are counting these as also limitations, that is. -- 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/4] list-object: add get_commit_count function
On Fri, Jul 03, 2015 at 10:49:40AM -0700, Junio C Hamano wrote: Lawrence Siebert lawrencesieb...@gmail.com writes: Moving commit counting from rev-list into list-object which is a step toward letting git log do counting as well. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- No way. Look at the things provided by list-objects.c API. They are not about formatting outputs. printf() calls do not belong there. Moreover, if we are going to provide an abstracted function to show the commit count, we would also need to provide one to _create_ the count. IOW, this get_commit_count, wherever it goes, should be accompanied by the matching code that is put into show_commit in patch 2. -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 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
Jeff King p...@peff.net writes: On Wed, Jul 01, 2015 at 01:39:49PM -0700, Junio C Hamano wrote: There is a slight complication on sending an empty line without any termination, though ;-) The reader that calls packet_read() cannot tell such a payload from a flush packet, I think. *That* may be something we want to document. Usually flush packets are , and an empty data packet is 0004. Or are you talking about some kind of flush inside the pkt-data stream? Neither. At the wire level there is a difference, but the callers of most often used function in pkt-line API, packet_read(), says while (1) { len = packet_read(); if (!len) { /* flush */ break; } ... do things on the len bytes received ... ... and then on to the next packet ... } I think the least intrusive change to the caller side would be to teach packet_read() to keep a static and let the callers do this: while (1) { len = packet_read(); if (!len packet_last_was_flush()) { /* flush */ break; } ... do things on the len bytes received ... ... and then on to the next packet ... } even though that looks very ugly. len = packet_read(..., flag); if (!len (flag PKT_LAST_WAS_FLUSH)) { /* flush */ ... might be better. -- 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 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Fri, Jul 03, 2015 at 11:43:33AM -0700, Shawn Pearce wrote: For (2), hopefully we can implement it in the same way, and rely on empty sideband-0 packets. I haven't tested it in practice, though (I have some very rough patches for (1) already). sideband-0 is not going to work for JGit clients. Er, sorry, mental hiccup. It is 0-length sideband-1 packets that I meant. The same as for upload-pack keepalives. JGit clients are strict about the sideband stream being 1,2,3 and fail hard if they get any other stream from the server[1]. I think git does, too. -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] l10n: de.po: translate 65 new messages
Am 3. Juli 2015 um 17:50 schrieb Ralf Thielow ralf.thie...@gmail.com: Translate 65 new messages came from git.pot update in 64f23b0 (l10n: git.pot: v2.5.0 round 1 (65 new, 15 removed)). Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- #: dir.c:1945 msgid Untracked cache is disabled on this system. -msgstr +msgstr Cache für unversionierten Dateien ist auf diesem System deaktiviert. Just saw this typo, should be unversionierte Dateien. Will be fixed in a reroll. -- 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 3/4] status: give more information during rebase -i
Matthieu Moy matthieu@grenoble-inp.fr writes: I would agree with more strict is it was about rejecting the input (to catch errors), but here we're still accepting it without complaining Yes, by more strict, I meant that I would prefer to keep things we do not understand as intact as possible, while transforming what we do understand into whatever shape we deem appropriate. Actually, there's a hidden benefit in accepting not-well-formatted input: it mimicks the shell equivalent closer, which means that we're close to replacing the shell's collapse_todo_ids and expand_todo_ids in C which would avoid C/shell duplication. ;-) But as I said above, that is a mere would prefer preference, so I can go either way. -- 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] pager: do not leak GIT_PAGER_IN_USE to the pager
On Fri, Jul 03, 2015 at 10:18:45AM -0700, Junio C Hamano wrote: Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that I imagine you mean 2e6c012e here. becomes the upstream of the spawned pager can still tell that we have spawned the pager and decide to do colored output even when its output no longer goes to a terminal (i.e. isatty(1)). But we forgot to clear it from the enviornment of the spawned pager. This is not a problem in a sane world, but if you have a handful of thousands Git users in your organization, somebody is bound to do strange things, e.g. typing !ENTER instead of 'q' to get control back from $LESS. GIT_PAGER_IN_USE is still set in that subshell spawned by less, and all sorts of interesting things starts happening, e.g. git diff | cat starts coloring its output. We can clear the environment variable in the half of the fork that runs the pager to avoid the confusion. I think this is a reasonable fix. I guess it would be possible that some pager would want to react differently depending on the variable, but I could not think of a useful case. And certainly your pager, being the pager itself, can assume that the pager is in use. ;) At the very worst, somebody can set GIT_PAGER=GIT_PAGER_IN_USE=1 my-pager if they truly want to do something bizarre. So, Acked-by: Jeff King p...@peff.net -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 2/2] index-pack: kill union delta_base to save memory
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Once we know the number of objects in the input pack, we allocate an array of nr_objects of struct delta_entry. On x86-64, this struct is 32 bytes long. The union delta_base, which is part of struct delta_entry, provides enough space to store either ofs-delta (8 bytes) or ref-delta (20 bytes). Sorry for responding to a patch 7000+ messages ago, but some kind folks at Google were puzzled by this code, and I think they found a small bug. static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) { - struct delta_entry **sorted_by_pos; + struct ref_delta_entry **sorted_by_pos; int i, n = 0; /* @@ -1282,28 +1344,25 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) * resolving deltas in the same order as their position in the pack. */ sorted_by_pos = xmalloc(nr_unresolved * sizeof(*sorted_by_pos)); - for (i = 0; i nr_deltas; i++) { - if (objects[deltas[i].obj_no].real_type != OBJ_REF_DELTA) - continue; - sorted_by_pos[n++] = deltas[i]; - } + for (i = 0; i nr_ref_deltas; i++) + sorted_by_pos[n++] = ref_deltas[i]; qsort(sorted_by_pos, n, sizeof(*sorted_by_pos), delta_pos_compare); You allocate an array of nr_unresolved (which is the sum of nr_ref_deltas and nr_ofs_deltas in the new world order with patch) entries, fill only the first nr_ref_deltas entries of it, and then sort that first n (= nr_ref_deltas) entries. The qsort and the subsequent loop only looks at the first n entries, so nothing is filling or reading these nr_ofs_deltas entres at the end. I do not see any wrong behaviour other than temporary wastage of nr_ofs_deltas pointers that would be caused by this, but this allocation is misleading. Also, the old code before this change had to use 'i' and 'n' because some of the things we see in the (old) deltas[] array we scanned with 'i' would not make it into the sorted_by_pos[] array in the old world order, but now because you have only ref delta in a separate ref_deltas[] array, they increment lockstep. That also puzzled me while re-reading this code. Perhaps something like this is needed? builtin/index-pack.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 48fa472..d6c48cd 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1334,7 +1334,7 @@ static int delta_pos_compare(const void *_a, const void *_b) static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) { struct ref_delta_entry **sorted_by_pos; - int i, n = 0; + int i; /* * Since many unresolved deltas may well be themselves base objects @@ -1346,12 +1346,12 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) * before deltas depending on them, a good heuristic is to start * resolving deltas in the same order as their position in the pack. */ - sorted_by_pos = xmalloc(nr_unresolved * sizeof(*sorted_by_pos)); + sorted_by_pos = xmalloc(nr_ref_deltas * sizeof(*sorted_by_pos)); for (i = 0; i nr_ref_deltas; i++) - sorted_by_pos[n++] = ref_deltas[i]; - qsort(sorted_by_pos, n, sizeof(*sorted_by_pos), delta_pos_compare); + sorted_by_pos[i] = ref_deltas[i]; + qsort(sorted_by_pos, nr_ref_deltas, sizeof(*sorted_by_pos), delta_pos_compare); - for (i = 0; i n; i++) { + for (i = 0; i nr_ref_deltas; i++) { struct ref_delta_entry *d = sorted_by_pos[i]; enum object_type type; struct base_data *base_obj = alloc_base_data(); -- 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/4] list-object: add get_commit_count function
Lawrence Siebert lawrencesieb...@gmail.com writes: Moving commit counting from rev-list into list-object which is a step toward letting git log do counting as well. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- No way. Look at the things provided by list-objects.c API. They are not about formatting outputs. printf() calls do not belong there. -- 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 3/7] pack-protocol.txt: Mark all LFs in push-cert as required
On Fri, Jul 03, 2015 at 10:45:59AM -0700, Junio C Hamano wrote: Usually flush packets are , and an empty data packet is 0004. Or are you talking about some kind of flush inside the pkt-data stream? Neither. At the wire level there is a difference, but the callers of most often used function in pkt-line API, packet_read(), says while (1) { len = packet_read(); if (!len) { /* flush */ break; } ... do things on the len bytes received ... ... and then on to the next packet ... } Ah, I see. Yeah, that is a problem. The solutions you proposed seem like good workarounds to me, but we are unfortunately stuck with existing clients and servers which did not behave that way. I wondered briefly whether this impacted the keepalives we added to `upload-pack` in 05e9515; those are implemented as 0-byte data packets, which we send during the potentially long counting/delta-compression phase before we send out pack data. It works there because the packets actually contain a single sideband byte, so they are never mistaken for a flush packet. Related, I recently ran into a case where I think we should do the same for pushes. After receiving the pack, `index-pack` may chew on the result for literally minutes (try pushing up the entire linux.git history sometime). We say nothing at all on the wire until we've finished that, run check_everything_connected, and run all hooks. Some clients (or intermediates on the connection) may give up after a few minutes of silence. I think we should have: 1. Some progress eye-candy from the server to tell us that something is happening, and how close we are to finishing (basically index-pack -v). 2. When progress is disabled, similar keepalive packets saying nope, no output yet. For (2), hopefully we can implement it in the same way, and rely on empty sideband-0 packets. I haven't tested it in practice, though (I have some very rough patches for (1) already). -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
CLAIM!!!...
Dear Winner, Your email address attached to ticket number 7-27-31-37-43 WON One Million British Pound in the BINGO LOTTO INTERNATIONAL. Email to (bingo...@aim.com) for claim. Congratulations once again. Yours Faithfully, Mr. David -- 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 v2 08/23] checkout: fix bug with --to and relative HEAD
On Sat, Jul 4, 2015 at 7:17 AM, Eric Sunshine sunsh...@sunshineco.com wrote: Given git checkout --to path HEAD~1, the new worktree's HEAD should begin life at the current branch's HEAD~1, however, it actually ends up at HEAD~2. The happens because: 1. git-checkout resolves HEAD~1 2. to satisfy is_git_directory, prepare_linked_worktree() creates a HEAD in the new worktree with the value of the resolved HEAD~1 3. git-checkout re-invokes itself with the same arguments within the new worktree to populate the worktree 4. the sub git-checkout resolves HEAD~1 relative to its own HEAD, which is the resolved HEAD~1 from the original invocation, resulting unexpectedly and incorrectly in HEAD~2 (relative to the original) Fix this by unconditionally assigning the current worktree's HEAD as the value of the new worktree's HEAD. Good catch! @@ -924,9 +925,11 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, * value would do because this value will be ignored and * replaced at the next (real) checkout. */ This comment any valid value would do.. will be ignored is proved incorrect. + if (!resolve_ref_unsafe(HEAD, 0, rev, NULL)) + die(_(unable to resolve HEAD)); strbuf_reset(sb); strbuf_addf(sb, %s/HEAD, sb_repo.buf); - write_file(sb.buf, 1, %s\n, sha1_to_hex(new-commit-object.sha1)); + write_file(sb.buf, 1, %s\n, sha1_to_hex(rev)); strbuf_reset(sb); strbuf_addf(sb, %s/commondir, sb_repo.buf); write_file(sb.buf, 1, ../..\n); -- Duy -- 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 v2 13/23] worktree: introduce add command
On Sat, Jul 4, 2015 at 7:17 AM, Eric Sunshine sunsh...@sunshineco.com wrote: COMMANDS +add path branch:: + +Check out `branch` into a separate working directory, `path`, creating +`path` if necessary. The new working directory is linked to the current +repository, sharing everything except working directory specific files +such as HEAD, index, etc. If `path` already exists, it must be empty. Side note, must be empty is an implementation limitation. I think the two-way merge employed by git-checkout can deal with dirty path and only perform the checkout if there is no data loss. But we can leave this for later. -- Duy -- 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 v2 16/23] worktree: add -b/-B options
On Sat, Jul 4, 2015 at 7:17 AM, Eric Sunshine sunsh...@sunshineco.com wrote: One of git-worktree's roles is to populate the new worktree, much like git-checkout, and thus, for convenience, ought to support several of the same shortcuts. Toward this goal, add -b/-B options to create a new branch and check it out in the new worktree. There are some other ref manipulation options we can bring over like --orphan and --track. But you can totally leave them out and we can add them back when people actually need them. -- Duy -- 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] l10n: de.po: translate 65 new messages
Translate 65 new messages came from git.pot update in 64f23b0 (l10n: git.pot: v2.5.0 round 1 (65 new, 15 removed)). Signed-off-by: Ralf Thielow ralf.thie...@gmail.com --- po/de.po | 186 --- 1 file changed, 95 insertions(+), 91 deletions(-) diff --git a/po/de.po b/po/de.po index d52184c..8d56d19 100644 --- a/po/de.po +++ b/po/de.po @@ -539,19 +539,19 @@ msgstr #: diff.c:3570 #, c-format msgid Failed to parse --submodule option parameter: '%s' msgstr Fehler beim Parsen des --submodule Optionsparameters: '%s' #: dir.c:1852 msgid failed to get kernel name and information -msgstr +msgstr Fehler beim Sammeln von Namen und Informationen zum Kernel #: dir.c:1945 msgid Untracked cache is disabled on this system. -msgstr +msgstr Cache für unversionierten Dateien ist auf diesem System deaktiviert. #: gpg-interface.c:129 gpg-interface.c:200 msgid could not run gpg. msgstr konnte gpg nicht ausführen #: gpg-interface.c:141 msgid gpg did not accept the data @@ -593,15 +593,15 @@ msgstr Vorhandene Git-Kommandos in '%s' #: help.c:214 msgid git commands available from elsewhere on your $PATH msgstr Vorhandene Git-Kommandos irgendwo in Ihrem $PATH #: help.c:246 msgid These are common Git commands used in various situations: -msgstr +msgstr Allgemeine Git-Kommandos, verwendet in verschiedenen Situationen: #: help.c:311 #, c-format msgid '%s' appears to be a git command, but we were not\n able to execute it. Maybe git-%s is broken? msgstr @@ -1089,50 +1089,51 @@ msgid Internal error msgstr Interner Fehler #: remote.c:1723 remote.c:1766 msgid HEAD does not point to a branch msgstr HEAD zeigt auf keinen Branch #: remote.c:1732 -#, fuzzy, c-format +#, c-format msgid no such branch: '%s' -msgstr Kein solcher Branch '%s' +msgstr Kein solcher Branch: '%s' #: remote.c:1735 -#, fuzzy, c-format +#, c-format msgid no upstream configured for branch '%s' msgstr Kein Upstream-Branch für Branch '%s' konfiguriert. #: remote.c:1741 -#, fuzzy, c-format +#, c-format msgid upstream branch '%s' not stored as a remote-tracking branch -msgstr Upstream-Branch '%s' ist nicht als Remote-Tracking-Branch gespeichert +msgstr Upstream-Branch '%s' nicht als Remote-Tracking-Branch gespeichert #: remote.c:1756 #, c-format msgid push destination '%s' on remote '%s' has no local tracking branch -msgstr +msgstr Ziel für \push\ '%s' auf Remote-Repository '%s' hat keinen lokal +gefolgten Branch #: remote.c:1771 -#, fuzzy, c-format +#, c-format msgid branch '%s' has no remote for pushing msgstr Branch '%s' hat keinen Upstream-Branch gesetzt #: remote.c:1782 #, c-format msgid push refspecs for '%s' do not include '%s' -msgstr +msgstr Push-Refspecs für '%s' beinhalten nicht '%s' #: remote.c:1795 msgid push has no destination (push.default is 'nothing') -msgstr +msgstr kein Ziel für \push\ (push.default ist 'nothing') #: remote.c:1817 msgid cannot resolve 'simple' push to a single destination -msgstr +msgstr kann einzelnes Ziel für \push\ im Modus 'simple' nicht auflösen #: remote.c:2124 #, c-format msgid Your branch is based on '%s', but the upstream is gone.\n msgstr Ihr Branch basiert auf '%s', aber Upstream-Branch wurde entfernt.\n #: remote.c:2128 @@ -1463,17 +1464,17 @@ msgid Can't revert as initial commit msgstr Kann nicht als allerersten Commit einen Revert ausführen. #: sequencer.c:1124 msgid Can't cherry-pick into empty head msgstr Kann nicht als allerersten Commit einen Cherry-Pick ausführen. #: setup.c:243 -#, fuzzy, c-format +#, c-format msgid failed to read %s -msgstr Fehler beim Löschen von %s +msgstr Fehler beim Lesen von %s #: sha1_name.c:453 msgid Git normally never creates a ref that ends with 40 hex characters\n because it will be ignored when you just specify 40-hex. These refs\n may be created by mistake. For example,\n \n @@ -1604,27 +1605,27 @@ msgid no such user msgstr kein solcher Benutzer #: wrapper.c:564 msgid unable to get current working directory msgstr Konnte aktuelles Arbeitsverzeichnis nicht bekommen. #: wrapper.c:575 -#, fuzzy, c-format +#, c-format msgid could not open %s for writing msgstr Konnte '%s' nicht zum Schreiben öffnen. #: wrapper.c:587 -#, fuzzy, c-format +#, c-format msgid could not write to %s -msgstr Konnte nicht nach %s schreiben +msgstr Konnte nicht nach '%s' schreiben. #: wrapper.c:593 -#, fuzzy, c-format +#, c-format msgid could not close %s -msgstr konnte %s nicht parsen +msgstr Konnte '%s' nicht schließen. #: wt-status.c:150 msgid Unmerged paths: msgstr Nicht zusammengeführte Pfade: #: wt-status.c:177 wt-status.c:204 #, c-format @@ -2120,17 +2121,16 @@ msgid Could not open '%s' for writing. msgstr Konnte '%s' nicht zum Schreiben öffnen. #: builtin/add.c:209 msgid Could not write patch msgstr Konnte Patch nicht schreiben #: builtin/add.c:212 -#, fuzzy msgid editing patch failed -msgstr Konnte
Re: [PATCH 00/12] Improve git-am test coverage
On Thu, Jul 2, 2015 at 11:16 AM, Paul Tan pyoka...@gmail.com wrote: Increase test coverage of git-am.sh to help prevent regressions that could arise from the rewrite of git-am.sh to C. This patch series, along with pt/am-foreign, improved test coverage as measured by kcov from 56.5%[1] to 67.3%[2]. No tests for git-am's interactive mode, though, as test_terminal does not seem to attach a pseudo-tty to stdin(?), thus making git-am's test -t 0 check fail. This is part of my GSoC project to rewrite git-am.sh to a C builtin[3]. The whole series looks good to me. [1] http://pyokagan.github.io/git/20150430132408-a75942b//kcov-merged/git-am.eb79278e.html [2] http://pyokagan.github.io/git/20150702173751-2fdae08//kcov-merged/git-am.eb79278e.html [3] https://gist.github.com/pyokagan/1b7b0d1f4dab6ba3cef1 Paul Tan (12): t4150: am.messageid really adds the message id t4150: am fails if index is dirty t4151: am --abort will keep dirty index intact t4150: am refuses patches when paused t4150: am --resolved fails if index has no changes t4150: am --resolved fails if index has unmerged entries t4150: am with applypatch-msg hook t4150: am with pre-applypatch hook t4150: am with post-applypatch hook t4150: tests for am --[no-]scissors t3418: non-interactive rebase --continue with rerere enabled t3901: test git-am encoding conversion t/t3418-rebase-continue.sh | 19 t/t3901-i18n-patch.sh | 62 t/t4150-am.sh | 228 + t/t4151-am-abort.sh| 15 +++ 4 files changed, 324 insertions(+) -- 2.5.0.rc1.81.gfe77482 -- 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] pager: do not leak GIT_PAGER_IN_USE to the pager
Since 2e6c01e2, we export GIT_PAGER_IN_USE so that a process that becomes the upstream of the spawned pager can still tell that we have spawned the pager and decide to do colored output even when its output no longer goes to a terminal (i.e. isatty(1)). But we forgot to clear it from the enviornment of the spawned pager. This is not a problem in a sane world, but if you have a handful of thousands Git users in your organization, somebody is bound to do strange things, e.g. typing !ENTER instead of 'q' to get control back from $LESS. GIT_PAGER_IN_USE is still set in that subshell spawned by less, and all sorts of interesting things starts happening, e.g. git diff | cat starts coloring its output. We can clear the environment variable in the half of the fork that runs the pager to avoid the confusion. Signed-off-by: Junio C Hamano gits...@pobox.com --- * Arguably, git diff | cat that is colored is much minor problem than that the user keep using that subshell from pager without realizing. The user may run more git commands that spawn a pager and do the !ENTER infinite times creating a deep process tree and then logout many number of times. pager.c | 1 + 1 file changed, 1 insertion(+) diff --git a/pager.c b/pager.c index 98b2682..070dc11 100644 --- a/pager.c +++ b/pager.c @@ -78,6 +78,7 @@ void setup_pager(void) argv_array_push(pager_process.env_array, LESS=FRX); if (!getenv(LV)) argv_array_push(pager_process.env_array, LV=-c); + argv_array_push(pager_process.env_array, GIT_PAGER_IN_USE); if (start_command(pager_process)) return; -- 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] --count feature for git shortlog
John Keeping j...@keeping.me.uk writes: On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote: On 2015-06-29 18:46, Lawrence Siebert wrote: I appreciate your help. Okay, That all makes sense. I would note that something like: git shortlog -s $FILENAME: | cut -f 1 | paste -sd+ - | bc seems like it run much faster then: git log --oneline $FILENAME | wc -l How does it compare to `git rev-list -- $FILENAME | wc -l`? Or even `git rev-list --count HEAD -- $FILENAME`. Ahh, OK. I didn't know we already had rev-list --count. Then please disregard the suggestion to add the option to log; it still holds true that the option does not belong to shortlog, but I do think how many changes were made to this path statistics driven by a script should use rev-list plumbing, and if it already has --count option, that is perfect ;-) 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: [PUB]What's cooking in git.git (Jul 2015, #01; Wed, 1)
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: * ad/bisect-terms (2015-06-29) 10 commits ... The bottom part has been quite well cooked. Perhaps split it into two topisc and merge the earlier ones to 'next' before the rest settles. Michael's idea to make 'good/bad' more intelligent does have certain attractiveness ($gname/272867). I think it makes sense to merge the first patches soon: - bisect: don't mix option parsing and non-trivial code - bisect: simplify the addition of new bisect terms - bisect: replace hardcoded bad|good by variables - Documentation/bisect: revise overall content - Documentation/bisect: move getting help section to the end - bisect: correction of typo I have nothing to add on the last ones, but they can cook in pu a bit longer. Do you expect anything from my side? Not at this moment. Thanks for helping this topic move forward. -- 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 name-rev not accepting abbreviated SHA with --stdin
On Fri, Jul 3, 2015 at 6:26 PM, Sitaram Chamarty sitar...@gmail.com wrote: Jokes apart, I'm not sure the chances of *both* those things happening -- an accidental hash-like string in the text *and* it matching an existing hash -- are high enough to bother. If it can be done without too much code, it probably should. To be fair to the original implementor, I think we didn't have an API to ask do we have a committish object with this name? with an abbreviated SHA-1. All we had was do we have an object with this name?. As the only answer the command can give is an exteneded SHA-1 for committish, it is understandable that hitting blobs and trees (which typically are much more numerous than committishes) with false positives would have been a real risk the implementation wanted to avoid. -- 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 v2 18/23] checkout: retire --to option
On Sat, Jul 4, 2015 at 7:17 AM, Eric Sunshine sunsh...@sunshineco.com wrote: Now that git worktree add has achieved user-facing feature-parity with git checkout --to, retire the latter. Move the actual linked worktree creation functionality, prepare_linked_checkout() and its helpers, verbatim from checkout.c to worktree.c. This effectively reverts changes to checkout.c by 529fef2 (checkout: support checking out into a new working directory, 2014-11-30) with the exception of merge_working_tree() and switch_branches() which still require specialized knowledge that a the checkout is occurring in a newly-created linked worktree (signaled to them by the private GIT_CHECKOUT_NEW_WORKTREE environment variable). Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 156 + builtin/worktree.c | 138 --- 2 files changed, 133 insertions(+), 161 deletions(-) If I didn't lose track of changes, --to is still described in git-checkout.txt? -- Duy -- 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 v2 07/23] Documentation/git-worktree: add EXAMPLES section
On Fri, Jul 3, 2015 at 8:17 PM, Eric Sunshine sunsh...@sunshineco.com wrote: Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- +EXAMPLES + +You are middle of a refactoring session and your boss comes in and demands s/middle/in the / +that you fix something immediately. You might typically use +linkgit:git-stash[1] to store your changes away temporarily, however, your +worktree is in such a state of disarray (with new, removed, moved files, +and other bits and pieces strewn around) that you don't want to risk +disturbing any of it. Instead, you create a temporary linked worktree to +make the emergency fix, remove it when done, and then resume your earlier +refactoring session. + + +$ git branch emergency-fix master +$ git checkout --to ../temp emergency-fix +$ pushd ../temp +# ... hack hack hack ... +$ git commit -a -m 'emergency fix for boss' +$ popd +$ rm -rf ../temp +$ git worktree prune + + BUGS Multiple checkout support for submodules is incomplete. It is NOT -- 2.5.0.rc1.197.g417e668 -- 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 v2 04/23] Documentation/git-worktree: add BUGS section
On Sat, Jul 4, 2015 at 7:17 AM, Eric Sunshine sunsh...@sunshineco.com wrote: +git-worktree could provide more automation for tasks currently +performed manually or via other commands, such as: + +- `add` to create a new linked worktree +- `remove` to remove a linked worktree and its administrative files (and + warn if the worktree is dirty) +- `mv` to move or rename a worktree and update its administrative files +- `lock` to prevent automatic pruning of administrative files (for instance, + for a worktree on a portable device) No need to re-roll if this is the only change in the series, but we also need list/ls here. -- Duy -- 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 v2 19/23] checkout: require worktree unconditionally
On Fri, Jul 3, 2015 at 8:17 PM, Eric Sunshine sunsh...@sunshineco.com wrote: In order to allow linked worktree creation via git checkout -b from a s/-b/--to/ bare repository, 3473ad0 (checkout: don't require a work tree when checking out into a new one, 2014-11-30) dropped git-checkout's unconditional NEED_WORK_TREE requirement and instead performed worktree setup conditionally based upon presence or absence of the --to option. Now that --to has been retired and git-checkout is no longer responsible for linked worktree creation, the NEED_WORK_TREE requirement can once again be unconditional. This effectively reverts 3473ad0, except for the tests it added which now check bare repository behavior of git worktree add instead. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 2 -- git.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index b1e68b3..5754554 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1218,8 +1218,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.new_worktree_mode = getenv(GIT_CHECKOUT_NEW_WORKTREE) != NULL; - setup_work_tree(); - if (conflict_style) { opts.merge = 1; /* implied */ git_xmerge_config(merge.conflictstyle, conflict_style, NULL); diff --git a/git.c b/git.c index f227838..21a6398 100644 --- a/git.c +++ b/git.c @@ -383,7 +383,7 @@ static struct cmd_struct commands[] = { { check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE }, { check-mailmap, cmd_check_mailmap, RUN_SETUP }, { check-ref-format, cmd_check_ref_format }, - { checkout, cmd_checkout, RUN_SETUP }, + { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE }, { checkout-index, cmd_checkout_index, RUN_SETUP | NEED_WORK_TREE}, { cherry, cmd_cherry, RUN_SETUP }, -- 2.5.0.rc1.197.g417e668 -- 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 v2 13/23] worktree: introduce add command
The plan is to relocate git checkout --to functionality to git worktree add. As a first step, introduce a bare-bones git-worktree add command along with documentation. At this stage, git worktree add merely invokes git checkout --to behind the scenes, but an upcoming patch will move the actual functionality (checkout.c:prepare_linked_checkout() and its helpers) to worktree.c. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 21 +++-- builtin/worktree.c | 32 2 files changed, 43 insertions(+), 10 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 028bbd9..59191f9 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,13 +9,13 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] +'git worktree add' path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION --- -Manage multiple worktrees attached to the same repository. These are -created by the command `git checkout --to`. +Manage multiple worktrees attached to the same repository. A git repository can support multiple working trees, allowing you to check out more than one branch at a time. With `git checkout --to` a new working @@ -45,6 +45,13 @@ pruning should be suppressed. See section DETAILS for more information. COMMANDS +add path branch:: + +Check out `branch` into a separate working directory, `path`, creating +`path` if necessary. The new working directory is linked to the current +repository, sharing everything except working directory specific files +such as HEAD, index, etc. If `path` already exists, it must be empty. + prune:: Prune working tree information in $GIT_DIR/worktrees. @@ -118,7 +125,7 @@ refactoring session. $ git branch emergency-fix master -$ git checkout --to ../temp emergency-fix +$ git worktree add ../temp emergency-fix $ pushd ../temp # ... hack hack hack ... $ git commit -a -m 'emergency fix for boss' @@ -133,20 +140,14 @@ Multiple checkout support for submodules is incomplete. It is NOT recommended to make multiple checkouts of a superproject. git-worktree could provide more automation for tasks currently -performed manually or via other commands, such as: +performed manually, such as: -- `add` to create a new linked worktree - `remove` to remove a linked worktree and its administrative files (and warn if the worktree is dirty) - `mv` to move or rename a worktree and update its administrative files - `lock` to prevent automatic pruning of administrative files (for instance, for a worktree on a portable device) -SEE ALSO - - -linkgit:git-checkout[1] - GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/worktree.c b/builtin/worktree.c index 2a729c6..b82861e 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -2,8 +2,11 @@ #include builtin.h #include dir.h #include parse-options.h +#include argv-array.h +#include run-command.h static const char * const worktree_usage[] = { + N_(git worktree add path branch), N_(git worktree prune [options]), NULL }; @@ -119,6 +122,33 @@ static int prune(int ac, const char **av, const char *prefix) return 0; } + +static int add(int ac, const char **av, const char *prefix) +{ + struct child_process c; + const char *path, *branch; + struct argv_array cmd = ARGV_ARRAY_INIT; + struct option options[] = { + OPT_END() + }; + + ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (ac != 2) + usage_with_options(worktree_usage, options); + + path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0]; + branch = av[1]; + + argv_array_push(cmd, checkout); + argv_array_pushl(cmd, --to, path, NULL); + argv_array_push(cmd, branch); + + memset(c, 0, sizeof(c)); + c.git_cmd = 1; + c.argv = cmd.argv; + return run_command(c); +} + int cmd_worktree(int ac, const char **av, const char *prefix) { struct option options[] = { @@ -127,6 +157,8 @@ int cmd_worktree(int ac, const char **av, const char *prefix) if (ac 2) usage_with_options(worktree_usage, options); + if (!strcmp(av[1], add)) + return add(ac - 1, av + 1, prefix); if (!strcmp(av[1], prune)) return prune(ac - 1, av + 1, prefix); usage_with_options(worktree_usage, options); -- 2.5.0.rc1.197.g417e668 -- 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 v2 10/23] checkout: prepare_linked_checkout: drop now-unused 'new' argument
The only references to 'new' were folded out by the last two patches. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 0cb81ee..0dcdde2 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -854,8 +854,7 @@ static void remove_junk_on_signal(int signo) raise(signo); } -static int prepare_linked_checkout(const struct checkout_opts *opts, - struct branch_info *new) +static int prepare_linked_checkout(const struct checkout_opts *opts) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; @@ -1299,7 +1298,7 @@ static int checkout_branch(struct checkout_opts *opts, if (opts-new_worktree) { if (!new-commit) die(_(no branch specified)); - return prepare_linked_checkout(opts, new); + return prepare_linked_checkout(opts); } if (!new-commit opts-new_branch) { -- 2.5.0.rc1.197.g417e668 -- 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 v2 23/23] checkout: retire --ignore-other-worktrees in favor of --force
As a safeguard, checking out a branch already checked out by a different worktree is disallowed. This behavior can be overridden with --ignore-other-worktrees, however, this option is neither obvious nor particularly discoverable. As a common safeguard override, --force is more likely to come to mind. Therefore, overload it to also suppress the check for a branch already checked out elsewhere. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- I plopped this patch at the end of the series so that it can be dropped easily if desired since Junio indicated[1] that he would moderately object to this change. [1]: http://thread.gmane.org/gmane.comp.version-control.git/273032/focus=273108 Documentation/git-checkout.txt | 9 +++-- builtin/checkout.c | 8 +++- builtin/worktree.c | 2 +- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 77b7141..41148ce 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -111,6 +111,9 @@ OPTIONS + When checking out paths from the index, do not fail upon unmerged entries; instead, unmerged entries are ignored. ++ +By default, checking out a branch already checked out in another worktree +is disallowed. This overrides that safeguard. --ours:: --theirs:: @@ -232,12 +235,6 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode. specific files such as HEAD, index, etc. See linkgit:git-worktree[1] for a description of linked worktrees. ---ignore-other-worktrees:: - `git checkout` refuses when the wanted ref is already checked - out by another worktree. This option makes it check the ref - out anyway. In other words, the ref can be held by more than one - worktree. - branch:: Branch to checkout; if it refers to a branch (i.e., a name that, when prepended with refs/heads/, is a valid ref), then that diff --git a/builtin/checkout.c b/builtin/checkout.c index 5754554..01c7c30 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -35,7 +35,6 @@ struct checkout_opts { int writeout_stage; int overwrite_ignore; int ignore_skipworktree; - int ignore_other_worktrees; const char *new_branch; const char *new_branch_force; @@ -903,7 +902,8 @@ static void check_linked_checkout(struct branch_info *new, const char *id) strbuf_rtrim(gitdir); } else strbuf_addstr(gitdir, get_git_common_dir()); - die(_('%s' is already checked out at '%s'), new-name, gitdir.buf); + die(_('%s' is already checked out at '%s'; use --force to override), + new-name, gitdir.buf); done: strbuf_release(path); strbuf_release(sb); @@ -1151,7 +1151,7 @@ static int checkout_branch(struct checkout_opts *opts, char *head_ref = resolve_refdup(HEAD, 0, sha1, flag); if (head_ref (!(flag REF_ISSYMREF) || strcmp(head_ref, new-path)) - !opts-ignore_other_worktrees) + !opts-force) check_linked_checkouts(new); free(head_ref); } @@ -1198,8 +1198,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) N_(do not limit pathspecs to sparse entries only)), OPT_HIDDEN_BOOL(0, guess, dwim_new_local_branch, N_(second guess 'git checkout no-such-branch')), - OPT_BOOL(0, ignore-other-worktrees, opts.ignore_other_worktrees, -N_(do not check if another worktree is holding the given ref)), OPT_END(), }; diff --git a/builtin/worktree.c b/builtin/worktree.c index c8c6fa7..8a6c7fa 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -298,7 +298,7 @@ static int add(int ac, const char **av, const char *prefix) argv_array_push(cmd, checkout); if (force) - argv_array_push(cmd, --ignore-other-worktrees); + argv_array_push(cmd, --force); if (new_branch) argv_array_pushl(cmd, -b, new_branch, NULL); if (new_branch_force) -- 2.5.0.rc1.197.g417e668 -- 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 v2 20/23] worktree: extract basename computation to new function
A subsequent patch will also need to compute the basename of the new worktree, so factor out this logic into a new function. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/worktree.c | 29 - 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 7cfaec6..a1d863d 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -152,6 +152,25 @@ static void remove_junk_on_signal(int signo) raise(signo); } +static const char *worktree_basename(const char *path, int *olen) +{ + const char *name; + int len; + + len = strlen(path); + while (len is_dir_sep(path[len - 1])) + len--; + + for (name = path + len - 1; name path; name--) + if (is_dir_sep(*name)) { + name++; + break; + } + + *olen = len; + return name; +} + static int add_worktree(const char *path, const char **child_argv) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; @@ -165,15 +184,7 @@ static int add_worktree(const char *path, const char **child_argv) if (file_exists(path) !is_empty_dir(path)) die(_('%s' already exists), path); - len = strlen(path); - while (len is_dir_sep(path[len - 1])) - len--; - - for (name = path + len - 1; name path; name--) - if (is_dir_sep(*name)) { - name++; - break; - } + name = worktree_basename(path, len); strbuf_addstr(sb_repo, git_path(worktrees/%.*s, (int)(path + len - name), name)); len = sb_repo.len; -- 2.5.0.rc1.197.g417e668 -- 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 v2 18/23] checkout: retire --to option
Now that git worktree add has achieved user-facing feature-parity with git checkout --to, retire the latter. Move the actual linked worktree creation functionality, prepare_linked_checkout() and its helpers, verbatim from checkout.c to worktree.c. This effectively reverts changes to checkout.c by 529fef2 (checkout: support checking out into a new working directory, 2014-11-30) with the exception of merge_working_tree() and switch_branches() which still require specialized knowledge that a the checkout is occurring in a newly-created linked worktree (signaled to them by the private GIT_CHECKOUT_NEW_WORKTREE environment variable). Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 156 + builtin/worktree.c | 138 --- 2 files changed, 133 insertions(+), 161 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 30fe786..b1e68b3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -19,8 +19,6 @@ #include ll-merge.h #include resolve-undo.h #include submodule.h -#include argv-array.h -#include sigchain.h static const char * const checkout_usage[] = { N_(git checkout [options] branch), @@ -51,8 +49,6 @@ struct checkout_opts { struct pathspec pathspec; struct tree *source_tree; - const char *new_worktree; - const char **saved_argv; int new_worktree_mode; }; @@ -255,9 +251,6 @@ static int checkout_paths(const struct checkout_opts *opts, die(_(Cannot update paths and switch to branch '%s' at the same time.), opts-new_branch); - if (opts-new_worktree) - die(_('%s' cannot be used with updating paths), --to); - if (opts-patch_mode) return run_add_interactive(revision, --patch=checkout, opts-pathspec); @@ -825,137 +818,6 @@ static int switch_branches(const struct checkout_opts *opts, return ret || writeout_error; } -static char *junk_work_tree; -static char *junk_git_dir; -static int is_junk; -static pid_t junk_pid; - -static void remove_junk(void) -{ - struct strbuf sb = STRBUF_INIT; - if (!is_junk || getpid() != junk_pid) - return; - if (junk_git_dir) { - strbuf_addstr(sb, junk_git_dir); - remove_dir_recursively(sb, 0); - strbuf_reset(sb); - } - if (junk_work_tree) { - strbuf_addstr(sb, junk_work_tree); - remove_dir_recursively(sb, 0); - } - strbuf_release(sb); -} - -static void remove_junk_on_signal(int signo) -{ - remove_junk(); - sigchain_pop(signo); - raise(signo); -} - -static int prepare_linked_checkout(const char *path, const char **child_argv) -{ - struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; - struct strbuf sb = STRBUF_INIT; - const char *name; - struct stat st; - struct child_process cp; - int counter = 0, len, ret; - unsigned char rev[20]; - - if (file_exists(path) !is_empty_dir(path)) - die(_('%s' already exists), path); - - len = strlen(path); - while (len is_dir_sep(path[len - 1])) - len--; - - for (name = path + len - 1; name path; name--) - if (is_dir_sep(*name)) { - name++; - break; - } - strbuf_addstr(sb_repo, - git_path(worktrees/%.*s, (int)(path + len - name), name)); - len = sb_repo.len; - if (safe_create_leading_directories_const(sb_repo.buf)) - die_errno(_(could not create leading directories of '%s'), - sb_repo.buf); - while (!stat(sb_repo.buf, st)) { - counter++; - strbuf_setlen(sb_repo, len); - strbuf_addf(sb_repo, %d, counter); - } - name = strrchr(sb_repo.buf, '/') + 1; - - junk_pid = getpid(); - atexit(remove_junk); - sigchain_push_common(remove_junk_on_signal); - - if (mkdir(sb_repo.buf, 0777)) - die_errno(_(could not create directory of '%s'), sb_repo.buf); - junk_git_dir = xstrdup(sb_repo.buf); - is_junk = 1; - - /* -* lock the incomplete repo so prune won't delete it, unlock -* after the preparation is over. -*/ - strbuf_addf(sb, %s/locked, sb_repo.buf); - write_file(sb.buf, 1, initializing\n); - - strbuf_addf(sb_git, %s/.git, path); - if (safe_create_leading_directories_const(sb_git.buf)) - die_errno(_(could not create leading directories of '%s'), - sb_git.buf); - junk_work_tree = xstrdup(path); - - strbuf_reset(sb); - strbuf_addf(sb, %s/gitdir, sb_repo.buf); - write_file(sb.buf, 1, %s\n, real_path(sb_git.buf)); -
[PATCH v2 15/23] worktree: add --detach option
One of git-worktree's roles is to populate the new worktree, much like git-checkout, and thus, for convenience, ought to support several of the same shortcuts. Toward this goal, add a --detach option to detach HEAD in the new worktree. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 6 +- builtin/worktree.c | 5 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 552c8e4..96e2142 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] path branch +'git worktree add' [-f] [--detach] path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -65,6 +65,10 @@ OPTIONS is already checked out by another worktree. This option overrides that safeguard. +--detach:: + With `add`, detach HEAD in the new worktree. See DETACHED HEAD in + linkgit:git-checkout[1]. + -n:: --dry-run:: With `prune`, do not remove anything; just report what it would diff --git a/builtin/worktree.c b/builtin/worktree.c index c9d6462..6967369 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -126,11 +126,12 @@ static int prune(int ac, const char **av, const char *prefix) static int add(int ac, const char **av, const char *prefix) { struct child_process c; - int force = 0; + int force = 0, detach = 0; const char *path, *branch; struct argv_array cmd = ARGV_ARRAY_INIT; struct option options[] = { OPT__FORCE(force, N_(checkout branch even if already checked out in other worktree)), + OPT_BOOL(0, detach, detach, N_(detach HEAD at named commit)), OPT_END() }; @@ -145,6 +146,8 @@ static int add(int ac, const char **av, const char *prefix) argv_array_pushl(cmd, --to, path, NULL); if (force) argv_array_push(cmd, --ignore-other-worktrees); + if (detach) + argv_array_push(cmd, --detach); argv_array_push(cmd, branch); memset(c, 0, sizeof(c)); -- 2.5.0.rc1.197.g417e668 -- 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 v2 22/23] worktree: add: auto-vivify new branch when branch is omitted
As a convenience, when branch is omitted from git worktree path branch and neither -b nor -B used, automatically create a new branch named after path, as if -b $(basename path) was specified. Thus, git worktree add ../hotfix creates a new branch named hotfix and associates it with new worktree ../hotfix. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 8 ++-- builtin/worktree.c | 8 ++-- t/t2025-worktree-add.sh| 7 +++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 5ca11f6..938bdab 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] [--detach] [-b new-branch] path branch +'git worktree add' [-f] [--detach] [-b new-branch] path [branch] 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -45,12 +45,16 @@ pruning should be suppressed. See section DETAILS for more information. COMMANDS -add path branch:: +add path [branch]:: Check out `branch` into a separate working directory, `path`, creating `path` if necessary. The new working directory is linked to the current repository, sharing everything except working directory specific files such as HEAD, index, etc. If `path` already exists, it must be empty. ++ +If `branch` is omitted and neither `-b` nor `-B` used, then, as a +convenience, a new branch rooted at HEAD is created automatically, as if +`-b $(basename path)` was specified. prune:: diff --git a/builtin/worktree.c b/builtin/worktree.c index bf634a6..c8c6fa7 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -286,12 +286,16 @@ static int add(int ac, const char **av, const char *prefix) die(_(-b and -B are mutually exclusive)); if (ac 1 || ac 2) usage_with_options(worktree_usage, options); - if (ac 2 !new_branch !new_branch_force) - usage_with_options(worktree_usage, options); path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0]; branch = ac 2 ? HEAD : av[1]; + if (ac 2 !new_branch !new_branch_force) { + int n; + const char *s = worktree_basename(path, n); + new_branch = xstrndup(s, n); + } + argv_array_push(cmd, checkout); if (force) argv_array_push(cmd, --ignore-other-worktrees); diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh index 95a1141..59d73ff 100755 --- a/t/t2025-worktree-add.sh +++ b/t/t2025-worktree-add.sh @@ -147,4 +147,11 @@ test_expect_success 'add -b with branch omitted' ' test_cmp expected actual ' +test_expect_success 'add with branch omitted' ' + git rev-parse HEAD expected + git worktree add wiffle/bat + git rev-parse bat actual + test_cmp expected actual +' + test_done -- 2.5.0.rc1.197.g417e668 -- 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 v2 14/23] worktree: add --force option
By default, git worktree add refuses to create a new worktree when the requested branch is already checked out elsewhere. Add a --force option to override this safeguard. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 8 +++- builtin/worktree.c | 6 +- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 59191f9..552c8e4 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' path branch +'git worktree add' [-f] path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -59,6 +59,12 @@ Prune working tree information in $GIT_DIR/worktrees. OPTIONS --- +-f:: +--force:: + By default, `add` refuses to create a new worktree when `branch` + is already checked out by another worktree. This option overrides + that safeguard. + -n:: --dry-run:: With `prune`, do not remove anything; just report what it would diff --git a/builtin/worktree.c b/builtin/worktree.c index b82861e..c9d6462 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -6,7 +6,7 @@ #include run-command.h static const char * const worktree_usage[] = { - N_(git worktree add path branch), + N_(git worktree add [options] path branch), N_(git worktree prune [options]), NULL }; @@ -126,9 +126,11 @@ static int prune(int ac, const char **av, const char *prefix) static int add(int ac, const char **av, const char *prefix) { struct child_process c; + int force = 0; const char *path, *branch; struct argv_array cmd = ARGV_ARRAY_INIT; struct option options[] = { + OPT__FORCE(force, N_(checkout branch even if already checked out in other worktree)), OPT_END() }; @@ -141,6 +143,8 @@ static int add(int ac, const char **av, const char *prefix) argv_array_push(cmd, checkout); argv_array_pushl(cmd, --to, path, NULL); + if (force) + argv_array_push(cmd, --ignore-other-worktrees); argv_array_push(cmd, branch); memset(c, 0, sizeof(c)); -- 2.5.0.rc1.197.g417e668 -- 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 v2 19/23] checkout: require worktree unconditionally
In order to allow linked worktree creation via git checkout -b from a bare repository, 3473ad0 (checkout: don't require a work tree when checking out into a new one, 2014-11-30) dropped git-checkout's unconditional NEED_WORK_TREE requirement and instead performed worktree setup conditionally based upon presence or absence of the --to option. Now that --to has been retired and git-checkout is no longer responsible for linked worktree creation, the NEED_WORK_TREE requirement can once again be unconditional. This effectively reverts 3473ad0, except for the tests it added which now check bare repository behavior of git worktree add instead. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 2 -- git.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index b1e68b3..5754554 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1218,8 +1218,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.new_worktree_mode = getenv(GIT_CHECKOUT_NEW_WORKTREE) != NULL; - setup_work_tree(); - if (conflict_style) { opts.merge = 1; /* implied */ git_xmerge_config(merge.conflictstyle, conflict_style, NULL); diff --git a/git.c b/git.c index f227838..21a6398 100644 --- a/git.c +++ b/git.c @@ -383,7 +383,7 @@ static struct cmd_struct commands[] = { { check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE }, { check-mailmap, cmd_check_mailmap, RUN_SETUP }, { check-ref-format, cmd_check_ref_format }, - { checkout, cmd_checkout, RUN_SETUP }, + { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE }, { checkout-index, cmd_checkout_index, RUN_SETUP | NEED_WORK_TREE}, { cherry, cmd_cherry, RUN_SETUP }, -- 2.5.0.rc1.197.g417e668 -- 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 v2 17/23] tests: worktree: retrofit checkout --to tests for worktree add
With the introduction of git worktree add, git checkout --to is slated for removal. Therefore, retrofit linked worktree creation tests to use git worktree add instead. (The test to check exclusivity of checkout --to and checkout paths is dropped altogether since it becomes meaningless with retirement of checkout --to.) Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- t/{t2025-checkout-to.sh = t2025-worktree-add.sh} | 48 +++ t/t2026-prune-linked-checkouts.sh | 2 +- t/t7410-submodule-checkout-to.sh | 4 +- 3 files changed, 25 insertions(+), 29 deletions(-) rename t/{t2025-checkout-to.sh = t2025-worktree-add.sh} (59%) diff --git a/t/t2025-checkout-to.sh b/t/t2025-worktree-add.sh similarity index 59% rename from t/t2025-checkout-to.sh rename to t/t2025-worktree-add.sh index 0fd731b..192c936 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-worktree-add.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='test git checkout --to' +test_description='test git worktree add' . ./test-lib.sh @@ -8,22 +8,18 @@ test_expect_success 'setup' ' test_commit init ' -test_expect_success 'checkout --to not updating paths' ' - test_must_fail git checkout --to -- init.t -' - -test_expect_success 'checkout --to an existing worktree' ' +test_expect_success 'add an existing worktree' ' mkdir -p existing/subtree - test_must_fail git checkout --detach --to existing master + test_must_fail git worktree add --detach existing master ' -test_expect_success 'checkout --to an existing empty worktree' ' +test_expect_success 'add an existing empty worktree' ' mkdir existing_empty - git checkout --detach --to existing_empty master + git worktree add --detach existing_empty master ' -test_expect_success 'checkout --to refuses to checkout locked branch' ' - test_must_fail git checkout --to zere master +test_expect_success 'add refuses to checkout locked branch' ' + test_must_fail git worktree add zere master ! test -d zere ! test -d .git/worktrees/zere ' @@ -36,9 +32,9 @@ test_expect_success 'checking out paths not complaining about linked checkouts' ) ' -test_expect_success 'checkout --to a new worktree' ' +test_expect_success 'add worktree' ' git rev-parse HEAD expect - git checkout --detach --to here master + git worktree add --detach here master ( cd here test_cmp ../init.t init.t @@ -49,27 +45,27 @@ test_expect_success 'checkout --to a new worktree' ' ) ' -test_expect_success 'checkout --to a new worktree from a subdir' ' +test_expect_success 'add worktree from a subdir' ' ( mkdir sub cd sub - git checkout --detach --to here master + git worktree add --detach here master cd here test_cmp ../../init.t init.t ) ' -test_expect_success 'checkout --to from a linked checkout' ' +test_expect_success 'add from a linked checkout' ' ( cd here - git checkout --detach --to nested-here master + git worktree add --detach nested-here master cd nested-here git fsck ) ' -test_expect_success 'checkout --to a new worktree creating new branch' ' - git checkout --to there -b newmaster master +test_expect_success 'add worktree creating new branch' ' + git worktree add -b newmaster there master ( cd there test_cmp ../init.t init.t @@ -90,7 +86,7 @@ test_expect_success 'die the same branch is already checked out' ' test_expect_success 'not die the same branch is already checked out' ' ( cd here - git checkout --ignore-other-worktrees --to anothernewmaster newmaster + git worktree add --force anothernewmaster newmaster ) ' @@ -101,15 +97,15 @@ test_expect_success 'not die on re-checking out current branch' ' ) ' -test_expect_success 'checkout --to from a bare repo' ' +test_expect_success 'add from a bare repo' ' ( git clone --bare . bare cd bare - git checkout --to ../there2 -b bare-master master + git worktree add -b bare-master ../there2 master ) ' -test_expect_success 'checkout from a bare repo without --to' ' +test_expect_success 'checkout from a bare repo without add' ' ( cd bare test_must_fail git checkout master @@ -129,17 +125,17 @@ test_expect_success 'checkout with grafts' ' EOF git log --format=%s -2 actual test_cmp expected actual - git checkout --detach --to grafted master + git worktree add --detach grafted master git --git-dir=grafted/.git log --format=%s -2 actual test_cmp
[PATCH v2 02/23] Documentation/git-worktree: associate options with commands
git-worktree options affect some worktree commands but not others, but this is not necessarily obvious from the option descriptions. Make this clear by indicating explicitly which commands are affected by which options. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 41103e5..1ac1217 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -28,15 +28,15 @@ OPTIONS -n:: --dry-run:: - Do not remove anything; just report what it would + With `prune`, do not remove anything; just report what it would remove. -v:: --verbose:: - Report all removals. + With `prune`, report all removals. --expire time:: - Only expire unused worktrees older than time. + With `prune`, only expire unused worktrees older than time. SEE ALSO -- 2.5.0.rc1.197.g417e668 -- 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 v2 06/23] Documentation/git-worktree: add high-level 'lock' overview
Due to the (current) absence of a git worktree lock command, locking a worktree's administrative files to prevent automatic pruning is a manual task, necessarily requiring low-level understanding of linked worktree functionality. However, this level of detail does not belong in the high-level DESCRIPTION section, so add a generalized discussion of locking to DESCRIPTION and move the technical information to DETAILS. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 2fdfb3e..410f0b4 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -37,15 +37,11 @@ at least one git command inside the linked working directory (e.g. `git status`) in order to update its administrative files in the repository so that they do not get automatically pruned. -To prevent a $GIT_DIR/worktrees entry from from being pruned (which -can be useful in some situations, such as when the -entry's working tree is stored on a portable device), add a file named -'locked' to the entry's directory. The file contains the reason in -plain text. For example, if a linked working tree's `.git` file points -to `/path/main/.git/worktrees/test-next` then a file named -`/path/main/.git/worktrees/test-next/locked` will prevent the -`test-next` entry from being pruned. See -linkgit:gitrepository-layout[5] for details. +If a linked working tree is stored on a portable device or network share +which is not always mounted, you can prevent its administrative files from +being pruned by creating a file named 'lock' alongside the other +administrative files, optionally containing a plain text reason that +pruning should be suppressed. See section DETAILS for more information. COMMANDS @@ -99,6 +95,16 @@ thumb is do not make any assumption about whether a path belongs to $GIT_DIR or $GIT_COMMON_DIR when you need to directly access something inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. +To prevent a $GIT_DIR/worktrees entry from from being pruned (which +can be useful in some situations, such as when the +entry's working tree is stored on a portable device), add a file named +'locked' to the entry's directory. The file contains the reason in +plain text. For example, if a linked working tree's `.git` file points +to `/path/main/.git/worktrees/test-next` then a file named +`/path/main/.git/worktrees/test-next/locked` will prevent the +`test-next` entry from being pruned. See +linkgit:gitrepository-layout[5] for details. + BUGS Multiple checkout support for submodules is incomplete. It is NOT -- 2.5.0.rc1.197.g417e668 -- 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 v2 03/23] Documentation: move linked worktree description from checkout to worktree
Now that the git-worktree command exists, its documentation page is the natural place for the linked worktree description to reside. Relocate the MULTIPLE WORKING TREES description verbatim from git-checkout.txt to git-worktree.txt. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- When Junio queued v1[1] on 'pu', he changed the second sentence of the first paragraph of the description to say ...a new working tree is created and associated... in place of the original ...a new working tree is associated I wanted this to be a pure documentation- movement patch, so I did not carry over his modification. Moreover, his text is not quite accurate since, although path will be created if missing, path can also be pre-existing, provided that it is an empty directory. Patch 13/23 adds documentation which states this explicitly. [1]: http://thread.gmane.org/gmane.comp.version-control.git/273032 Documentation/git-checkout.txt | 69 ++ Documentation/git-worktree.txt | 62 + 2 files changed, 64 insertions(+), 67 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index ce223e6..77b7141 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -229,8 +229,8 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode. Check out a branch in a separate working directory at `path`. A new working directory is linked to the current repository, sharing everything except working directory - specific files such as HEAD, index... See MULTIPLE WORKING - TREES section for more information. + specific files such as HEAD, index, etc. See + linkgit:git-worktree[1] for a description of linked worktrees. --ignore-other-worktrees:: `git checkout` refuses when the wanted ref is already checked @@ -401,71 +401,6 @@ $ git reflog -2 HEAD # or $ git log -g -2 HEAD -MULTIPLE WORKING TREES --- - -A git repository can support multiple working trees, allowing you to check -out more than one branch at a time. With `git checkout --to` a new working -tree is associated with the repository. This new working tree is called a -linked working tree as opposed to the main working tree prepared by git -init or git clone. A repository has one main working tree (if it's not a -bare repository) and zero or more linked working trees. - -Each linked working tree has a private sub-directory in the repository's -$GIT_DIR/worktrees directory. The private sub-directory's name is usually -the base name of the linked working tree's path, possibly appended with a -number to make it unique. For example, when `$GIT_DIR=/path/main/.git` the -command `git checkout --to /path/other/test-next next` creates the linked -working tree in `/path/other/test-next` and also creates a -`$GIT_DIR/worktrees/test-next` directory (or `$GIT_DIR/worktrees/test-next1` -if `test-next` is already taken). - -Within a linked working tree, $GIT_DIR is set to point to this private -directory (e.g. `/path/main/.git/worktrees/test-next` in the example) and -$GIT_COMMON_DIR is set to point back to the main working tree's $GIT_DIR -(e.g. `/path/main/.git`). These settings are made in a `.git` file located at -the top directory of the linked working tree. - -Path resolution via `git rev-parse --git-path` uses either -$GIT_DIR or $GIT_COMMON_DIR depending on the path. For example, in the -linked working tree `git rev-parse --git-path HEAD` returns -`/path/main/.git/worktrees/test-next/HEAD` (not -`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git -rev-parse --git-path refs/heads/master` uses -$GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`, -since refs are shared across all working trees. - -See linkgit:gitrepository-layout[5] for more information. The rule of -thumb is do not make any assumption about whether a path belongs to -$GIT_DIR or $GIT_COMMON_DIR when you need to directly access something -inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. - -When you are done with a linked working tree you can simply delete it. -The working tree's entry in the repository's $GIT_DIR/worktrees -directory will eventually be removed automatically (see -`gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run -`git worktree prune` in the main or any linked working tree to -clean up any stale entries in $GIT_DIR/worktrees. - -If you move a linked working directory to another file system, or -within a file system that does not support hard links, you need to run -at least one git command inside the linked working directory -(e.g. `git status`) in order to update its entry in $GIT_DIR/worktrees -so that it does not get automatically removed. - -To prevent a $GIT_DIR/worktrees entry from from being pruned (which -can be useful in some situations, such as when the -entry's
[PATCH v2 05/23] Documentation/git-worktree: split technical info from general description
The DESCRIPTION section should provide a high-level overview of linked worktree functionality to bring users up to speed quickly, without overloading them with low-level details, so relocate the technical information to a new DETAILS section. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 70 ++ 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 4fbcdd2..2fdfb3e 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -24,47 +24,18 @@ tree is associated with the repository. This new working tree is called a init or git clone. A repository has one main working tree (if it's not a bare repository) and zero or more linked working trees. -Each linked working tree has a private sub-directory in the repository's -$GIT_DIR/worktrees directory. The private sub-directory's name is usually -the base name of the linked working tree's path, possibly appended with a -number to make it unique. For example, when `$GIT_DIR=/path/main/.git` the -command `git checkout --to /path/other/test-next next` creates the linked -working tree in `/path/other/test-next` and also creates a -`$GIT_DIR/worktrees/test-next` directory (or `$GIT_DIR/worktrees/test-next1` -if `test-next` is already taken). - -Within a linked working tree, $GIT_DIR is set to point to this private -directory (e.g. `/path/main/.git/worktrees/test-next` in the example) and -$GIT_COMMON_DIR is set to point back to the main working tree's $GIT_DIR -(e.g. `/path/main/.git`). These settings are made in a `.git` file located at -the top directory of the linked working tree. - -Path resolution via `git rev-parse --git-path` uses either -$GIT_DIR or $GIT_COMMON_DIR depending on the path. For example, in the -linked working tree `git rev-parse --git-path HEAD` returns -`/path/main/.git/worktrees/test-next/HEAD` (not -`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git -rev-parse --git-path refs/heads/master` uses -$GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`, -since refs are shared across all working trees. - -See linkgit:gitrepository-layout[5] for more information. The rule of -thumb is do not make any assumption about whether a path belongs to -$GIT_DIR or $GIT_COMMON_DIR when you need to directly access something -inside $GIT_DIR. Use `git rev-parse --git-path` to get the final path. - When you are done with a linked working tree you can simply delete it. -The working tree's entry in the repository's $GIT_DIR/worktrees -directory will eventually be removed automatically (see +The working tree's administrative files in the repository (see +DETAILS below) will eventually be removed automatically (see `gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run `git worktree prune` in the main or any linked working tree to -clean up any stale entries in $GIT_DIR/worktrees. +clean up any stale administrative files. If you move a linked working directory to another file system, or within a file system that does not support hard links, you need to run at least one git command inside the linked working directory -(e.g. `git status`) in order to update its entry in $GIT_DIR/worktrees -so that it does not get automatically removed. +(e.g. `git status`) in order to update its administrative files in the +repository so that they do not get automatically pruned. To prevent a $GIT_DIR/worktrees entry from from being pruned (which can be useful in some situations, such as when the @@ -97,6 +68,37 @@ OPTIONS --expire time:: With `prune`, only expire unused worktrees older than time. +DETAILS +--- +Each linked working tree has a private sub-directory in the repository's +$GIT_DIR/worktrees directory. The private sub-directory's name is usually +the base name of the linked working tree's path, possibly appended with a +number to make it unique. For example, when `$GIT_DIR=/path/main/.git` the +command `git checkout --to /path/other/test-next next` creates the linked +working tree in `/path/other/test-next` and also creates a +`$GIT_DIR/worktrees/test-next` directory (or `$GIT_DIR/worktrees/test-next1` +if `test-next` is already taken). + +Within a linked working tree, $GIT_DIR is set to point to this private +directory (e.g. `/path/main/.git/worktrees/test-next` in the example) and +$GIT_COMMON_DIR is set to point back to the main working tree's $GIT_DIR +(e.g. `/path/main/.git`). These settings are made in a `.git` file located at +the top directory of the linked working tree. + +Path resolution via `git rev-parse --git-path` uses either +$GIT_DIR or $GIT_COMMON_DIR depending on the path. For example, in the +linked working tree `git rev-parse --git-path HEAD` returns +`/path/main/.git/worktrees/test-next/HEAD` (not +`/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git +rev-parse
[PATCH v2 08/23] checkout: fix bug with --to and relative HEAD
Given git checkout --to path HEAD~1, the new worktree's HEAD should begin life at the current branch's HEAD~1, however, it actually ends up at HEAD~2. The happens because: 1. git-checkout resolves HEAD~1 2. to satisfy is_git_directory, prepare_linked_worktree() creates a HEAD in the new worktree with the value of the resolved HEAD~1 3. git-checkout re-invokes itself with the same arguments within the new worktree to populate the worktree 4. the sub git-checkout resolves HEAD~1 relative to its own HEAD, which is the resolved HEAD~1 from the original invocation, resulting unexpectedly and incorrectly in HEAD~2 (relative to the original) Fix this by unconditionally assigning the current worktree's HEAD as the value of the new worktree's HEAD. As a side-effect, this change also eliminates a dependence within prepare_linked_checkout() upon 'struct branch_info'. The eventual plan is to relocate git checkout --to functionality to git worktree add, and worktree.c won't have knowledge of 'struct branch_info', so removal of this dependency is a step toward that goal. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 5 - t/t2025-checkout-to.sh | 10 ++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 2079aa4..f5f953d 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -863,6 +863,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, struct stat st; struct child_process cp; int counter = 0, len, ret; + unsigned char rev[20]; if (!new-commit) die(_(no branch specified)); @@ -924,9 +925,11 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, * value would do because this value will be ignored and * replaced at the next (real) checkout. */ + if (!resolve_ref_unsafe(HEAD, 0, rev, NULL)) + die(_(unable to resolve HEAD)); strbuf_reset(sb); strbuf_addf(sb, %s/HEAD, sb_repo.buf); - write_file(sb.buf, 1, %s\n, sha1_to_hex(new-commit-object.sha1)); + write_file(sb.buf, 1, %s\n, sha1_to_hex(rev)); strbuf_reset(sb); strbuf_addf(sb, %s/commondir, sb_repo.buf); write_file(sb.buf, 1, ../..\n); diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh index a8d9336..0fd731b 100755 --- a/t/t2025-checkout-to.sh +++ b/t/t2025-checkout-to.sh @@ -134,4 +134,14 @@ test_expect_success 'checkout with grafts' ' test_cmp expected actual ' +test_expect_success 'checkout --to from relative HEAD' ' + test_commit a + test_commit b + test_commit c + git rev-parse HEAD~1 expected + git checkout --to relhead HEAD~1 + git -C relhead rev-parse HEAD actual + test_cmp expected actual +' + test_done -- 2.5.0.rc1.197.g417e668 -- 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 v2 11/23] checkout: make --to unconditionally verbose
prepare_linked_checkout() respects git-checkout's --quiet flag, however, the plan is to relocate git checkout --to functionality to git worktree add, and git-worktree does not (yet) have a --quiet flag. Consequently, make prepare_linked_checkout() unconditionally verbose to ease eventual code movement to worktree.c. (A --quiet flag can be added to git-worktree later if there is demand for it.) Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 0dcdde2..86b1745 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -931,8 +931,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts) strbuf_addf(sb, %s/commondir, sb_repo.buf); write_file(sb.buf, 1, ../..\n); - if (!opts-quiet) - fprintf_ln(stderr, _(Enter %s (identifier %s)), path, name); + fprintf_ln(stderr, _(Enter %s (identifier %s)), path, name); setenv(GIT_CHECKOUT_NEW_WORKTREE, 1, 1); setenv(GIT_DIR_ENVIRONMENT, sb_git.buf, 1); -- 2.5.0.rc1.197.g417e668 -- 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 v2 16/23] worktree: add -b/-B options
One of git-worktree's roles is to populate the new worktree, much like git-checkout, and thus, for convenience, ought to support several of the same shortcuts. Toward this goal, add -b/-B options to create a new branch and check it out in the new worktree. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- For brevity, I intentionally mentioned only -b in the synopsis, and omitted -B. Documentation/git-worktree.txt | 13 ++--- builtin/worktree.c | 11 +++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 96e2142..f6c3747 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -9,7 +9,7 @@ git-worktree - Manage multiple worktrees SYNOPSIS [verse] -'git worktree add' [-f] [--detach] path branch +'git worktree add' [-f] [--detach] [-b new-branch] path branch 'git worktree prune' [-n] [-v] [--expire expire] DESCRIPTION @@ -65,6 +65,14 @@ OPTIONS is already checked out by another worktree. This option overrides that safeguard. +-b new-branch:: +-B new-branch:: + With `add`, create a new branch named `new-branch` starting at + `branch`, and check out `new-branch` into the new worktree. + By default, `-b` refuses to create a new branch if it already + exists. `-B` overrides this safeguard, resetting `new-branch` to + `branch`. + --detach:: With `add`, detach HEAD in the new worktree. See DETACHED HEAD in linkgit:git-checkout[1]. @@ -134,8 +142,7 @@ make the emergency fix, remove it when done, and then resume your earlier refactoring session. -$ git branch emergency-fix master -$ git worktree add ../temp emergency-fix +$ git worktree add -b emergency-fix ../temp master $ pushd ../temp # ... hack hack hack ... $ git commit -a -m 'emergency fix for boss' diff --git a/builtin/worktree.c b/builtin/worktree.c index 6967369..d461d31 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -127,15 +127,22 @@ static int add(int ac, const char **av, const char *prefix) { struct child_process c; int force = 0, detach = 0; + const char *new_branch = NULL, *new_branch_force = NULL; const char *path, *branch; struct argv_array cmd = ARGV_ARRAY_INIT; struct option options[] = { OPT__FORCE(force, N_(checkout branch even if already checked out in other worktree)), + OPT_STRING('b', NULL, new_branch, N_(branch), + N_(create a new branch)), + OPT_STRING('B', NULL, new_branch_force, N_(branch), + N_(create or reset a branch)), OPT_BOOL(0, detach, detach, N_(detach HEAD at named commit)), OPT_END() }; ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (new_branch new_branch_force) + die(_(-b and -B are mutually exclusive)); if (ac != 2) usage_with_options(worktree_usage, options); @@ -146,6 +153,10 @@ static int add(int ac, const char **av, const char *prefix) argv_array_pushl(cmd, --to, path, NULL); if (force) argv_array_push(cmd, --ignore-other-worktrees); + if (new_branch) + argv_array_pushl(cmd, -b, new_branch, NULL); + if (new_branch_force) + argv_array_pushl(cmd, -B, new_branch_force, NULL); if (detach) argv_array_push(cmd, --detach); argv_array_push(cmd, branch); -- 2.5.0.rc1.197.g417e668 -- 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 v2 12/23] checkout: drop 'checkout_opts' dependency from prepare_linked_checkout
The plan is to relocate git checkout --to functionality to git worktree add, however, worktree.c won't have access to the 'struct checkout_opts' passed to prepare_linked_worktree(), which it consults for the pathname of the new worktree and the argv[] of the command it should run to populate the new worktree. Facilitate relocation of prepare_linked_worktree() by instead having it accept the pathname and argv[] directly, thus eliminating the final references to 'struct checkout_opts'. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 86b1745..30fe786 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -854,11 +854,11 @@ static void remove_junk_on_signal(int signo) raise(signo); } -static int prepare_linked_checkout(const struct checkout_opts *opts) +static int prepare_linked_checkout(const char *path, const char **child_argv) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; - const char *path = opts-new_worktree, *name; + const char *name; struct stat st; struct child_process cp; int counter = 0, len, ret; @@ -938,7 +938,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts) setenv(GIT_WORK_TREE_ENVIRONMENT, path, 1); memset(cp, 0, sizeof(cp)); cp.git_cmd = 1; - cp.argv = opts-saved_argv; + cp.argv = child_argv; ret = run_command(cp); if (!ret) { is_junk = 0; @@ -1297,7 +1297,8 @@ static int checkout_branch(struct checkout_opts *opts, if (opts-new_worktree) { if (!new-commit) die(_(no branch specified)); - return prepare_linked_checkout(opts); + return prepare_linked_checkout(opts-new_worktree, + opts-saved_argv); } if (!new-commit opts-new_branch) { -- 2.5.0.rc1.197.g417e668 -- 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 v2 00/23] replace checkout --to with worktree add
This is v2 of the series to replace git checkout --to with git worktree add. It's built atop Duy's df0b6cf (worktree: new place for git prune --worktrees, 2015-06-29) which introduces the git-worktree command and replaces git prune --worktrees with git worktree prune. Although v1[*1*] consisted of only 1 patch, v2 is just a wee bit longer at 23 patches. Sorry for the length. Fortunately, most patches are quite small, thus (hopefully) easy to review. v2 goes a bit beyond v1 by implementing Duy's suggestion[*2*] of allowing branch in git worktree add path branch to be omitted, in which case, as a convenience, it auto-vivifies a new branch named after path, as if it had been invoked as git worktree add -b $(basename path) path HEAD. Moreover, unlike git checkout -b newbranch --to path branch, the branch in git worktree add -b newbranch path branch is also optional, and defaults to HEAD. v2 does not attempt either of the suggestions by Junio[*3*] or Duy[*4*] for eliminating git-checkout from the equation, which would allow us to remove the final couple bits of code in git-checkout which require intimate knowledge that the checkout is occurring in a newly created linked worktree. This series is already too long, and I didn't want it to grow further by implementing either of those ideas. Instead, this series leaves git-worktree at a state where one or the other of those suggestions can be done as follow-on patches touching only the underlying machinery, without affecting the user-facing interface. [*1*]: http://thread.gmane.org/gmane.comp.version-control.git/273032 [*2*]: http://thread.gmane.org/gmane.comp.version-control.git/273032/focus=273035 [*3*]: via private email which suggested using git-reset --hard rather than git checkout to populate the new linked worktree. [*4*]: http://thread.gmane.org/gmane.comp.version-control.git/273032/focus=273226 Eric Sunshine (23): Documentation/git-checkout: fix incorrect worktree prune command Documentation/git-worktree: associate options with commands Documentation: move linked worktree description from checkout to worktree Documentation/git-worktree: add BUGS section Documentation/git-worktree: split technical info from general description Documentation/git-worktree: add high-level 'lock' overview Documentation/git-worktree: add EXAMPLES section checkout: fix bug with --to and relative HEAD checkout: relocate --to's no branch specified check checkout: prepare_linked_checkout: drop now-unused 'new' argument checkout: make --to unconditionally verbose checkout: drop 'checkout_opts' dependency from prepare_linked_checkout worktree: introduce add command worktree: add --force option worktree: add --detach option worktree: add -b/-B options tests: worktree: retrofit checkout --to tests for worktree add checkout: retire --to option checkout: require worktree unconditionally worktree: extract basename computation to new function worktree: add: make -b/-B default to HEAD when branch is omitted worktree: add: auto-vivify new branch when branch is omitted checkout: retire --ignore-other-worktrees in favor of --force Documentation/git-checkout.txt| 78 + Documentation/git-worktree.txt| 141 +++- builtin/checkout.c| 161 +- builtin/worktree.c| 193 ++ git.c | 2 +- t/{t2025-checkout-to.sh = t2025-worktree-add.sh} | 68 +--- t/t2026-prune-linked-checkouts.sh | 2 +- t/t7410-submodule-checkout-to.sh | 4 +- 8 files changed, 383 insertions(+), 266 deletions(-) rename t/{t2025-checkout-to.sh = t2025-worktree-add.sh} (51%) -- 2.5.0.rc1.197.g417e668 -- 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 v2 01/23] Documentation/git-checkout: fix incorrect worktree prune command
This was missed when git prune --worktrees became git worktree prune. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-checkout.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 72def5b..ce223e6 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -444,7 +444,7 @@ When you are done with a linked working tree you can simply delete it. The working tree's entry in the repository's $GIT_DIR/worktrees directory will eventually be removed automatically (see `gc.pruneworktreesexpire` in linkgit::git-config[1]), or you can run -`git prune --worktrees` in the main or any linked working tree to +`git worktree prune` in the main or any linked working tree to clean up any stale entries in $GIT_DIR/worktrees. If you move a linked working directory to another file system, or -- 2.5.0.rc1.197.g417e668 -- 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 v2 07/23] Documentation/git-worktree: add EXAMPLES section
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 22 ++ 1 file changed, 22 insertions(+) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 410f0b4..028bbd9 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -105,6 +105,28 @@ to `/path/main/.git/worktrees/test-next` then a file named `test-next` entry from being pruned. See linkgit:gitrepository-layout[5] for details. +EXAMPLES + +You are middle of a refactoring session and your boss comes in and demands +that you fix something immediately. You might typically use +linkgit:git-stash[1] to store your changes away temporarily, however, your +worktree is in such a state of disarray (with new, removed, moved files, +and other bits and pieces strewn around) that you don't want to risk +disturbing any of it. Instead, you create a temporary linked worktree to +make the emergency fix, remove it when done, and then resume your earlier +refactoring session. + + +$ git branch emergency-fix master +$ git checkout --to ../temp emergency-fix +$ pushd ../temp +# ... hack hack hack ... +$ git commit -a -m 'emergency fix for boss' +$ popd +$ rm -rf ../temp +$ git worktree prune + + BUGS Multiple checkout support for submodules is incomplete. It is NOT -- 2.5.0.rc1.197.g417e668 -- 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 v2 04/23] Documentation/git-worktree: add BUGS section
Relocate submodule warning to BUGS and enumerate missing commands. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Documentation/git-worktree.txt | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt index 3d28896..4fbcdd2 100644 --- a/Documentation/git-worktree.txt +++ b/Documentation/git-worktree.txt @@ -76,9 +76,6 @@ to `/path/main/.git/worktrees/test-next` then a file named `test-next` entry from being pruned. See linkgit:gitrepository-layout[5] for details. -Multiple checkout support for submodules is incomplete. It is NOT -recommended to make multiple checkouts of a superproject. - COMMANDS prune:: @@ -100,6 +97,21 @@ OPTIONS --expire time:: With `prune`, only expire unused worktrees older than time. +BUGS + +Multiple checkout support for submodules is incomplete. It is NOT +recommended to make multiple checkouts of a superproject. + +git-worktree could provide more automation for tasks currently +performed manually or via other commands, such as: + +- `add` to create a new linked worktree +- `remove` to remove a linked worktree and its administrative files (and + warn if the worktree is dirty) +- `mv` to move or rename a worktree and update its administrative files +- `lock` to prevent automatic pruning of administrative files (for instance, + for a worktree on a portable device) + SEE ALSO -- 2.5.0.rc1.197.g417e668 -- 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 v2 09/23] checkout: relocate --to's no branch specified check
The plan is to relocate git checkout --to functionality to git worktree add, however, this check expects a 'struct branch_info' which git-worktree won't have at hand. It will, however, have access to its own command-line from which it can pick up the branch name. Therefore, as a preparatory step, rather than having prepare_linked_checkout() perform this check, make it the caller's responsibility. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- builtin/checkout.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index f5f953d..0cb81ee 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -865,8 +865,6 @@ static int prepare_linked_checkout(const struct checkout_opts *opts, int counter = 0, len, ret; unsigned char rev[20]; - if (!new-commit) - die(_(no branch specified)); if (file_exists(path) !is_empty_dir(path)) die(_('%s' already exists), path); @@ -1298,8 +1296,11 @@ static int checkout_branch(struct checkout_opts *opts, free(head_ref); } - if (opts-new_worktree) + if (opts-new_worktree) { + if (!new-commit) + die(_(no branch specified)); return prepare_linked_checkout(opts, new); + } if (!new-commit opts-new_branch) { unsigned char rev[20]; -- 2.5.0.rc1.197.g417e668 -- 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] --count feature for git shortlog
On Fri, Jul 3, 2015 at 10:31 AM, Junio C Hamano gits...@pobox.com wrote: John Keeping j...@keeping.me.uk writes: On Tue, Jun 30, 2015 at 02:10:49PM +0200, Johannes Schindelin wrote: On 2015-06-29 18:46, Lawrence Siebert wrote: I appreciate your help. Okay, That all makes sense. I would note that something like: git shortlog -s $FILENAME: | cut -f 1 | paste -sd+ - | bc seems like it run much faster then: git log --oneline $FILENAME | wc -l How does it compare to `git rev-list -- $FILENAME | wc -l`? Or even `git rev-list --count HEAD -- $FILENAME`. Ahh, OK. I didn't know we already had rev-list --count. Then please disregard the suggestion to add the option to log; it still holds true that the option does not belong to shortlog, but I do think how many changes were made to this path statistics driven by a script should use rev-list plumbing, and if it already has --count option, that is perfect ;-) Thanks. Junio, I think, respectfully, there is still a benefit to adding it as a feature to log, in that more Git users know of and use log than rev-list. I hadn't heard of rev-list before joining this mailing list. That means log --count will get used more. That also means that more eyeballs will hit --count with bug reports and better tests; I've already seen 2-3 suggestions for log --count tests that rev-list --count also doesn't have tests for. I would like to keep working on implementing log --count, sharing code with rev-list where possible so they both are improved, unless you are saying you won't merge. Thanks, Lawrence -- About Me: http://about.me/lawrencesiebert Constantly Coding: http://constantcoding.blogspot.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: [PATCH v2 3/4] log --count: added test
Matthieu Moy matthieu@grenoble-inp.fr writes: If implementing a proper count is too hard, one option is to forbid --count -S and --count -G to avoid confusion. Let's not go there. Letting people to use --oneline | wc -l is far better unless we can get --count that behaves the same as that, only faster. -- 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 name-rev not accepting abbreviated SHA with --stdin
On 07/03/2015 11:06 PM, Junio C Hamano wrote: Sitaram Chamarty sitar...@gmail.com writes: On 06/25/2015 05:41 AM, Junio C Hamano wrote: Sitaram Chamarty sitar...@gmail.com writes: This *is* documented, but I'm curious why this distinction is made. I think it is from mere laziness, and also in a smaller degree coming from an expectation that --stdin would be fed by another script like rev-list where feeding full 40-hex is less work than feeding unique abbreviated prefix. Makes sense; thanks. Maybe if I feel really adventurous I will, one day, look at the code :-) Sorry, but I suspect this is not 100% laziness; it is meant to read text that has object names sprinkled in and output text with object names substituted. I suspect that this was done to prevent a short string that may look like an object name like deadbabe from getting converted into an unrelated commit object name. As a perl programmer, laziness is much more palatable to me as a reason ;-) Jokes apart, I'm not sure the chances of *both* those things happening -- an accidental hash-like string in the text *and* it matching an existing hash -- are high enough to bother. If it can be done without too much code, it probably should. -- 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 v2 3/4] log --count: added test
Matthieu Moy matthieu@grenoble-inp.fr writes: Also, some revision-limiting options can reduce the count like git log --grep whatever and you should check that you actually count the right number here. (I don't know this part of the code enough, but I'm not sure you actually deal with this properly) Yes, rev-list, when the revision range is limited (with or without pathspec), can give --count once limit_list() finishes, but log filters the result of limit_list() further with at least three separate phases. - options in the grep family (--grep/--author/etc.) lets you skip commits based on the contents of the commit object; - options in the diff family (-w/-b/etc.) may let git log consider a commit because the pathspec limit thought two blobs were different at byte-by-byte level, but after running diff with these looser comparison, git log may realize that there weren't any interesting change introduced by the commit [*1*]; - and finally, of course log --max-count=20 may further limit the maximum number of commits that are shown. This of course is not interesting in the context of --count in the sense that git log --count -20 --grep=foo maint..master may not be immediately a sensible thing to do (but we never know. Perhaps your user may be asking do we have 20 or more commits that say 'foo' in the range?) An implementation of --count to take the first and the third ones in account may not be too hard, but I am fairly familiar with the codepath for the second one and I think it would be very tricky. Note that these additional things log does over rev-list *DO* justify addition of --count to log (because rev-list --count cannot emulate these); I am however not sure if it is worth the additional complexity we need to add to the codepath (especially for the second phase). I'd need to take another look at the codepaths involved myself to be sure, but I suspect the damage to the codepath for the second may end up to be extensive when we do decide to fix the possible bug in it. [Footnote] *1* They may still show the log message in such a case where -b/-w was asked and commit had only whitespace changes, but I think we should consider that a bug. -- 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] index-pack: fix overallocation of sorted_by_pos array
When c6458e60 (index-pack: kill union delta_base to save memory, 2015-04-18) attempted to reduce the memory footprint of index-pack, one of the key thing it did was to keep track of ref-deltas and ofs-deltas separately. In fix_unresolved_deltas(), however it forgot that it now wants to look only at ref deltas in one place. The code allocated an array sufficient to hold both ref- and ofs-deltas, stuffed only ref-deltas in the array, sorted it (with the right count) and iterated over the array (with the right count). There is no externally visible ill effect, other than a small wastage of memory, but the code was misleading. Also, the old code before this change had to use 'i' and 'n' because some of the things we see in the (old) deltas[] array we scanned with 'i' would not make it into the sorted_by_pos[] array in the old world order, but now because you have only ref delta in a separate ref_deltas[] array, they increment lockstep. We no longer need separate variables (nor the nr_unresolved parameter). Helped-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- * This time with a formal log message. Again, this is not really a bug-fix; just returning the memory that we allocated without using to the system, and making the code easier to follow. builtin/index-pack.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 3ed53e3..fa13e20 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1223,7 +1223,7 @@ static void resolve_deltas(void) * - append objects to convert thin pack to full pack if required * - write the final 20-byte SHA-1 */ -static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved); +static void fix_unresolved_deltas(struct sha1file *f); static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned char *pack_sha1) { if (nr_ref_deltas + nr_ofs_deltas == nr_resolved_deltas) { @@ -1245,7 +1245,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha memset(objects + nr_objects + 1, 0, nr_unresolved * sizeof(*objects)); f = sha1fd(output_fd, curr_pack); - fix_unresolved_deltas(f, nr_unresolved); + fix_unresolved_deltas(f); strbuf_addf(msg, _(completed with %d local objects), nr_objects - nr_objects_initial); stop_progress_msg(progress, msg.buf); @@ -1328,10 +1328,10 @@ static int delta_pos_compare(const void *_a, const void *_b) return a-obj_no - b-obj_no; } -static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) +static void fix_unresolved_deltas(struct sha1file *f) { struct ref_delta_entry **sorted_by_pos; - int i, n = 0; + int i; /* * Since many unresolved deltas may well be themselves base objects @@ -1343,12 +1343,12 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved) * before deltas depending on them, a good heuristic is to start * resolving deltas in the same order as their position in the pack. */ - sorted_by_pos = xmalloc(nr_unresolved * sizeof(*sorted_by_pos)); + sorted_by_pos = xmalloc(nr_ref_deltas * sizeof(*sorted_by_pos)); for (i = 0; i nr_ref_deltas; i++) - sorted_by_pos[n++] = ref_deltas[i]; - qsort(sorted_by_pos, n, sizeof(*sorted_by_pos), delta_pos_compare); + sorted_by_pos[i] = ref_deltas[i]; + qsort(sorted_by_pos, nr_ref_deltas, sizeof(*sorted_by_pos), delta_pos_compare); - for (i = 0; i n; i++) { + for (i = 0; i nr_ref_deltas; i++) { struct ref_delta_entry *d = sorted_by_pos[i]; enum object_type type; struct base_data *base_obj = alloc_base_data(); -- 2.5.0-rc1-213-g8b7a1c9 -- 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-p4] import with labels fails when commit is not transferred
Sorry for not replying earlier, and thanks for taking the time to investigate this! It's a pretty subtle corner case: I think a test case would be useful. I'm going to try to put something together, unless you beat me to it! (I think t9811-git-p4-label-import.sh is the one that needs extending). Thanks! Luke On 30/06/15 09:45, Holl, Marcus wrote: Hi, I have an issue with the git p4 tooling regarding import of labels. My git version is 2.4.5 I try to transform a perforce repository. My command line is: git p4 clone --verbose --detect-branches --import-local --import-labels --destination DESTINATION //depot@all The relevant parts in the gitconfig is: [git-p4] branchUser = USERNAME For that user there is a branch mapping defined with a lot of entries like: //depot/trunk/... //depot/branches/ipro-status-8-2--branch/... //depot/trunk/... //depot/branches/9-0-preview/... //depot/trunk/... //depot/branches/release-8-0-0-branch/... //depot/trunk/... //depot/branches/release-8-1-0-branch/... //depot/trunk/... //depot/branches/release-8-2-0-branch/... //depot/trunk/... //depot/branches/release-8-3-0-branch/... //depot/trunk/... //depot/branches/release-8-4-branch/... //depot/trunk/... //depot/branches/release-8-5-branch/... ... The import fails with the log output that can be found at the bottom of this mail. git log -all -grep \[git-p4:.*change\ =\ 69035\] reports nothing. The commit is not contained in the git repository. p4 describe for changelist 69035 returns a reasonable result. This change contains one file located at a path in the perforce folder structure that comes without corresponding entry in the perforce branch mapping. According to the given branch mapping it looks reasonable to me that the change is omitted in the git repository. But in my opinion the import should not fail in such a case. A reasonable behavior would be to blacklist the label (add it to git-p4.ignoredP4Labels) and to continue with the next label. Attached is a proposal for a fix that needs to be carefully reviews since I'm not that experienced with python. Other proposals for resolving this issue are highly appreciated. Thanks a lot and best regards, Marcus Holl Log output: Reading pipe: ['git', 'rev-list', '--max-count=1', '--reverse', ':/\\[git-p4:.*change = 69035\\]'] fatal: ambiguous argument ':/\[git-p4:.*change = 69035\]': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git command [revision...] -- [file...]' ied with change: 69078, original Date: 2010-04-22T09:07:24.00Z\n', 'Update': '2013/11/02 07:40:31', 'Label': 'release-8-1-0-976', 'Access': '2015/06/26 14:50:15', 'Owner': 'svn_p4_converter', 'Options': 'unlocked noautoreload'} p4 label release-8-1-0-976 mapped to git commit 82a11809928b86a7bde03cf486428de52ab3380f writing tag release-9-0-0-179 for commit fb8370cd04806686c567ad720d065436f2334b4a labelDetails= {'code': 'stat', 'Description': 'Created or modified with change: 96984, original Date: 2011-12-22T16:01:25.681427Z\n', 'Update': '2013/11/02 15:15:50', 'Label': 'release-9-0-0-179', 'Access': '2015/06/26 14:50:16', 'Owner': 'build', 'Options': 'unlocked noautoreload'} p4 label release-9-0-0-179 mapped to git commit fb8370cd04806686c567ad720d065436f2334b4a Traceback (most recent call last): File /usr/lib/git/git-p4, line 3297, in module main() File /usr/lib/git/git-p4, line 3291, in main if not cmd.run(args): File /usr/lib/git/git-p4, line 3165, in run if not P4Sync.run(self, depotPaths): File /usr/lib/git/git-p4, line 3045, in run self.importP4Labels(self.gitStream, missingP4Labels) File /usr/lib/git/git-p4, line 2421, in importP4Labels --reverse, :/\[git-p4:.*change = %d\] % changelist]) File /usr/lib/git/git-p4, line 138, in read_pipe die('Command failed: %s' % str(c)) File /usr/lib/git/git-p4, line 106, in die raise Exception(msg) Exception: Command failed: ['git', 'rev-list', '--max-count=1', '--reverse', ':/\\[git-p4:.*change = 69035\\]'] -- 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] log: add log.follow config option
David Turner dtur...@twopensource.com writes: Twitter's history is almost completely linear, so it's useful for us. Since it looks like the patch won't be useful outside of our context, I'll just rewrite it to check the pathspec count, and not upstream it until follow becomes more general. As long as it's an opt-in and that the documentation states the limitations clearly enough, I think it makes sense to me to have this upstream. -- 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 v2 2/4] log: add --count option to git log
Lawrence Siebert lawrencesieb...@gmail.com writes: +static void show_object(struct object *obj, + const struct name_path *path, const char *component, + void *cb_data) +{ + return; +} It seems streange to me to have a function named show_object when it does not show anything. Maybe name it null_travers_cb to make it clear it's a callback and it does nothing? Not a strong objection though, it's only a static function. +static void show_commit(struct commit *commit, void *data) +{ + struct rev_info *revs = (struct rev_info *)data; + if (commit-object.flags PATCHSAME) + revs-count_same++; + else if (commit-object.flags SYMMETRIC_LEFT) + revs-count_left++; + else + revs-count_right++; + if (commit-parents) { + free_commit_list(commit-parents); + commit-parents = NULL; + } + free_commit_buffer(commit); +} + static int cmd_log_walk(struct rev_info *rev) { struct commit *commit; int saved_nrl = 0; int saved_dcctc = 0; + if (rev-count) { + prepare_revision_walk(rev); + traverse_commit_list(rev, show_commit, show_object, rev); + get_commit_count(rev); + } I didn't test, but it seems this does a full graph traversal before starting the output. A very important property of git log is that it starts showing revisions immediately, even when ran on a very long history (it shows the first screen immediately and continues working in the background while the first page is displayed in the pager). Is it the case? If so, it should be changed. If not, perhaps explain why in the commit message. -- 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 v2 3/4] log --count: added test
Lawrence Siebert lawrencesieb...@gmail.com writes: added test comparing output between git log --count HEAD and git rev-list --count HEAD Unless there is a very long list of tests, I'd rather see this squashed with PATCH 2/4. As a reviewer I prefer having code and tests in the same place. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- t/t4202-log.sh | 7 +++ 1 file changed, 7 insertions(+) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 1b2e981..35f8d82 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' ' test_must_fail git log --graph --no-walk ' +test_expect_success 'log --count' ' + git log --count HEAD actual + git rev-list --count HEAD expect The weird space is still there. Also, we write actual, not actual in the Git coding style. That is actually a rather weak test. rev-list --count interacts with --left-right, so I guess you want to test --count --left-right. Also, some revision-limiting options can reduce the count like git log --grep whatever and you should check that you actually count the right number here. (I don't know this part of the code enough, but I'm not sure you actually deal with this properly) -- 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 v2 1/4] list-object: add get_commit_count function
Mattieu, Understood. I don't suppose there is any commonly code formatting tool to automate formatting in the git style, is there? Thanks, Lawrence On Fri, Jul 3, 2015 at 12:24 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Lawrence Siebert lawrencesieb...@gmail.com writes: +void get_commit_count(struct rev_info * revs) { Please, write struct rev_info *revs (stick * to revs). +void get_commit_count(struct rev_info * revs); Likewise. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- About Me: http://about.me/lawrencesiebert Constantly Coding: http://constantcoding.blogspot.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: [PATCH v2 1/4] list-object: add get_commit_count function
Lawrence Siebert lawrencesieb...@gmail.com writes: +void get_commit_count(struct rev_info * revs) { Please, write struct rev_info *revs (stick * to revs). +void get_commit_count(struct rev_info * revs); Likewise. -- 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 v2 2/4] log: add --count option to git log
traverse_commit_list requires a function to be passed to it. That said, after further review it can probably pass NULL and have it work fine. If not, I'll use your naming convention. `git rev-list --count` (or actually `git rev-list --count HEAD`, which this borrows the code from, produces a single value, a numeric count. I think walking the entire list is necessary to get the final value, which is what we want with --count. Thanks, Lawrence Siebert On Fri, Jul 3, 2015 at 12:29 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Lawrence Siebert lawrencesieb...@gmail.com writes: +static void show_object(struct object *obj, + const struct name_path *path, const char *component, + void *cb_data) +{ + return; +} It seems streange to me to have a function named show_object when it does not show anything. Maybe name it null_travers_cb to make it clear it's a callback and it does nothing? Not a strong objection though, it's only a static function. +static void show_commit(struct commit *commit, void *data) +{ + struct rev_info *revs = (struct rev_info *)data; + if (commit-object.flags PATCHSAME) + revs-count_same++; + else if (commit-object.flags SYMMETRIC_LEFT) + revs-count_left++; + else + revs-count_right++; + if (commit-parents) { + free_commit_list(commit-parents); + commit-parents = NULL; + } + free_commit_buffer(commit); +} + static int cmd_log_walk(struct rev_info *rev) { struct commit *commit; int saved_nrl = 0; int saved_dcctc = 0; + if (rev-count) { + prepare_revision_walk(rev); + traverse_commit_list(rev, show_commit, show_object, rev); + get_commit_count(rev); + } I didn't test, but it seems this does a full graph traversal before starting the output. A very important property of git log is that it starts showing revisions immediately, even when ran on a very long history (it shows the first screen immediately and continues working in the background while the first page is displayed in the pager). Is it the case? If so, it should be changed. If not, perhaps explain why in the commit message. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- About Me: http://about.me/lawrencesiebert Constantly Coding: http://constantcoding.blogspot.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: [PATCH v2 3/4] log --count: added test
Matthieu, Ok, I'll fix that. I think I can also add tests, I can look at the tests for rev-list --count, with the understanding that I saw somebody else had made changes for the --use-bitmap-index option, and I am basing off of master for this, and thus don't feel comfortable with --use-bitmap-index at this time. If it's acceptable practice, I'll just squash everything I do on this feature and it's tests into one commit with a more detailed comment, and send the patch for that. I wasn't sure about how much history I should save, and how much I should split stuff up, so I appreciate your clarification. Thank you for your time, Lawrence Siebert On Fri, Jul 3, 2015 at 12:34 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Lawrence Siebert lawrencesieb...@gmail.com writes: added test comparing output between git log --count HEAD and git rev-list --count HEAD Unless there is a very long list of tests, I'd rather see this squashed with PATCH 2/4. As a reviewer I prefer having code and tests in the same place. Signed-off-by: Lawrence Siebert lawrencesieb...@gmail.com --- t/t4202-log.sh | 7 +++ 1 file changed, 7 insertions(+) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index 1b2e981..35f8d82 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -871,4 +871,11 @@ test_expect_success 'log --graph --no-walk is forbidden' ' test_must_fail git log --graph --no-walk ' +test_expect_success 'log --count' ' + git log --count HEAD actual + git rev-list --count HEAD expect The weird space is still there. Also, we write actual, not actual in the Git coding style. That is actually a rather weak test. rev-list --count interacts with --left-right, so I guess you want to test --count --left-right. Also, some revision-limiting options can reduce the count like git log --grep whatever and you should check that you actually count the right number here. (I don't know this part of the code enough, but I'm not sure you actually deal with this properly) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- About Me: http://about.me/lawrencesiebert Constantly Coding: http://constantcoding.blogspot.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: [PATCH v2 1/4] list-object: add get_commit_count function
Lawrence Siebert lawrencesieb...@gmail.com writes: Mattieu, Understood. I don't suppose there is any commonly code formatting tool to automate formatting in the git style, is there? IIRC, someone posted a configuration file for clang-format that essentially matches the Git coding style. You can read Documentation/CodingGuidelines. Unsurprisingly, Git uses a coding style similar to the Linux kernel, so anything you find about the kernel essentially applies here too. -- 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 v2 2/4] log: add --count option to git log
Hi, Please, don't top-post on this list. Quote the parts you're replying to and reply below. Lawrence Siebert lawrencesieb...@gmail.com writes: traverse_commit_list requires a function to be passed to it. That said, after further review it can probably pass NULL and have it work fine. If not, I'll use your naming convention. If possible, passing NULL would be best. `git rev-list --count` (or actually `git rev-list --count HEAD`, which this borrows the code from, produces a single value, a numeric count. I think walking the entire list is necessary to get the final value, which is what we want with --count. OK, that was me answering emails before coffee. I thought --count was producing the count _in addition_ to the normal output. But then I don't understand by looking at the code how you prevent the normal output. I just tested, and indeed it does work (I guess all the magic is already there from rev-list --count, and it was more or less a bug that log --count was not using it properly), but you may want to explain better what's going on in the commit message. -- 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 v2 3/4] log --count: added test
Lawrence Siebert lawrencesieb...@gmail.com writes: Ok, I'll fix that. (Note: this is a typical example of why we don't top-post here. I made several remarks and I can't know what that refers to) (Meta-note: don't take the note as agressive, I know that top-posting is the norm in many other places, I'm just giving you a glimpse of the local culture ;-) ). If it's acceptable practice, I'll just squash everything I do on this feature and it's tests into one commit with a more detailed comment, and send the patch for that. I think at least two patches are better: your PATCH 1 is a typical preparation step, best reviewed alone in its own patch. Splitting history into several patches is good, but each patch should correspond to one logical step. Splitting code Vs doc Vs tests is usually not the right way. -- 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 v2 3/4] log --count: added test
Matthieu Moy matthieu@grenoble-inp.fr writes: Also, some revision-limiting options can reduce the count like git log --grep whatever OK, --grep seems to work, but -S and -G do not: $ ./bin-wrappers/git log -Sfoo --count 40012 $ ./bin-wrappers/git log -Sfoo --oneline | wc -l 925 $ ./bin-wrappers/git log --count 40012 See 251df09 (log: fix --max-count when used together with -S or -G, 2011-03-09) for a start of an explanation. If implementing a proper count is too hard, one option is to forbid --count -S and --count -G to avoid confusion. -- 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