Re: [PATCH 02/15] t5510: prepare test refs more straightforwardly
On 10/23/2013 08:36 PM, Junio C Hamano wrote: > Michael Haggerty writes: > >> "git fetch" was being used with contrived refspecs to create tags and >> remote-tracking branches in test repositories in preparation for the >> actual tests. This is obscure and also makes one wonder whether this >> is indeed just preparation or whether some side-effect of "git fetch" >> is being tested. >> >> So use the more straightforward commands "git tag" / "git update-ref" >> when preparing branches in test repositories. >> >> Signed-off-by: Michael Haggerty >> --- >> t/t5510-fetch.sh | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh >> index c5e5dfc..08d8dbb 100755 >> --- a/t/t5510-fetch.sh >> +++ b/t/t5510-fetch.sh >> @@ -88,7 +88,7 @@ test_expect_success 'fetch --prune on its own works as >> expected' ' >> cd "$D" && >> git clone . prune && >> cd prune && >> -git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && >> +git update-ref refs/remotes/origin/extrabranch master && > > As long as you have checked that our local 'master' should be at the > same commit as the origin's 'master' at this point, I think this > change is OK. It doesn't matter what the reference points at; the only point of these tests is to check whether branches that look like stale remote-tracking branches are pruned or not by the later command. > I wouldn't call the use of "very explicit, without any room for > mistake" refspecs "contrived", though. According to Wiktionary, contrived means "unnatural, forced". When the goal is just to create a local reference whose contents are irrelevant, "fetch" is not the first command that comes to my mind. It brings a lot of unnecessary machinery to bear on such a trivial task. Plus it is not very common in daily life to invoke "fetch" with a complicated, asymmetic refspec like this. Because of that it cost me a little extra time to convince myself that the "fetch" command wasn't trying to do something more. In that sense it seems "contrived" to me. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.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: RFE: support change-id generation natively
Am 10/24/2013 7:25, schrieb Duy Nguyen: > On Thu, Oct 24, 2013 at 11:11 AM, Nasser Grainawi > wrote: It is not clear to me how you envision to make it work. >>> >>> I don't have the source code. >> >> Now you do: >> https://gerrit.googlesource.com/gerrit/+/master/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg > > Thanks. So you do have tree sha-1 by running "git write-tree". But at > that point I'm not sure if cache-tree is written down to disk yet, so > write-tree could be more expensive than necessary (one good point for > building --change-id in). Consider that I make a commit with a change-id. Then I rewrite the commit, but keep the change-id. Then I push the rewritten commit to Gerrit. Gerrit does not have the objects that the change-id is based on; the change-id is just a random number and has no other significance. Right? Why do you go all the length in computing a change-id instead of just pulling 20 bytes from /dev/random? That said, I don't think that --change-id option that the user must not forget to use is any better than a hook that the user must not forget to install. -- Hannes -- 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: RFE: support change-id generation natively
On Thu, Oct 24, 2013 at 11:11 AM, Nasser Grainawi wrote: >>> It is not clear to me how you envision to make it work. >> >> I don't have the source code. > > Now you do: > https://gerrit.googlesource.com/gerrit/+/master/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg Thanks. So you do have tree sha-1 by running "git write-tree". But at that point I'm not sure if cache-tree is written down to disk yet, so write-tree could be more expensive than necessary (one good point for building --change-id in). -- 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: RFE: support change-id generation natively
On Oct 23, 2013, at 8:07 PM, Duy Nguyen wrote: > On Wed, Oct 23, 2013 at 11:00 PM, Junio C Hamano wrote: >> Duy Nguyen writes: >> >>> On Wed, Oct 23, 2013 at 2:50 AM, Junio C Hamano wrote: It would be just the matter of updating commit_tree_extended() in commit.c to: - detect the need to add a new Change-Id: trailer; - call hash_sha1_file() on the commit object buffer (assuming that a commit object that you can actually "git cat-file commit" using the change Id does not have to exist anywhere for Gerrit to work---otherwise you would need to call write_sha1_file() instead) before adding Change-Id: trailer; - add Change-Id: trailer to the buffer; and then finally - let the existing write_sha1_file() to write it out. >>> >>> I'm not objecting special support for Gerrit, but if the change is >>> just commit_tree_extended() why don't we just ship the commit hook in >>> a new "Gerrit" template? >> >> It is not clear to me how you envision to make it work. > > I don't have the source code. Now you do: https://gerrit.googlesource.com/gerrit/+/master/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg > But the commit-msg hook document [1] > describes roughly what you wrote below, except the tree part. And I > suppose the hook has been working fine so far. Reading back the > original post, James ruled out always-active hooks in general and > wanted the control per command line. Perhaps we should add > --no-hooks[=,] to "git commit"? Or maybe it's still > inconvenient and --change-id is best. > > [1] > http://gerrit-documentation.googlecode.com/svn/Documentation/2.0/cmd-hook-commit-msg.html > >> Naïvely thinking, an obvious place to do this kind of thing may be >> the "commit-msg" hook, where the hook reads what the user prepared, >> finds that there is no existing "Change-Id:" trailer, and decides to >> add one. >> >> But what value would it add on that line as the Id? >> >> It wants to use the name of the commit object that would result if >> it were to return without further editing the given message, but we >> do not give such a commit object name to the hook, so the hook needs >> to duplicate the logic to come up with one. It may be doable (after >> all, builtin/commit.c is open source), but we do not give the hook >> the commit object header (i.e. it does not know what the tree, >> parent(s), author, committer lines would say, nor it does not know >> if we are going to add an encoding line), so the hook needs to guess >> what we will put there, too. > -- > 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 -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- 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 grep: search whole tree by default?
On Wed, Oct 23, 2013 at 10:43:36PM +0200, Matthieu Moy wrote: > That may be an option. In the case of "git add -u", it was a bit more > complicated, since a badly used "git add" somehow looses data (not very > serious, you may only loos the index). So, saying after the fact "oh, by > the way, I messed up the index" was not a very good transition plan. > > In the case of "grep", I'm starting to get convinced that it's OK to do > so, because the user can basically re-run grep with the right argument > if needed. For the same reason, is it insane to want a config option to switch the default when no command-line option is given? These days I am mostly working on reasonably-sized projects, and would generally prefer full-tree grep. But in a past life, I worked on some large projects where I would never touch anything outside of a particular subtree, and I generally wanted a more limited grep (i.e., I would park my cwd in /repo/subsystem1 rather than /repo and work from there, and hits in /repo/subsystem2 were just useless noise). That would also provide people who do not like the change of default an escape hatch to keep the current behavior. And I do not think scripted use will be inconvenienced; they will already have to use "." or ":/" to be explicit (if they care) since the behavior is changing. > The warning could be de-activable with an advice.* option. Such a config option could also be used to shut up the warning. Though if the behavior change is deemed non-intrusive enough to not merit a deprecation period, I am not really sure it is worth having a noisy warning. -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: git grep: search whole tree by default?
On Wed, Oct 23, 2013 at 12:31 PM, Junio C Hamano wrote: > Jed Brown writes: > >> Junio C Hamano writes: >> >>> Jed Brown writes: >>> Junio C Hamano writes: > I suspect that it would be too late for 2.0 we want to do sometime > early next year, though. How would you manage transition from the current behavior? Warning people to explicitly use "." or ":/" during some interim period sounds worse than just switching the default behavior. >>> >>> "How would I"? >>> >>> You're asking that question only because you omitted too much from >>> the quote ;-) >> >> I meant that if the proposed migration plan were to be "just change it >> and people will learn" (because anything more gradual would actually be >> worse for users) then is it really too late for Git-2.0? > > I do not know it that is even a workable plan, but I need to sleep > on it and then hear opinion from others, but in general, if anybody > needs to ask if it is too late, then it already is. Making grep tree-wide would be very welcome here. IMO Git-2.0 *feels* like a good time to change the default since there's relatively little downside to doing so, but "early next year" is not very long to wait either; it doesn't seem like there's a strong reason to rush this in. -- 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: RFE: support change-id generation natively
On Wed, Oct 23, 2013 at 11:00 PM, Junio C Hamano wrote: > Duy Nguyen writes: > >> On Wed, Oct 23, 2013 at 2:50 AM, Junio C Hamano wrote: >>> It would be just the matter of updating commit_tree_extended() in >>> commit.c to: >>> >>> - detect the need to add a new Change-Id: trailer; >>> >>> - call hash_sha1_file() on the commit object buffer (assuming that >>>a commit object that you can actually "git cat-file commit" using >>>the change Id does not have to exist anywhere for Gerrit to >>>work---otherwise you would need to call write_sha1_file() >>>instead) before adding Change-Id: trailer; >>> >>> - add Change-Id: trailer to the buffer; and then finally >>> >>> - let the existing write_sha1_file() to write it out. >> >> I'm not objecting special support for Gerrit, but if the change is >> just commit_tree_extended() why don't we just ship the commit hook in >> a new "Gerrit" template? > > It is not clear to me how you envision to make it work. I don't have the source code. But the commit-msg hook document [1] describes roughly what you wrote below, except the tree part. And I suppose the hook has been working fine so far. Reading back the original post, James ruled out always-active hooks in general and wanted the control per command line. Perhaps we should add --no-hooks[=,] to "git commit"? Or maybe it's still inconvenient and --change-id is best. [1] http://gerrit-documentation.googlecode.com/svn/Documentation/2.0/cmd-hook-commit-msg.html > Naïvely thinking, an obvious place to do this kind of thing may be > the "commit-msg" hook, where the hook reads what the user prepared, > finds that there is no existing "Change-Id:" trailer, and decides to > add one. > > But what value would it add on that line as the Id? > > It wants to use the name of the commit object that would result if > it were to return without further editing the given message, but we > do not give such a commit object name to the hook, so the hook needs > to duplicate the logic to come up with one. It may be doable (after > all, builtin/commit.c is open source), but we do not give the hook > the commit object header (i.e. it does not know what the tree, > parent(s), author, committer lines would say, nor it does not know > if we are going to add an encoding line), so the hook needs to guess > what we will put there, too. -- 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 v2] entry.c: convert checkout_entry to use strbuf
The old code does not do boundary check so any paths longer than
PATH_MAX can cause buffer overflow. Replace it with strbuf to handle
paths of arbitrary length.
The OS may reject if the path is too long though. But in that case we
report the cause (e.g. name too long) and usually move on to checking
out the next entry.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
v2 does two strbuf_add() instead of one hard-to-read strbuf_addf()
entry.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/entry.c b/entry.c
index acc892f..fbb4863 100644
--- a/entry.c
+++ b/entry.c
@@ -237,16 +237,19 @@ static int check_path(const char *path, int len, struct
stat *st, int skiplen)
int checkout_entry(struct cache_entry *ce,
const struct checkout *state, char *topath)
{
- static char path[PATH_MAX + 1];
+ static struct strbuf path_buf = STRBUF_INIT;
+ char *path;
struct stat st;
- int len = state->base_dir_len;
+ int len;
if (topath)
return write_entry(ce, topath, state, 1);
- memcpy(path, state->base_dir, len);
- strcpy(path + len, ce->name);
- len += ce_namelen(ce);
+ strbuf_reset(&path_buf);
+ strbuf_add(&path_buf, state->base_dir, state->base_dir_len);
+ strbuf_add(&path_buf, ce->name, ce_namelen(ce));
+ path = path_buf.buf;
+ len = path_buf.len;
if (!check_path(path, len, &st, state->base_dir_len)) {
unsigned changed = ce_match_stat(ce, &st,
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
--
1.8.2.82.gc24b958
--
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] entry.c: convert write_entry to use strbuf
On Thu, Oct 24, 2013 at 12:52 AM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> The strcpy call in open_output_fd() implies that the output buffer >> must be at least 25 chars long. > > Hmph, where does that 25 come from? > > [snipped] Much better. Thanks. -- 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 00/14] Officially start moving to the term 'staging area'
Am 19.10.2013 16:08, schrieb Philip Oakley: > From: "Karsten Blees" >> Am 15.10.2013 00:29, schrieb Felipe Contreras: >>> tl;dr: everyone except Junio C Hamano and Drew Northup agrees; we >>> should move >>> away from the name "the index". >>> >>> It has been discussed many times in the past that 'index' is not an >>> appropriate description for what the high-level user does with it, >>> and >>> it has been agreed that 'staging area' is the best term. >>> >> >> I haven't followed the previous discussion, but if a final conclusion >> towards 'staging area' has already been reached, it should probably be >> revised. > > Do you mean that how that conclusion was reached should be summarised, > or that you don't think it's an appropriate summary of the broader > weltanschauung? > The latter. I don't know about 'broader', but I'll try to summarize _my_ world view: (1) Audience matters For actual users, we need an accurate model that supports a variety of use cases without falling apart. IMO, a working model is more important than simplicity. Finally, its more important to agree on the actual model than on a vague term that can mean many things (theater stage vs. loading dock...). For potential users / decision makers, we need to describe git's features in unmistakable terms that don't need extra explanation. In this sense, the index / cache / staging area is not a feature in itself but facilitates a variaty of broader features: - fine grained commit control (via index (add -i), but also commit -p, commit --amend, cherry-pick, rebase etc.) - performance - merging (2) Index An index, as in a library, maps almost perfectly to what the git index is _and_ what we do with it. No, I don't mean .so/.dll/.lib files, I'm talking about the real thing with shelves of books and a big box with index cards (aka the index). The defining characteristic of a book (or publication in general) is its content, not its physical representation (paper). There are typically many indistinguishable copies of the same book. An author can continue working on the manuscript without affecting the copy at the library at all. When a new or updated publication is submitted to the library, it is first added to the index and placed on a cart at the reception desk. Some time later, the librarian commits the content of the cart to the shelves. A user of the library will typically consult the index to lookup information or to check if his personal copy of a publication is up to date. The index can be thrown away and rebuilt from the content of the shelves. A big library may have a central repository and several local branches (aka field offices) that can be synchronized by comparing their indexes card by card. Granted, a library is typically not versioned, and its unlikely that any one user will have checked out a full copy of the library's content. But otherwise, its pretty similar to git... (3a) Staging area (logistics) A staging area, as in (military) logistics / transportation, is about moving physical goods around. You move an item from your stock to the staging area, then onto the truck and finally deliver it to the customer. The defining characteristic of a physical good is its physical existence. Each item is uniquely identifiable by a serial number. There may be many of the same kind, but there are no exact copies. Problem #1: If an item in the staging area is broken, you fix it directly in the staging area, because that's where it _is_. Thus you also don't need to stage the item again. That's how conventional SCMs work: they track the identity (serial number, file name) of things. Problem #2: The transportation model only supports additions. You cannot add an item to your staging area that, upon delivery, will magically remove itself from the possession of the customer. Let alone that you'd have to steal it first to be able to physically place it into your staging area. This can be fixed by slightly modifying our mental model: instead of real things, lets think about "staging changes" (or deltas, or patches). Again, that's what conventional SCMs do and what git exactly does _not_ do. Problem #3: In logistics, the state / inventory of the customer is irrelevant. If a customer orders an item he already has, its his problem. There's no need for core commands like status, diff or reset, and there's no way to explain what they do with a staging area model. What if a customer buys at another shop without telling us, effectively changing his inventory (git reset --soft)? This shouldn't affect our staging area at all, right? But with git it does...ooops. (3b) Staging area (other meanings) I don't see how a stage (as in a theater) is in any way related to the git index. Data staging (as in loading a datawarehouse or web-server) fits to some extent, as its also about copying information, not moving physical things. [...] >> >> 1.) Recording individual files to commit in advance (instead
Re: [PATCH v2 2/2] Update documentation for http.continue option
On Tue, Oct 22, 2013 at 08:21:48PM -0700, Shawn Pearce wrote: > From my perspective, it is OK to defaulting to use 100-continue if the > server supports Negotiate. If the user is stuck behind a broken proxy > and can't authenticate, they can't authenticate. They can either set > the variable to false, or fix their proxy, or use a different server, > etc. I think Jonathan's suggestion was to get rid of the variable altogether and simply make the code conditional on whether the server is offering GSS-Negotiate. I plan to make the use of 100-continue conditional on large_request as well, so that it only covers the case where it would otherwise fail. People who have broken proxies or broken servers and are using GSS-Negotiate (which, as I said, is probably very few people, if any) will simply have to increase the postbuffer size as before. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v2 2/2] Update documentation for http.continue option
On Wed, Oct 23, 2013 at 08:47:57AM -0700, Junio C Hamano wrote: > Jonathan Nieder writes: > > > Wouldn't a natural fix be to *always* use "Expect: 100-continue" when > > and only when the probe_rpc() revealed a server supporting > > GSS-Negotiate authentication? > > A stupid question. Is GSS-Negotiate the only special case? I believe so, because it's the only case where we don't have a password. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
What's cooking in git.git (Oct 2013, #05; Wed, 23)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * bc/gnome-keyring (2013-10-16) 16 commits (merged to 'next' on 2013-10-18 at 25024aa) + contrib/git-credential-gnome-keyring.c: support really ancient gnome-keyring + contrib/git-credential-gnome-keyring.c: support ancient gnome-keyring + contrib/git-credential-gnome-keyring.c: report failure to store password + contrib/git-credential-gnome-keyring.c: use glib messaging functions + contrib/git-credential-gnome-keyring.c: use glib memory allocation functions + contrib/git-credential-gnome-keyring.c: use secure memory for reading passwords + contrib/git-credential-gnome-keyring.c: use secure memory functions for passwds + contrib/git-credential-gnome-keyring.c: use gnome helpers in keyring_object() + contrib/git-credential-gnome-keyring.c: set Gnome application name + contrib/git-credential-gnome-keyring.c: ensure buffer is non-empty before accessing + contrib/git-credential-gnome-keyring.c: strlen() returns size_t, not ssize_t + contrib/git-credential-gnome-keyring.c: exit non-zero when called incorrectly + contrib/git-credential-gnome-keyring.c: add static where applicable + contrib/git-credential-gnome-keyring.c: *style* use "if ()" not "if()" etc. + contrib/git-credential-gnome-keyring.c: remove unused die() function + contrib/git-credential-gnome-keyring.c: remove unnecessary pre-declarations Cleanups and tweaks for credential handling to work with ancient versions of the gnome-keyring library that are still in use. * hu/cherry-pick-previous-branch (2013-10-10) 1 commit (merged to 'next' on 2013-10-14 at 090934f) + cherry-pick: handle "-" after parsing options "git cherry-pick" without further options would segfault. Could use a follow-up to handle '-' after argv[1] better. * jc/pack-objects (2013-02-04) 1 commit (merged to 'next' on 2013-10-14 at 8e8feb6) + pack-objects: shrink struct object_entry * jc/prompt-upstream (2013-10-14) 1 commit (merged to 'next' on 2013-10-14 at 270ef7b) + git-prompt.sh: optionally show upstream branch name An enhancement to the GIT_PS1_SHOWUPSTREAM facility. * mg/more-textconv (2013-05-10) 7 commits (merged to 'next' on 2013-10-14 at 8a12490) + grep: honor --textconv for the case rev:path + grep: allow to use textconv filters + t7008: demonstrate behavior of grep with textconv + cat-file: do not die on --textconv without textconv filters + show: honor --textconv for blobs + diff_opt: track whether flags have been set explicitly + t4030: demonstrate behavior of show with textconv Make "git grep" and "git show" pay attention to --textconv when dealing with blob objects. * po/dot-url (2013-10-15) 3 commits (merged to 'next' on 2013-10-15 at 312d0af) + doc/cli: make "dot repository" an independent bullet point (merged to 'next' on 2013-09-20 at 6a12786) + config doc: update dot-repository notes + doc: command line interface (cli) dot-repository dwimmery Explain how '.' can be used to refer to the "current repository" in the documentation. -- [New Topics] * ap/remote-hg-unquote-cquote (2013-10-23) 1 commit - remote-hg: unquote C-style paths when exporting A fast-import stream expresses a pathname with funny characters by quoting them in C style; remote-hg remote helper forgot to unquote such a path. Will merge to 'next'. * jl/pack-transfer-avoid-double-close (2013-10-23) 1 commit - Clear fd after closing to avoid double-close error The codepath that send_pack() calls pack_objects() mistakenly closed the same file descriptor twice, leading to potentially closing a wrong file descriptor that was opened in the meantime. Will merge to 'next'. Needs to be merged later to 'maint'. * nd/magic-pathspec (2013-10-22) 1 commit - Fix calling parse_pathspec with no paths nor PATHSPEC_PREFER_* flags All callers to parse_pathspec() must choose between getting no pathspec or one path that is limited to the current directory when there is no paths given on the command line, but there were two callers that violated this rule, triggering a BUG(). Will merge to 'next'. * sb/git-svn-docs-indent-with-ht (2013-10-22) 1 commit - git-svn docs: Use tabs consistently within the ascii doc Will merge to 'next'. * tr/gitk-doc-update (2013-10-22) 1 commit - Documentation: revamp gitk(1) Will merge to 'next'. * tr/valgrind-test-fix (2013-10-22) 2 commits - Revert "test-lib: allow prefixing a custom string before "ok N" etc." - Revert "test-lib: support running tests under valgrind in parallel" Will merge to 'next'. * sb/repack-in-c (2013-10-22) 1 commit (
Re: Working patterns
wor...@alum.mit.edu (Dale R. Worley) writes: > It seems to me that the two sets of Git commands should be invokable > under the same circumstances, that there is a design specification as > to how Git can be invoked, and both implementations should match that. As far as I know, the design for any (be it script or command) is that they should be able to run from the top-level and from any subdirectory within, but running from outside the top-level of the working tree has never been part of the design goal. If some commands tolerated it, that was by mere accident and not by design. -- 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 grep: search whole tree by default?
Jed Brown writes: > Junio C Hamano writes: > >> Jed Brown writes: >> >>> Junio C Hamano writes: I suspect that it would be too late for 2.0 we want to do sometime early next year, though. >>> >>> How would you manage transition from the current behavior? Warning >>> people to explicitly use "." or ":/" during some interim period sounds >>> worse than just switching the default behavior. >> >> "How would I"? >> >> You're asking that question only because you omitted too much from >> the quote ;-) > > I meant that if the proposed migration plan were to be "just change it > and people will learn" (because anything more gradual would actually be > worse for users) then is it really too late for Git-2.0? That may be an option. In the case of "git add -u", it was a bit more complicated, since a badly used "git add" somehow looses data (not very serious, you may only loos the index). So, saying after the fact "oh, by the way, I messed up the index" was not a very good transition plan. In the case of "grep", I'm starting to get convinced that it's OK to do so, because the user can basically re-run grep with the right argument if needed. The warning could be de-activable with an advice.* option. Again, no strong opinion here, but that seems workable to me. -- 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
Working patterns
> The pattern I use is to have this: > > /repository/.git > with core.worktree = /working > /working/... > > then > > cd /repository > git add /working/x/y > git ... The point I'm trying to make is that it appears that all of the Git commands implemented as programs use the same code at the beginning to determine the location of the Git repository and the root directory of the work tree -- to determine GIT_DIR and GIT_WORK_TREE effectively. And the code works with my work pattern, which seems intuitively correct to me. It seems to me that this code should implement the design, and the design should match what the code does. However, the parallel code in the Git commands that are scripts, which seems to be in git-sh-setup, does not implement the same design as the C code does. It seems to me that the two sets of Git commands should be invokable under the same circumstances, that there is a design specification as to how Git can be invoked, and both implementations should match that. Dale -- 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 grep: search whole tree by default?
Jed Brown writes: > Junio C Hamano writes: > >> Jed Brown writes: >> >>> Junio C Hamano writes: I suspect that it would be too late for 2.0 we want to do sometime early next year, though. >>> >>> How would you manage transition from the current behavior? Warning >>> people to explicitly use "." or ":/" during some interim period sounds >>> worse than just switching the default behavior. >> >> "How would I"? >> >> You're asking that question only because you omitted too much from >> the quote ;-) > > I meant that if the proposed migration plan were to be "just change it > and people will learn" (because anything more gradual would actually be > worse for users) then is it really too late for Git-2.0? I do not know it that is even a workable plan, but I need to sleep on it and then hear opinion from others, but in general, if anybody needs to ask if it is too late, then it already 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
Rebasing merge commit fails during --preserve-merges when ancestor merge is deleted
I have this situation and an accompanying sample repository reproducing the issue: https://gist.github.com/fj/7124501/raw/8d37058c1452379d0ae58bd87b0b9e0380bd80b2/gistfile1.txt * I would like to rebase and delete a spurious ancestor merge commit (bbd8966 in the example). * When I do that, however, a successor merge commit (97ba1d7) that has the ancestor merge commit (bbd8966) fails with the noted error: = error: Commit 97ba1d761f60b901f56766886da4ed678e56abea is a merge but no -m option was given. fatal: cherry-pick failed Could not pick 97ba1d761f60b901f56766886da4ed678e56abea = The expected result for me is that a new merge commit that merges 6b8c765 (which now occupies the topological spot of the previous spurious commit, bbd8966) and 11d8b92. What am I doing wrong here? Is there a way to fix multiple instances of this in a history without doing tedious cherry-picking? Thanks! ~ jf -- 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 grep: search whole tree by default?
Junio C Hamano writes: > Jed Brown writes: > >> Junio C Hamano writes: >>> I suspect that it would be too late for 2.0 we want to do sometime >>> early next year, though. >> >> How would you manage transition from the current behavior? Warning >> people to explicitly use "." or ":/" during some interim period sounds >> worse than just switching the default behavior. > > "How would I"? > > You're asking that question only because you omitted too much from > the quote ;-) I meant that if the proposed migration plan were to be "just change it and people will learn" (because anything more gradual would actually be worse for users) then is it really too late for Git-2.0? pgp3mQcztjeYU.pgp Description: PGP signature
Re: git grep: search whole tree by default?
Jed Brown writes: > Junio C Hamano writes: >> I suspect that it would be too late for 2.0 we want to do sometime >> early next year, though. > > How would you manage transition from the current behavior? Warning > people to explicitly use "." or ":/" during some interim period sounds > worse than just switching the default behavior. "How would I"? You're asking that question only because you omitted too much from the quote ;-) Matthieu Moy wrote: > In summary: changing is painful. The case of "git add" was really bad, > since the same command had different behavior depending on the options > given, so it was clearly worth the pain. In the case of "git grep", the > current behavior is not _that_ bad, so nobody bothered to do the change. > > (by "do the change", I mean propose a migration plan, convince people > that it is good, ...) I agree that it is up to those who really want to switch the default. -- 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 08/15] ref_remove_duplicates(): improve documentation comment
Michael Haggerty writes: > Signed-off-by: Michael Haggerty Up to this point the patches all look very sensible (modulo minor nits I sent separately). Will come back to the rest of the topics later. Thanks. > --- > remote.h | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/remote.h b/remote.h > index 131130a..40293c0 100644 > --- a/remote.h > +++ b/remote.h > @@ -149,7 +149,14 @@ int resolve_remote_symref(struct ref *ref, struct ref > *list); > int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1); > > /* > - * Removes and frees any duplicate refs in the map. > + * Remove and free all but the first of any entries in the input list > + * that map the same remote reference to the same local reference. If > + * there are two entries that map different remote references to the > + * same local reference, die. > + * > + * Note that the first entry is never removed; therefore, the pointer > + * passed in as argument still points to the head of the list after > + * the function returns. > */ > void ref_remove_duplicates(struct ref *ref_map); -- 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 05/15] get_ref_map(): rename local variables
Michael Haggerty writes:
> Rename "refs" -> "refspecs" and "ref_count" -> "refspec_count" to
> reduce confusion, because they describe an array of "struct refspec",
> as opposed to the "struct ref" objects that are also used in this
> function.
Good. In general, we'd prefer to name an array of things that are
primarily walked in the index order "thing[]", so that "thing number
3" can be spelled thing[3] (not things[3]) in the code, though.
> Signed-off-by: Michael Haggerty
> ---
> builtin/fetch.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index bd7a101..2248abf 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -165,8 +165,8 @@ static void find_non_local_tags(struct transport
> *transport,
> struct ref ***tail);
>
> static struct ref *get_ref_map(struct transport *transport,
> -struct refspec *refs, int ref_count, int tags,
> -int *autotags)
> +struct refspec *refspecs, int refspec_count,
> +int tags, int *autotags)
> {
> int i;
> struct ref *rm;
> @@ -175,12 +175,12 @@ static struct ref *get_ref_map(struct transport
> *transport,
>
> const struct ref *remote_refs = transport_get_remote_refs(transport);
>
> - if (ref_count || tags == TAGS_SET) {
> + if (refspec_count || tags == TAGS_SET) {
> struct ref **old_tail;
>
> - for (i = 0; i < ref_count; i++) {
> - get_fetch_map(remote_refs, &refs[i], &tail, 0);
> - if (refs[i].dst && refs[i].dst[0])
> + for (i = 0; i < refspec_count; i++) {
> + get_fetch_map(remote_refs, &refspecs[i], &tail, 0);
> + if (refspecs[i].dst && refspecs[i].dst[0])
> *autotags = 1;
> }
> /* Merge everything on the command line, but not --tags */
--
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 04/15] api-remote.txt: correct section "struct refspect"
Michael Haggerty writes: >> Subject: Re: [PATCH 04/15] api-remote.txt: correct section "struct refspect" refspect??? > * Replace reference to function parse_ref_spec() with references to > functions parse_fetch_refspec() and parse_push_refspec(). > > * Correct description of src and dst: they *do* include the '*' > characters. * Replace a single SP after a full-stop at the end of sentence with double SP as if we were back in the typewriter age. The last one made it hard to spot what actually got changed, though. The updated text otherwise looks correct. Thanks. > Signed-off-by: Michael Haggerty > --- > Documentation/technical/api-remote.txt | 20 ++-- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/Documentation/technical/api-remote.txt > b/Documentation/technical/api-remote.txt > index 4be8776..5d245aa 100644 > --- a/Documentation/technical/api-remote.txt > +++ b/Documentation/technical/api-remote.txt > @@ -58,16 +58,16 @@ default remote, given the current branch and > configuration. > struct refspec > -- > > -A struct refspec holds the parsed interpretation of a refspec. If it > -will force updates (starts with a '+'), force is true. If it is a > -pattern (sides end with '*') pattern is true. src and dest are the two > -sides (if a pattern, only the part outside of the wildcards); if there > -is only one side, it is src, and dst is NULL; if sides exist but are > -empty (i.e., the refspec either starts or ends with ':'), the > -corresponding side is "". > - > -This parsing can be done to an array of strings to give an array of > -struct refpsecs with parse_ref_spec(). > +A struct refspec holds the parsed interpretation of a refspec. If it > +will force updates (starts with a '+'), force is true. If it is a > +pattern (sides end with '*') pattern is true. src and dest are the > +two sides (including '*' characters if present); if there is only one > +side, it is src, and dst is NULL; if sides exist but are empty (i.e., > +the refspec either starts or ends with ':'), the corresponding side is > +"". > + > +An array of strings can be parsed into an array of struct refspecs > +using parse_fetch_refspec() or parse_push_refspec(). > > remote_find_tracking(), given a remote and a struct refspec with > either src or dst filled out, will fill out the other such that the -- 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 02/15] t5510: prepare test refs more straightforwardly
Michael Haggerty writes: > "git fetch" was being used with contrived refspecs to create tags and > remote-tracking branches in test repositories in preparation for the > actual tests. This is obscure and also makes one wonder whether this > is indeed just preparation or whether some side-effect of "git fetch" > is being tested. > > So use the more straightforward commands "git tag" / "git update-ref" > when preparing branches in test repositories. > > Signed-off-by: Michael Haggerty > --- > t/t5510-fetch.sh | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh > index c5e5dfc..08d8dbb 100755 > --- a/t/t5510-fetch.sh > +++ b/t/t5510-fetch.sh > @@ -88,7 +88,7 @@ test_expect_success 'fetch --prune on its own works as > expected' ' > cd "$D" && > git clone . prune && > cd prune && > - git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && > + git update-ref refs/remotes/origin/extrabranch master && As long as you have checked that our local 'master' should be at the same commit as the origin's 'master' at this point, I think this change is OK. I wouldn't call the use of "very explicit, without any room for mistake" refspecs "contrived", though. -- 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 grep: search whole tree by default?
Junio C Hamano writes: > I suspect that it would be too late for 2.0 we want to do sometime > early next year, though. How would you manage transition from the current behavior? Warning people to explicitly use "." or ":/" during some interim period sounds worse than just switching the default behavior. pgpfQuneqo8Z9.pgp Description: PGP signature
Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf
On Wed, Oct 23, 2013 at 11:09:27AM -0700, Junio C Hamano wrote: > > This is not something you introduced, but while we are here, you may > > want to use ce->namelen, which would be a little faster than treating it > > as a string (especially for strbuf, as it can then know up front how big > > the size is). > > H, do you mean something like this on top? > > diff --git a/entry.c b/entry.c > index d955af5..0d48292 100644 > --- a/entry.c > +++ b/entry.c > @@ -246,7 +246,9 @@ int checkout_entry(struct cache_entry *ce, > return write_entry(ce, topath, state, 1); > > strbuf_reset(&path_buf); > - strbuf_addf(&path_buf, "%.*s%s", state->base_dir_len, state->base_dir, > ce->name); > + strbuf_addf(&path_buf, "%.*s%.*s", > + state->base_dir_len, state->base_dir, > + ce_namelen(ce), ce->name); > path = path_buf.buf; > len = path_buf.len; Yes, though I actually find Erik's version with two separate strbuf_add invocations slightly more readable (it _could_ result in two allocations, but again, we are amortizing the growth over many calls anyway, so most of them will not need to grow the buffer at all). -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 1/2] entry.c: convert checkout_entry to use strbuf
Jeff King writes: > On Wed, Oct 23, 2013 at 07:55:06PM +0700, Nguyen Thai Ngoc Duy wrote: > ... >> -memcpy(path, state->base_dir, len); >> -strcpy(path + len, ce->name); >> -len += ce_namelen(ce); >> +strbuf_reset(&path_buf); >> +strbuf_addf(&path_buf, "%.*s%s", state->base_dir_len, state->base_dir, >> ce->name); >> +path = path_buf.buf; >> +len = path_buf.len; > > This is not something you introduced, but while we are here, you may > want to use ce->namelen, which would be a little faster than treating it > as a string (especially for strbuf, as it can then know up front how big > the size is). H, do you mean something like this on top? diff --git a/entry.c b/entry.c index d955af5..0d48292 100644 --- a/entry.c +++ b/entry.c @@ -246,7 +246,9 @@ int checkout_entry(struct cache_entry *ce, return write_entry(ce, topath, state, 1); strbuf_reset(&path_buf); - strbuf_addf(&path_buf, "%.*s%s", state->base_dir_len, state->base_dir, ce->name); + strbuf_addf(&path_buf, "%.*s%.*s", + state->base_dir_len, state->base_dir, + ce_namelen(ce), ce->name); path = path_buf.buf; len = path_buf.len; -- 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] entry.c: convert write_entry to use strbuf
Nguyễn Thái Ngọc Duy writes:
> The strcpy call in open_output_fd() implies that the output buffer
> must be at least 25 chars long.
Hmph, where does that 25 come from?
> And it's true. The only caller that
> can trigger that code is checkout-index, which has the buffer of
> PATH_MAX chars (and any systems that have PATH_MAX shorter than 25
> chars are just insane).
>
> But in order to say that, one has to walk through a dozen of
> functions. Just convert it to strbuf to avoid the constraint and
> confusion.
Wouldn't it be far clearer to document what is going on especially
around the topath parameter to checkout_entry(), than to introduce
unnecessary strbuf overhead?
At first glance, it might appear that the caller of checkout_entry()
can specify to which path the contents are written out, but in
reality topath[] is to point at the buffer to store the temporary
path generated by the lower guts of write_entry(). It is unclear in
the original code and that is worth an in-code comment.
And when describing that API requirement, we would need to say how
big a buffer the caller must allocate for topath[] in the comment.
That size does not have to be platform-dependent PATH_MAX.
Something like this?
builtin/checkout-index.c | 2 +-
cache.h | 1 +
entry.c | 8
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index b1feda7..4ed6b23 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -14,7 +14,7 @@
static int line_termination = '\n';
static int checkout_stage; /* default to checkout stage0 */
static int to_tempfile;
-static char topath[4][PATH_MAX + 1];
+static char topath[4][TEMPORARY_FILENAME_LENGTH + 1];
static struct checkout state;
diff --git a/cache.h b/cache.h
index 85b544f..3118b7f 100644
--- a/cache.h
+++ b/cache.h
@@ -975,6 +975,7 @@ struct checkout {
refresh_cache:1;
};
+#define TEMPORARY_FILENAME_LENGTH 25
extern int checkout_entry(struct cache_entry *ce, const struct checkout
*state, char *topath);
struct cache_def {
diff --git a/entry.c b/entry.c
index d955af5..2df4ee1 100644
--- a/entry.c
+++ b/entry.c
@@ -234,6 +234,14 @@ static int check_path(const char *path, int len, struct
stat *st, int skiplen)
return lstat(path, st);
}
+/*
+ * Write the contents from ce out to the working tree.
+ *
+ * When topath[] is not NULL, instead of writing to the working tree
+ * file named by ce, a temporary file is created by this function and
+ * its name is returned in topath[], which must be able to hold at
+ * least TEMPORARY_FILENAME_LENGTH bytes long.
+ */
int checkout_entry(struct cache_entry *ce,
const struct checkout *state, char *topath)
{
--
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/2] entry.c: convert checkout_entry to use strbuf
On Wed, Oct 23, 2013 at 07:34:18PM +0200, Erik Faye-Lund wrote: > >> - memcpy(path, state->base_dir, len); > >> - strcpy(path + len, ce->name); > >> - len += ce_namelen(ce); > >> + strbuf_reset(&path_buf); > >> + strbuf_addf(&path_buf, "%.*s%s", state->base_dir_len, > >> state->base_dir, ce->name); > >> + path = path_buf.buf; > >> + len = path_buf.len; > > > > This is not something you introduced, but while we are here, you may > > want to use ce->namelen, which would be a little faster than treating it > > as a string (especially for strbuf, as it can then know up front how big > > the size is). > > > > I doubt it's measurable, though (especially as the growth cost is > > amortized due to the static buffer). > > I somehow feel that: > > strbuf_reset(&path_buf); > strbuf_add(&path_buf, state->base_dir, state->base_dir_len); > strbuf_addch(&path_buf, '/'); > strbuf_add(&path_buf, state->name, state->name_len); > > feels a bit neater than using strbuf_addf. But that might just be me. I agree. But note that your addch is a bug. :) -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 1/2] entry.c: convert checkout_entry to use strbuf
On Wed, Oct 23, 2013 at 7:29 PM, Jeff King wrote: > On Wed, Oct 23, 2013 at 07:55:06PM +0700, Nguyen Thai Ngoc Duy wrote: > >> The old code does not do boundary check so any paths longer than >> PATH_MAX can cause buffer overflow. Replace it with strbuf to handle >> paths of arbitrary length. > > I think this is a reasonable solution. If we have such a long path, we > are probably about to feed it to open() or another syscall, and we will > just get ENAMETOOLONG there anyway. But certainly we need to fix the > buffer overflow, and we are probably better off letting the syscall > report failure than calling die(), because we generally handle the > syscall failure more gracefully (e.g., by reporting the failed path but > continuing). > >> - memcpy(path, state->base_dir, len); >> - strcpy(path + len, ce->name); >> - len += ce_namelen(ce); >> + strbuf_reset(&path_buf); >> + strbuf_addf(&path_buf, "%.*s%s", state->base_dir_len, state->base_dir, >> ce->name); >> + path = path_buf.buf; >> + len = path_buf.len; > > This is not something you introduced, but while we are here, you may > want to use ce->namelen, which would be a little faster than treating it > as a string (especially for strbuf, as it can then know up front how big > the size is). > > I doubt it's measurable, though (especially as the growth cost is > amortized due to the static buffer). I somehow feel that: strbuf_reset(&path_buf); strbuf_add(&path_buf, state->base_dir, state->base_dir_len); strbuf_addch(&path_buf, '/'); strbuf_add(&path_buf, state->name, state->name_len); feels a bit neater than using strbuf_addf. But that might just be me. -- 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/2] entry.c: convert checkout_entry to use strbuf
On Wed, Oct 23, 2013 at 07:55:06PM +0700, Nguyen Thai Ngoc Duy wrote: > The old code does not do boundary check so any paths longer than > PATH_MAX can cause buffer overflow. Replace it with strbuf to handle > paths of arbitrary length. I think this is a reasonable solution. If we have such a long path, we are probably about to feed it to open() or another syscall, and we will just get ENAMETOOLONG there anyway. But certainly we need to fix the buffer overflow, and we are probably better off letting the syscall report failure than calling die(), because we generally handle the syscall failure more gracefully (e.g., by reporting the failed path but continuing). > - memcpy(path, state->base_dir, len); > - strcpy(path + len, ce->name); > - len += ce_namelen(ce); > + strbuf_reset(&path_buf); > + strbuf_addf(&path_buf, "%.*s%s", state->base_dir_len, state->base_dir, > ce->name); > + path = path_buf.buf; > + len = path_buf.len; This is not something you introduced, but while we are here, you may want to use ce->namelen, which would be a little faster than treating it as a string (especially for strbuf, as it can then know up front how big the size is). I doubt it's measurable, though (especially as the growth cost is amortized due to the static buffer). -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: git grep: search whole tree by default?
Matthieu Moy writes: > In summary: changing is painful. The case of "git add" was really bad, > since the same command had different behavior depending on the options > given, so it was clearly worth the pain. In the case of "git grep", the > current behavior is not _that_ bad, so nobody bothered to do the change. > > (by "do the change", I mean propose a migration plan, convince people > that it is good, ...) > > I'd personally be slightly in favor of changing to tree-wide, but > without strong opinion. After reading that old thread again, I tend to think that the only reason to favor "git grep" to start at the $(cwd) is the backward compatibility. While I do expect that many people will be annoyed when "git grep" (no pathspecs) that is run in a subdirectory starts spitting out a large number of hits from places irrelevant for the current task at hand, hits from outside the $(cwd) is something they can _notice_ easily and their fingers will quickly learn to add "." without even thinking. I suspect that it would be too late for 2.0 we want to do sometime early next year, though. -- 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 00/15] Change semantics of "fetch --tags"
Michael Haggerty writes: > This is my proposed fix for the "local tag killer" problem that I > reported recently [1]. > > There are three main things changed by this patch series: > ... Haven't looked at any of the 1-15 messages, but the basic design to demote "--tags" from being an explicit "refs/tags/*:refs/tags/*" given from the command line to a more special case option sounds very sane and solid. With the auto-following of tags, I think it is a _mistake_ to have "--tags" anywhere on the command line (and "tagopt" config) these days, and I do not expect a huge fallout from incompatibility that arises from the change in behaviour. Thanks---looking forward to reading it through. -- 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 grep: search whole tree by default?
Piotr Krukowiecki writes: > I think there were discussion about how there are several git commands > which do not search in whole tree by default and that it's going to be > changed. I think "add" is one of such commands. Is 'grep' left > unchanged? In summary: changing is painful. The case of "git add" was really bad, since the same command had different behavior depending on the options given, so it was clearly worth the pain. In the case of "git grep", the current behavior is not _that_ bad, so nobody bothered to do the change. (by "do the change", I mean propose a migration plan, convince people that it is good, ...) I'd personally be slightly in favor of changing to tree-wide, but without strong opinion. -- 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] Clear fd after closing to avoid double-close error
Jens Lindström writes: > On Tue, Oct 22, 2013 at 8:42 PM, Junio C Hamano wrote: >> Duy Nguyen writes: > >>> Life would have been simpler if fd[1] was _always_ closed by >>> send_pack(), like in c20181e (start_command(), if .in/.out > 0, closes >>> file descriptors, not the callers - 2008-02-21). >> >> Yeah, that was also my first reaction when I saw the above three >> lines after reading the discussion that led to the diagnosis. > > If send_pack() always closes fd[1], then I believe "git send-pack > --stateless-rpc --helper-status" would die in print_helper_status(), > called after send_pack(), since fd[1] would be 1, to which > print_helper_status() will try to write. Ah, I obviously did not look far enough. Of course we could dup(2) the fd=1 to code it around, but it is not clear to me if it is worth it---your solution (v2) is clearer, so let's queue it with Acks we saw from Peff and Duy. 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: RFE: support change-id generation natively
Duy Nguyen writes: > On Wed, Oct 23, 2013 at 2:50 AM, Junio C Hamano wrote: >> It would be just the matter of updating commit_tree_extended() in >> commit.c to: >> >> - detect the need to add a new Change-Id: trailer; >> >> - call hash_sha1_file() on the commit object buffer (assuming that >>a commit object that you can actually "git cat-file commit" using >>the change Id does not have to exist anywhere for Gerrit to >>work---otherwise you would need to call write_sha1_file() >>instead) before adding Change-Id: trailer; >> >> - add Change-Id: trailer to the buffer; and then finally >> >> - let the existing write_sha1_file() to write it out. > > I'm not objecting special support for Gerrit, but if the change is > just commit_tree_extended() why don't we just ship the commit hook in > a new "Gerrit" template? It is not clear to me how you envision to make it work. Naïvely thinking, an obvious place to do this kind of thing may be the "commit-msg" hook, where the hook reads what the user prepared, finds that there is no existing "Change-Id:" trailer, and decides to add one. But what value would it add on that line as the Id? It wants to use the name of the commit object that would result if it were to return without further editing the given message, but we do not give such a commit object name to the hook, so the hook needs to duplicate the logic to come up with one. It may be doable (after all, builtin/commit.c is open source), but we do not give the hook the commit object header (i.e. it does not know what the tree, parent(s), author, committer lines would say, nor it does not know if we are going to add an encoding line), so the hook needs to guess what we will put there, too. -- 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 13/15] builtin/remote.c: reorder function definitions
Reorder function definitions to remove the need for forward
declarations.
Signed-off-by: Michael Haggerty
---
builtin/remote.c | 159 +++
1 file changed, 78 insertions(+), 81 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 4e14891..ecedd96 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -77,9 +77,6 @@ static const char * const builtin_remote_seturl_usage[] = {
static int verbose;
-static int show_all(void);
-static int prune_remote(const char *remote, int dry_run);
-
static inline int postfixcmp(const char *string, const char *postfix)
{
int len1 = strlen(string), len2 = strlen(postfix);
@@ -1084,6 +1081,64 @@ static int show_push_info_item(struct string_list_item
*item, void *cb_data)
return 0;
}
+static int get_one_entry(struct remote *remote, void *priv)
+{
+ struct string_list *list = priv;
+ struct strbuf url_buf = STRBUF_INIT;
+ const char **url;
+ int i, url_nr;
+
+ if (remote->url_nr > 0) {
+ strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
+ string_list_append(list, remote->name)->util =
+ strbuf_detach(&url_buf, NULL);
+ } else
+ string_list_append(list, remote->name)->util = NULL;
+ if (remote->pushurl_nr) {
+ url = remote->pushurl;
+ url_nr = remote->pushurl_nr;
+ } else {
+ url = remote->url;
+ url_nr = remote->url_nr;
+ }
+ for (i = 0; i < url_nr; i++)
+ {
+ strbuf_addf(&url_buf, "%s (push)", url[i]);
+ string_list_append(list, remote->name)->util =
+ strbuf_detach(&url_buf, NULL);
+ }
+
+ return 0;
+}
+
+static int show_all(void)
+{
+ struct string_list list = STRING_LIST_INIT_NODUP;
+ int result;
+
+ list.strdup_strings = 1;
+ result = for_each_remote(get_one_entry, &list);
+
+ if (!result) {
+ int i;
+
+ sort_string_list(&list);
+ for (i = 0; i < list.nr; i++) {
+ struct string_list_item *item = list.items + i;
+ if (verbose)
+ printf("%s\t%s\n", item->string,
+ item->util ? (const char *)item->util :
"");
+ else {
+ if (i && !strcmp((item - 1)->string,
item->string))
+ continue;
+ printf("%s\n", item->string);
+ }
+ }
+ }
+ string_list_clear(&list, 1);
+ return result;
+}
+
static int show(int argc, const char **argv)
{
int no_query = 0, result = 0, query_flag = 0;
@@ -1246,26 +1301,6 @@ static int set_head(int argc, const char **argv)
return result;
}
-static int prune(int argc, const char **argv)
-{
- int dry_run = 0, result = 0;
- struct option options[] = {
- OPT__DRY_RUN(&dry_run, N_("dry run")),
- OPT_END()
- };
-
- argc = parse_options(argc, argv, NULL, options,
builtin_remote_prune_usage,
-0);
-
- if (argc < 1)
- usage_with_options(builtin_remote_prune_usage, options);
-
- for (; argc; argc--, argv++)
- result |= prune_remote(*argv, dry_run);
-
- return result;
-}
-
static int prune_remote(const char *remote, int dry_run)
{
int result = 0, i;
@@ -1304,6 +1339,26 @@ static int prune_remote(const char *remote, int dry_run)
return result;
}
+static int prune(int argc, const char **argv)
+{
+ int dry_run = 0, result = 0;
+ struct option options[] = {
+ OPT__DRY_RUN(&dry_run, N_("dry run")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, NULL, options,
builtin_remote_prune_usage,
+0);
+
+ if (argc < 1)
+ usage_with_options(builtin_remote_prune_usage, options);
+
+ for (; argc; argc--, argv++)
+ result |= prune_remote(*argv, dry_run);
+
+ return result;
+}
+
static int get_remote_default(const char *key, const char *value, void *priv)
{
if (strcmp(key, "remotes.default") == 0) {
@@ -1505,64 +1560,6 @@ static int set_url(int argc, const char **argv)
return 0;
}
-static int get_one_entry(struct remote *remote, void *priv)
-{
- struct string_list *list = priv;
- struct strbuf url_buf = STRBUF_INIT;
- const char **url;
- int i, url_nr;
-
- if (remote->url_nr > 0) {
- strbuf_addf(&url_buf, "%s (fetch)", remote->url[0]);
- string_list_append(list, remote->name)->util =
- strbuf_detach(&url_buf, NULL);
- } else
- string_list_append(list, remote-
[PATCH 05/15] get_ref_map(): rename local variables
Rename "refs" -> "refspecs" and "ref_count" -> "refspec_count" to
reduce confusion, because they describe an array of "struct refspec",
as opposed to the "struct ref" objects that are also used in this
function.
Signed-off-by: Michael Haggerty
---
builtin/fetch.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bd7a101..2248abf 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -165,8 +165,8 @@ static void find_non_local_tags(struct transport *transport,
struct ref ***tail);
static struct ref *get_ref_map(struct transport *transport,
- struct refspec *refs, int ref_count, int tags,
- int *autotags)
+ struct refspec *refspecs, int refspec_count,
+ int tags, int *autotags)
{
int i;
struct ref *rm;
@@ -175,12 +175,12 @@ static struct ref *get_ref_map(struct transport
*transport,
const struct ref *remote_refs = transport_get_remote_refs(transport);
- if (ref_count || tags == TAGS_SET) {
+ if (refspec_count || tags == TAGS_SET) {
struct ref **old_tail;
- for (i = 0; i < ref_count; i++) {
- get_fetch_map(remote_refs, &refs[i], &tail, 0);
- if (refs[i].dst && refs[i].dst[0])
+ for (i = 0; i < refspec_count; i++) {
+ get_fetch_map(remote_refs, &refspecs[i], &tail, 0);
+ if (refspecs[i].dst && refspecs[i].dst[0])
*autotags = 1;
}
/* Merge everything on the command line, but not --tags */
--
1.8.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 09/15] builtin/fetch.c: reorder function definitions
Reorder function definitions to avoid the need for a forward
declaration of function find_non_local_tags().
Signed-off-by: Michael Haggerty
---
builtin/fetch.c | 198 +++-
1 file changed, 97 insertions(+), 101 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2248abf..61e8117 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -160,9 +160,105 @@ static void add_merge_config(struct ref **head,
}
}
+static int add_existing(const char *refname, const unsigned char *sha1,
+ int flag, void *cbdata)
+{
+ struct string_list *list = (struct string_list *)cbdata;
+ struct string_list_item *item = string_list_insert(list, refname);
+ item->util = xmalloc(20);
+ hashcpy(item->util, sha1);
+ return 0;
+}
+
+static int will_fetch(struct ref **head, const unsigned char *sha1)
+{
+ struct ref *rm = *head;
+ while (rm) {
+ if (!hashcmp(rm->old_sha1, sha1))
+ return 1;
+ rm = rm->next;
+ }
+ return 0;
+}
+
static void find_non_local_tags(struct transport *transport,
struct ref **head,
- struct ref ***tail);
+ struct ref ***tail)
+{
+ struct string_list existing_refs = STRING_LIST_INIT_DUP;
+ struct string_list remote_refs = STRING_LIST_INIT_NODUP;
+ const struct ref *ref;
+ struct string_list_item *item = NULL;
+
+ for_each_ref(add_existing, &existing_refs);
+ for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {
+ if (prefixcmp(ref->name, "refs/tags/"))
+ continue;
+
+ /*
+* The peeled ref always follows the matching base
+* ref, so if we see a peeled ref that we don't want
+* to fetch then we can mark the ref entry in the list
+* as one to ignore by setting util to NULL.
+*/
+ if (!suffixcmp(ref->name, "^{}")) {
+ if (item && !has_sha1_file(ref->old_sha1) &&
+ !will_fetch(head, ref->old_sha1) &&
+ !has_sha1_file(item->util) &&
+ !will_fetch(head, item->util))
+ item->util = NULL;
+ item = NULL;
+ continue;
+ }
+
+ /*
+* If item is non-NULL here, then we previously saw a
+* ref not followed by a peeled reference, so we need
+* to check if it is a lightweight tag that we want to
+* fetch.
+*/
+ if (item && !has_sha1_file(item->util) &&
+ !will_fetch(head, item->util))
+ item->util = NULL;
+
+ item = NULL;
+
+ /* skip duplicates and refs that we already have */
+ if (string_list_has_string(&remote_refs, ref->name) ||
+ string_list_has_string(&existing_refs, ref->name))
+ continue;
+
+ item = string_list_insert(&remote_refs, ref->name);
+ item->util = (void *)ref->old_sha1;
+ }
+ string_list_clear(&existing_refs, 1);
+
+ /*
+* We may have a final lightweight tag that needs to be
+* checked to see if it needs fetching.
+*/
+ if (item && !has_sha1_file(item->util) &&
+ !will_fetch(head, item->util))
+ item->util = NULL;
+
+ /*
+* For all the tags in the remote_refs string list,
+* add them to the list of refs to be fetched
+*/
+ for_each_string_list_item(item, &remote_refs) {
+ /* Unless we have already decided to ignore this item... */
+ if (item->util)
+ {
+ struct ref *rm = alloc_ref(item->string);
+ rm->peer_ref = alloc_ref(item->string);
+ hashcpy(rm->old_sha1, item->util);
+ **tail = rm;
+ *tail = &rm->next;
+ }
+ }
+
+ string_list_clear(&remote_refs, 0);
+}
static struct ref *get_ref_map(struct transport *transport,
struct refspec *refspecs, int refspec_count,
@@ -612,106 +708,6 @@ static int prune_refs(struct refspec *refs, int
ref_count, struct ref *ref_map)
return result;
}
-static int add_existing(const char *refname, const unsigned char *sha1,
- int flag, void *cbdata)
-{
- struct string_list *list = (struct string_list *)cbdata;
- struct string_list_item *item = string_list_insert(list, refname);
- item->util = xmalloc(20);
- hashcpy(item->util, sha1);
- return 0;
-}
-
-static int will_fetch(struct ref **head, const unsigned char
[PATCH 10/15] fetch --tags: fetch tags *in addition to* other stuff
Previously, fetch's "--tags" option was considered equivalent to
specifying the refspec "refs/tags/*:refs/tags/*" on the command line;
in particular, it caused the remote..refspec configuration to be
ignored.
But it is not very useful to fetch tags without also fetching other
references, whereas it *is* quite useful to be able to fetch tags *in
addition to* other references. So change the semantics of this option
to do the latter.
If a user wants to fetch *only* tags, then it is still possible to
specifying an explicit refspec:
git fetch 'refs/tags/*:refs/tags/*'
Please note that the documentation prior to 1.8.0.3 was ambiguous
about this aspect of "fetch --tags" behavior. Commit
f0cb2f137c 2012-12-14 fetch --tags: clarify documentation
made the documentation match the old behavior. This commit changes
the documentation to match the new behavior.
Signed-off-by: Michael Haggerty
---
The change to builtin/fetch.c:get_ref_map() has the side effect of
changing the order that the (struct ref) objects are listed in
ref_map. It seems to me that this could probably only matter in the
case of duplicates. But later ref_remove_duplicates() is called, so
duplicates are eliminated. However, ref_remove_duplicates() always
retains the first instance of duplicates and discards the rest. It is
conceivable that the boolean flags (which are not inspected by
ref_remove_duplicates()) could differ among the duplicates, and that
therefore changing the order of the items in the original list has the
effect of changing the flags on the items that are retained.
I don't know this code well enough to judge whether this might be a
problem.
If it is, then the correct approach is probably either to teach
ref_remove_duplicates() to ensure that the flags are also consistent
across duplicates, or to somehow combine the flags from all duplicates
into the one that is retained. Please let me know if this is needed.
Documentation/fetch-options.txt | 8 +++---
builtin/fetch.c | 46 +++-
git-pull.sh | 2 +-
t/t5510-fetch.sh | 22 ---
t/t5515/fetch.br-unconfig_--tags_.._.git | 1 +
t/t5515/fetch.master_--tags_.._.git | 1 +
t/t5525-fetch-tagopt.sh | 23
7 files changed, 71 insertions(+), 32 deletions(-)
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index ba1fe49..0e6d2ac 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -61,11 +61,9 @@ endif::git-pull[]
ifndef::git-pull[]
-t::
--tags::
- This is a short-hand for giving `refs/tags/*:refs/tags/*`
- refspec from the command line, to ask all tags to be fetched
- and stored locally. Because this acts as an explicit
- refspec, the default refspecs (configured with the
- remote.$name.fetch variable) are overridden and not used.
+ This is a short-hand requesting that all tags be fetched from
+ the remote in addition to whatever else is being fetched. It
+ is similar to using the refspec `refs/tags/*:refs/tags/*`.
--recurse-submodules[=yes|on-demand|no]::
This option controls if and under what conditions new commits of
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 61e8117..7edb1ea 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -271,7 +271,7 @@ static struct ref *get_ref_map(struct transport *transport,
const struct ref *remote_refs = transport_get_remote_refs(transport);
- if (refspec_count || tags == TAGS_SET) {
+ if (refspec_count) {
struct ref **old_tail;
for (i = 0; i < refspec_count; i++) {
@@ -279,11 +279,9 @@ static struct ref *get_ref_map(struct transport *transport,
if (refspecs[i].dst && refspecs[i].dst[0])
*autotags = 1;
}
- /* Merge everything on the command line, but not --tags */
+ /* Merge everything on the command line (but not --tags) */
for (rm = ref_map; rm; rm = rm->next)
rm->fetch_head_status = FETCH_HEAD_MERGE;
- if (tags == TAGS_SET)
- get_fetch_map(remote_refs, tag_refspec, &tail, 0);
/*
* For any refs that we happen to be fetching via command-line
@@ -334,8 +332,13 @@ static struct ref *get_ref_map(struct transport *transport,
tail = &ref_map->next;
}
}
- if (tags == TAGS_DEFAULT && *autotags)
+
+ if (tags == TAGS_SET)
+ /* also fetch all tags */
+ get_fetch_map(remote_refs, tag_refspec, &tail, 0);
+ else if (tags == TAGS_DEFAULT && *autotags)
find_non_local_tags(transport, &ref_map, &tail);
+
ref_remove_duplicates(ref_map);
return ref_map;
@@ -826,3
[PATCH 15/15] fetch, remote: properly convey --no-prune options to subprocesses
If --no-prune is passed to one of the following commands:
git fetch --all
git fetch --multiple
git fetch --recurse-submodules
git remote update
then it must also be passed to the "fetch" subprocesses that those
commands use to do their work. Otherwise there might be a fetch.prune
or remote..prune configuration setting that causes pruning to
occur, contrary to the user's express wish.
Signed-off-by: Michael Haggerty
---
builtin/fetch.c | 4 ++--
builtin/remote.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 47b63a7..8711df0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -915,8 +915,8 @@ static void add_options_to_argv(struct argv_array *argv)
{
if (dry_run)
argv_array_push(argv, "--dry-run");
- if (prune > 0)
- argv_array_push(argv, "--prune");
+ if (prune != -1)
+ argv_array_push(argv, prune ? "--prune" : "--no-prune");
if (update_head_ok)
argv_array_push(argv, "--update-head-ok");
if (force)
diff --git a/builtin/remote.c b/builtin/remote.c
index bffe2f9..f532f35 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1371,7 +1371,7 @@ static int get_remote_default(const char *key, const char
*value, void *priv)
static int update(int argc, const char **argv)
{
- int i, prune = 0;
+ int i, prune = -1;
struct option options[] = {
OPT_BOOL('p', "prune", &prune,
N_("prune remotes after fetching")),
@@ -1386,8 +1386,8 @@ static int update(int argc, const char **argv)
argv_array_push(&fetch_argv, "fetch");
- if (prune)
- argv_array_push(&fetch_argv, "--prune");
+ if (prune != -1)
+ argv_array_push(&fetch_argv, prune ? "--prune" : "--no-prune");
if (verbose)
argv_array_push(&fetch_argv, "-v");
argv_array_push(&fetch_argv, "--multiple");
--
1.8.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 11/15] fetch --prune: prune only based on explicit refspecs
The old behavior of "fetch --prune" was to prune whatever was being
fetched. In particular, "fetch --prune --tags" caused tags not only
to be fetched, but also to be pruned. This is inappropriate because
there is only one tags namespace that is shared among the local
repository and all remotes. Therefore, if the user defines a local
tag and then runs "git fetch --prune --tags", then the local tag is
deleted. Moreover, "--prune" and "--tags" can also be configured via
fetch.prune / remote..prune and remote..tagopt, making it
even less obvious that an invocation of "git fetch" could result in
tag lossage.
Since the command "git remote update" invokes "git fetch", it had the
same problem.
The command "git remote prune", on the other hand, disregarded the
setting of remote..tagopt, and so its behavior was inconsistent
with that of the other commands.
So the old behavior made it too easy to lose tags. To fix this
problem, change "fetch --prune" to prune references based only on
refspecs specified explicitly by the user, either on the command line
or via remote..fetch. Thus, tags are no longer made subject to
pruning by the --tags option or the remote..tagopt setting.
However, tags *are* still subject to pruning if they are fetched as
part of a refspec, and that is good. For example:
* On the command line,
git fetch --prune 'refs/tags/*:refs/tags/*'
causes tags, and only tags, to be fetched and pruned, and is
therefore a simple way for the user to get the equivalent of the old
behavior of "--prune --tag".
* For a remote that was configured with the "--mirror" option, the
configuration is set to include
[remote "name"]
fetch = +refs/*:refs/*
, which causes tags to be subject to pruning along with all other
references. This is the behavior that will typically be desired for
a mirror.
Signed-off-by: Michael Haggerty
---
Documentation/config.txt| 2 +-
Documentation/fetch-options.txt | 15 ---
builtin/fetch.c | 39 +--
t/t5510-fetch.sh| 10 +-
4 files changed, 27 insertions(+), 39 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d4d93c9..83c1700 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2086,7 +2086,7 @@ remote..vcs::
remote..prune::
When set to true, fetching from this remote by default will also
remove any remote-tracking branches which no longer exist on the
- remote (as if the `--prune` option was give on the command line).
+ remote (as if the `--prune` option was given on the command line).
Overrides `fetch.prune` settings, if any.
remotes.::
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 0e6d2ac..5d12219 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -41,8 +41,14 @@ ifndef::git-pull[]
-p::
--prune::
- After fetching, remove any remote-tracking branches which
- no longer exist on the remote.
+ After fetching, remove any remote-tracking branches that
+ no longer exist on the remote. Tags are not subject to
+ pruning in the usual case that they are fetched because of the
+ --tags option or remote..tagopt. However, if tags are
+ fetched due to an explicit refspec (either on the command line
+ or in the remote configuration, for example if the remote was
+ cloned with the --mirror option), then they are also subject
+ to pruning.
endif::git-pull[]
ifdef::git-pull[]
@@ -63,7 +69,10 @@ ifndef::git-pull[]
--tags::
This is a short-hand requesting that all tags be fetched from
the remote in addition to whatever else is being fetched. It
- is similar to using the refspec `refs/tags/*:refs/tags/*`.
+ is similar to using the refspec `refs/tags/*:refs/tags/*`,
+ except that it doesn't subject tags to pruning, regardless of
+ a --prune option or the configuration settings of fetch.prune
+ or remote..prune.
--recurse-submodules[=yes|on-demand|no]::
This option controls if and under what conditions new commits of
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7edb1ea..47b63a7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -829,38 +829,17 @@ static int do_fetch(struct transport *transport,
goto cleanup;
}
if (prune) {
- struct refspec *prune_refspecs;
- int prune_refspec_count;
-
+ /*
+* We only prune based on refspecs specified
+* explicitly (via command line or configuration); we
+* don't care whether --tags was specified.
+*/
if (ref_count) {
- prune_refspecs = refs;
- prune_refspec_count = ref_count;
- } else {
- prune_
[PATCH 00/15] Change semantics of "fetch --tags"
This is my proposed fix for the "local tag killer" problem that I reported recently [1]. There are three main things changed by this patch series: 1. "git fetch --tags" causes tags to be fetched *in addition to* any other refspecs that are configured for the remote, rather than *instead of*. I believe this is more useful behavior. It is also consistent with the documentation as it was written before it was disambiguated in 1.8.0.3. 2. "git fetch --prune" only prunes references that match an explicit refspec (either from the command line or from the remote..fetch configuration. In particular, using "--prune" and "--tag" together do *not* make tags subject to pruning. (Tags can still be pruned if the user specifies an explicit refspec "refs/tags/*:refs/tags/*".) 3. Previously, if the user invoked one of the following commands with --no-prune, the --no-prune option was not passed to the "git fetch" subprocesses that they invoked to do their work: git fetch --all git fetch --multiple git fetch --recurse-submodules git remote update If fetch.prune or remote..prune were set to true, this could result in unwanted reference pruning. The last commit in the series fixes this bug and should not be controversial. I had originally planned to solve the "local tag killer" problem by adding a new configuration option to define which reference namespaces were subject to pruning (e.g., remote..pruneRef="refs/remotes/*"). I may yet submit that patch series as a separate feature. But while working on it I hit on the present solution, which I think is simpler and more elegant (albeit a bit less flexible). Changes (1) and (2) introduce behavior changes, but I think that they are improvements and that the resulting backwards-incompatibility is acceptable: Change (1) means that "git fetch --tags " without any additional refspec arguments will fetch more references than it did before. But I don't think it is very useful to want to fetch tags without fetching other configured references, so I think it is OK [2]. Change (2) means that using "git fetch --tags --prune" will *not* prune tags. (This is the whole point of the change!) As discussed in the mailing list, it is usually bad policy to prune tags, because tags for the local repository and for all remote repositories currently share a single namespace, "refs/tags/*". Therefore, pruning tags based on information from a single remote risks pruning local tags or tags that have been obtained from another remote. The main exception, when one probably *does* want to prune tags, is when fetching into a mirror clone. But mirror clones have "remote..fetch=+refs/*:refs/*", and so even after this change tags will be subject to pruning when fetching into a mirror clone. The only other place I can find that does reference pruning is "git remote prune", but that codepath didn't respect remote..tagopt anyway and therefore it *didn't* prune tags unless they were part of an explicit refspec; i.e., this codepath already behaved the "new" way that other pruning codepaths now behave. Patches 1-9 are just preliminary cleanup and documentation improvements. Patch 10 implements change (1) described above. Patch 11 implements change (2). Patches 12-14 are some more minor cleanups. Patch 15 implements change (3). [1] http://article.gmane.org/gmane.comp.version-control.git/234723 [2] Indeed, I bet that most scripts that invoke "git fetch --tags " also invoke a plain "git fetch" immediately before or after to get the rest of the references. Michael Haggerty (15): t5510: use the correct tag name in test t5510: prepare test refs more straightforwardly t5510: check that "git fetch --prune --tags" does not prune branches api-remote.txt: correct section "struct refspect" get_ref_map(): rename local variables ref_remove_duplicates(): avoid redundant bisection ref_remove_duplicates(): simplify function ref_remove_duplicates(): improve documentation comment builtin/fetch.c: reorder function definitions fetch --tags: fetch tags *in addition to* other stuff fetch --prune: prune only based on explicit refspecs query_refspecs(): move some constants out of the loop builtin/remote.c: reorder function definitions builtin/remote.c:update(): use struct argv_array fetch, remote: properly convey --no-prune options to subprocesses Documentation/config.txt | 2 +- Documentation/fetch-options.txt | 21 ++- Documentation/technical/api-remote.txt | 20 +-- builtin/fetch.c | 253 +++ builtin/remote.c | 196 git-pull.sh | 2 +- remote.c | 44 +++--- remote.h | 9 +- t/t5510-fetch.sh | 36 - t/t5515/fetch.br-unconfig_--tags_.._.git | 1
[PATCH 03/15] t5510: check that "git fetch --prune --tags" does not prune branches
"git fetch --prune --tags" is currently interpreted as follows: * "--tags" is equivalent to specifying a refspec "refs/tags/*:refs/tags/*", and supersedes any default refspecs configured via remote.$REMOTE.fetch. * "--prune" only operates on the refspecs being fetched. Therefore, "git fetch --prune --tags" prunes tags in refs/tags/* but does not fetch or prune other references. The fact that this command does not prune references outside of refs/tags/* was previously untested. So add a test that verifies the status quo. However, the status quo is surprising, so it will be changed later in this patch series. Signed-off-by: Michael Haggerty --- t/t5510-fetch.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 08d8dbb..8328be1 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -118,9 +118,13 @@ test_expect_success 'fetch --prune --tags does not delete the remote-tracking br git clone . prune-tags && cd prune-tags && git tag sometag master && + # Create what looks like a remote-tracking branch from an earlier + # fetch that has since been deleted from the remote: + git update-ref refs/remotes/origin/fake-remote master && git fetch --prune --tags origin && git rev-parse origin/master && + git rev-parse origin/fake-remote && test_must_fail git rev-parse sometag ' -- 1.8.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 04/15] api-remote.txt: correct section "struct refspect"
* Replace reference to function parse_ref_spec() with references to functions parse_fetch_refspec() and parse_push_refspec(). * Correct description of src and dst: they *do* include the '*' characters. Signed-off-by: Michael Haggerty --- Documentation/technical/api-remote.txt | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/technical/api-remote.txt b/Documentation/technical/api-remote.txt index 4be8776..5d245aa 100644 --- a/Documentation/technical/api-remote.txt +++ b/Documentation/technical/api-remote.txt @@ -58,16 +58,16 @@ default remote, given the current branch and configuration. struct refspec -- -A struct refspec holds the parsed interpretation of a refspec. If it -will force updates (starts with a '+'), force is true. If it is a -pattern (sides end with '*') pattern is true. src and dest are the two -sides (if a pattern, only the part outside of the wildcards); if there -is only one side, it is src, and dst is NULL; if sides exist but are -empty (i.e., the refspec either starts or ends with ':'), the -corresponding side is "". - -This parsing can be done to an array of strings to give an array of -struct refpsecs with parse_ref_spec(). +A struct refspec holds the parsed interpretation of a refspec. If it +will force updates (starts with a '+'), force is true. If it is a +pattern (sides end with '*') pattern is true. src and dest are the +two sides (including '*' characters if present); if there is only one +side, it is src, and dst is NULL; if sides exist but are empty (i.e., +the refspec either starts or ends with ':'), the corresponding side is +"". + +An array of strings can be parsed into an array of struct refspecs +using parse_fetch_refspec() or parse_push_refspec(). remote_find_tracking(), given a remote and a struct refspec with either src or dst filled out, will fill out the other such that the -- 1.8.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 07/15] ref_remove_duplicates(): simplify function
* Use a dedicated variable, ref, for referring to the current item
rather than using the ref_map pointer for this purpose.
* Use a (struct ref **) as iteration variable to avoid having to keep
track of prev and next in addition to the pointer to the current
item.
Signed-off-by: Michael Haggerty
---
remote.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/remote.c b/remote.c
index a44e897..5ade07f 100644
--- a/remote.c
+++ b/remote.c
@@ -748,29 +748,33 @@ int for_each_remote(each_remote_fn fn, void *priv)
void ref_remove_duplicates(struct ref *ref_map)
{
struct string_list refs = STRING_LIST_INIT_NODUP;
- struct string_list_item *item = NULL;
- struct ref *prev = NULL, *next = NULL;
+ struct ref **p;
- for (; ref_map; prev = ref_map, ref_map = next) {
- next = ref_map->next;
- if (!ref_map->peer_ref)
+ for (p = &ref_map; *p; ) {
+ struct ref *ref = *p;
+ struct string_list_item *item;
+
+ if (!ref->peer_ref) {
+ p = &ref->next;
continue;
+ }
- item = string_list_insert(&refs, ref_map->peer_ref->name);
+ item = string_list_insert(&refs, ref->peer_ref->name);
if (item->util) {
/* Entry already existed */
if (strcmp(((struct ref *)item->util)->name,
- ref_map->name))
+ ref->name))
die("%s tracks both %s and %s",
- ref_map->peer_ref->name,
+ ref->peer_ref->name,
((struct ref *)item->util)->name,
- ref_map->name);
- prev->next = ref_map->next;
- free(ref_map->peer_ref);
- free(ref_map);
- ref_map = prev; /* skip this; we freed it */
+ ref->name);
+ /* item is a duplicate; remove and free it */
+ *p = ref->next;
+ free(ref->peer_ref);
+ free(ref);
} else {
- item->util = ref_map;
+ item->util = ref;
+ p = &ref->next;
}
}
string_list_clear(&refs, 0);
--
1.8.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
Guten Tag ,
Guten Tag , Ich bin Mr. Gabriel Brock Leiter Rechnungswesen Revision der Credit Suisse Bank North West London, hier in England. Ich schreibe Ihnen aus meinem Büro , die von einer großen immensen Nutzen für beide von uns sein wird. In meiner Abteilung , ein Filialleiter (Greater London region) , entdeckte ich eine verlassene Summe von £ 15.000.000,00 ( fünfzehn Millionen Pfund) in Rechnung unseres verstorbenen Kunden. Ich sah Ihr Kontakt während meiner privaten Suche im Info-Center , und ich habe eine tiefe glauben, dass Sie ganz ehrlich zu sein , engagiert und in der Lage mir zu helfen, in diesem Geschäft zu wagen. Es ist auf das, dass ich mich an Sie als Empfänger einer späten Kunde der Bank stehen , so dass die Summe von £ 15.000.000,00 ( fünfzehn Millionen Pfund) wird freigegeben und an Sie ausgezahlt werden als Begünstigte des Verstorbenen berechnet. Am Ende der Transaktion müssen Sie 45 % des gesamten Erbe und ich werde 55% zu nehmen. Ich werde Sie mit allen notwendigen Informationen, Dokumente und Beweise , um legal eine Sicherungskopie der Anspruch aus den verschiedenen Büros für die reibungslose Übertragung des Sondervermögens zu einem Ihrer Konten als den wahren Begünstigten zu verschaffen. Wenn dieser Vorschlag erfüllt Sie bitte zurück zu mir mit den folgenden Informationen : Vollständigen Namen , Telefon / Fax, Handynummern, Adresse, Alter / Geschlecht und Beruf. Ich erwarte Ihre dringende Antwort , damit wir weiter vorgehen . Mit freundlichen Grüßen , Herr Gabriel Brock -- 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: Re* [PATCH] remote-hg: unquote C-style paths when exporting
On Wed, Oct 23, 2013 at 5:44 PM, Junio C Hamano wrote:
> Antoine Pelisse writes:
>
>>> def c_style_unescape(string):
>>> if string[0] == string[-1] == '"':
>>> return string.decode('string-escape')[1:-1]
>>> return string
>>>
>>> It's in git-remote-bzr.py.
>>
>> Yeah, that's certainly better,
>>
>> Thanks,
>
> OK, so an amended one will look like this?
The commit message needs to be updated as well.
> -- >8 --
> From: Antoine Pelisse
> Subject: remote-hg: unquote C-style paths when exporting
>
> git-fast-import documentation says that paths can be C-style quoted.
> Unfortunately, the current remote-hg helper doesn't unquote quoted
> path and pass them as-is to Mercurial when the commit is created.
>
> This result in the following situation:
s/result/&s/
> - clone a mercurial repository with git
> - Add a file with space: `mkdir dir/foo\ bar`
- Add a file with space in a directory: `>dir/foo\ bar`
> - Commit that new file, and push the change to mercurial
> - The mercurial repository as now a new directory named '"dir', which
> contains a file named 'foo bar"'
I'm so ashamed I'd rather not report this one: s/as/has/
> Use python ast.literal_eval to unquote the string if it starts with ".
Use python str.decode('string-escape') to unquote the string if it
starts and ends with ".
> It has been tested with quotes, spaces, and utf-8 encoded file-names.
>
> Signed-off-by: Antoine Pelisse
> Signed-off-by: Junio C Hamano
> ---
> contrib/remote-helpers/git-remote-hg | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/contrib/remote-helpers/git-remote-hg
> b/contrib/remote-helpers/git-remote-hg
> index 0194c67..85abbed 100755
> --- a/contrib/remote-helpers/git-remote-hg
> +++ b/contrib/remote-helpers/git-remote-hg
> @@ -678,6 +678,11 @@ def get_merge_files(repo, p1, p2, files):
> f = { 'ctx' : repo[p1][e] }
> files[e] = f
>
> +def c_style_unescape(string):
> +if string[0] == string[-1] == '"':
> +return string.decode('string-escape')[1:-1]
> +return string
> +
> def parse_commit(parser):
> global marks, blob_marks, parsed_refs
> global mode
> @@ -720,6 +725,7 @@ def parse_commit(parser):
> f = { 'deleted' : True }
> else:
> die('Unknown file command: %s' % line)
> +path = c_style_unescape(path).decode('utf-8')
> files[path] = f
>
> # only export the commits if we are on an internal proxy repo
That is consistent with git-remote-bzr,
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
[PATCH 12/15] query_refspecs(): move some constants out of the loop
Signed-off-by: Michael Haggerty
---
remote.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/remote.c b/remote.c
index 5ade07f..ff5b62a 100644
--- a/remote.c
+++ b/remote.c
@@ -829,6 +829,8 @@ static int query_refspecs(struct refspec *refs, int
ref_count, struct refspec *q
{
int i;
int find_src = !query->src;
+ const char *needle = find_src ? query->dst : query->src;
+ char **result = find_src ? &query->src : &query->dst;
if (find_src && !query->dst)
return error("query_refspecs: need either src or dst");
@@ -837,8 +839,6 @@ static int query_refspecs(struct refspec *refs, int
ref_count, struct refspec *q
struct refspec *refspec = &refs[i];
const char *key = find_src ? refspec->dst : refspec->src;
const char *value = find_src ? refspec->src : refspec->dst;
- const char *needle = find_src ? query->dst : query->src;
- char **result = find_src ? &query->src : &query->dst;
if (!refspec->dst)
continue;
--
1.8.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 14/15] builtin/remote.c:update(): use struct argv_array
Use struct argv_array for calling the "git fetch" subprocesses.
Signed-off-by: Michael Haggerty
---
builtin/remote.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index ecedd96..bffe2f9 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -6,6 +6,7 @@
#include "strbuf.h"
#include "run-command.h"
#include "refs.h"
+#include "argv-array.h"
static const char * const builtin_remote_usage[] = {
N_("git remote [-v | --verbose]"),
@@ -1376,36 +1377,36 @@ static int update(int argc, const char **argv)
N_("prune remotes after fetching")),
OPT_END()
};
- const char **fetch_argv;
- int fetch_argc = 0;
+ struct argv_array fetch_argv = ARGV_ARRAY_INIT;
int default_defined = 0;
-
- fetch_argv = xmalloc(sizeof(char *) * (argc+5));
+ int retval;
argc = parse_options(argc, argv, NULL, options,
builtin_remote_update_usage,
PARSE_OPT_KEEP_ARGV0);
- fetch_argv[fetch_argc++] = "fetch";
+ argv_array_push(&fetch_argv, "fetch");
if (prune)
- fetch_argv[fetch_argc++] = "--prune";
+ argv_array_push(&fetch_argv, "--prune");
if (verbose)
- fetch_argv[fetch_argc++] = "-v";
- fetch_argv[fetch_argc++] = "--multiple";
+ argv_array_push(&fetch_argv, "-v");
+ argv_array_push(&fetch_argv, "--multiple");
if (argc < 2)
- fetch_argv[fetch_argc++] = "default";
+ argv_array_push(&fetch_argv, "default");
for (i = 1; i < argc; i++)
- fetch_argv[fetch_argc++] = argv[i];
+ argv_array_push(&fetch_argv, argv[i]);
- if (strcmp(fetch_argv[fetch_argc-1], "default") == 0) {
+ if (strcmp(fetch_argv.argv[fetch_argv.argc-1], "default") == 0) {
git_config(get_remote_default, &default_defined);
- if (!default_defined)
- fetch_argv[fetch_argc-1] = "--all";
+ if (!default_defined) {
+ argv_array_pop(&fetch_argv);
+ argv_array_push(&fetch_argv, "--all");
+ }
}
- fetch_argv[fetch_argc] = NULL;
-
- return run_command_v_opt(fetch_argv, RUN_GIT_CMD);
+ retval = run_command_v_opt(fetch_argv.argv, RUN_GIT_CMD);
+ argv_array_clear(&fetch_argv);
+ return retval;
}
static int remove_all_fetch_refspecs(const char *remote, const char *key)
--
1.8.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 02/15] t5510: prepare test refs more straightforwardly
"git fetch" was being used with contrived refspecs to create tags and remote-tracking branches in test repositories in preparation for the actual tests. This is obscure and also makes one wonder whether this is indeed just preparation or whether some side-effect of "git fetch" is being tested. So use the more straightforward commands "git tag" / "git update-ref" when preparing branches in test repositories. Signed-off-by: Michael Haggerty --- t/t5510-fetch.sh | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index c5e5dfc..08d8dbb 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -88,7 +88,7 @@ test_expect_success 'fetch --prune on its own works as expected' ' cd "$D" && git clone . prune && cd prune && - git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && + git update-ref refs/remotes/origin/extrabranch master && git fetch --prune origin && test_must_fail git rev-parse origin/extrabranch @@ -98,7 +98,7 @@ test_expect_success 'fetch --prune with a branch name keeps branches' ' cd "$D" && git clone . prune-branch && cd prune-branch && - git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && + git update-ref refs/remotes/origin/extrabranch master && git fetch --prune origin master && git rev-parse origin/extrabranch @@ -117,7 +117,7 @@ test_expect_success 'fetch --prune --tags does not delete the remote-tracking br cd "$D" && git clone . prune-tags && cd prune-tags && - git fetch origin refs/heads/master:refs/tags/sometag && + git tag sometag master && git fetch --prune --tags origin && git rev-parse origin/master && @@ -128,7 +128,7 @@ test_expect_success 'fetch --prune --tags with branch does not delete other remo cd "$D" && git clone . prune-tags-branch && cd prune-tags-branch && - git fetch origin refs/heads/master:refs/remotes/origin/extrabranch && + git update-ref refs/remotes/origin/extrabranch master && git fetch --prune --tags origin master && git rev-parse origin/extrabranch -- 1.8.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 06/15] ref_remove_duplicates(): avoid redundant bisection
The old code called string_list_lookup(), and if that failed called
string_list_insert(), thus doing the bisection search through the
string list twice in the latter code path.
Instead, just call string_list_insert() right away. If an entry for
that peer reference name already existed, then its util pointer is
always non-NULL.
Of course this doesn't change the fact that the repeated
string_list_insert() calls make the function scale like O(N^2) if the
input reference list is not already approximately sorted.
Signed-off-by: Michael Haggerty
---
The O(N^2) algorithm mentioned above essentially boils down to
for each reference:
insert reference into string_list (sorted)
Because the insertion requires later array entries to be shifted to
the right, the algorithm scales like O(N^2) if the entries are not
already approximately in order.
I can think of three easy ways to fix this:
* Use a hash map instead of a sorted string_list to record which
entries have already been seen.
* If the order of the result doesn't matter and it doesn't matter
*which* of the duplicates is retained, then insert all of the names
into the string_list unsorted, then sort them all in one go, then
iterate through looking for duplicates and stringing the survivors
together in a new linked list.
* If the order is important, or it is important that the *first* of
duplicates be retained, then iterate through the linked list twice.
The first time, just add the entries to the string_list. Then sort
the string_list using a stable sort (e.g., git_qsort()). Then
iterate through the linked list a second time as in the current
code.
See also my comments to commit 10.
However, I believe that the O(N^2) worst case is unlikely to bite in
most cases. This function is called from
builtin/fetch.c:get_ref_map(), and the input list is the concatenation
of several sub-lists. Most sublists come from resolving refspecs, and
the last comes from find_non_local_tags(). I believe that each of the
sublists is sorted. So the O(N^2) worst-case could only occur if more
than one of the sublists is large and at least two of the large
sublists are not in the correct order relative to each other.
I tried to concoct such a test case by creating a repository with
about 20k branches and 20k tags, instrumenting
ref_remove_duplicates(), and then fetching with something like
git fetch --prune origin refs/heads/*:refs/remotes/origin/* \
refs/tags/*:refs/tags/*
Indeed, the running time of ref_remove_duplicates() became significant
when the order of the refspecs was reversed, but it was still less
than a second out of a much longer command invocation.
So I didn't bother fixing the O(N^2) algorithm. If somebody reports
real-world slowness here, we can always come back to it.
remote.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/remote.c b/remote.c
index e9fedfa..a44e897 100644
--- a/remote.c
+++ b/remote.c
@@ -750,13 +750,15 @@ void ref_remove_duplicates(struct ref *ref_map)
struct string_list refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item = NULL;
struct ref *prev = NULL, *next = NULL;
+
for (; ref_map; prev = ref_map, ref_map = next) {
next = ref_map->next;
if (!ref_map->peer_ref)
continue;
- item = string_list_lookup(&refs, ref_map->peer_ref->name);
- if (item) {
+ item = string_list_insert(&refs, ref_map->peer_ref->name);
+ if (item->util) {
+ /* Entry already existed */
if (strcmp(((struct ref *)item->util)->name,
ref_map->name))
die("%s tracks both %s and %s",
@@ -767,11 +769,9 @@ void ref_remove_duplicates(struct ref *ref_map)
free(ref_map->peer_ref);
free(ref_map);
ref_map = prev; /* skip this; we freed it */
- continue;
+ } else {
+ item->util = ref_map;
}
-
- item = string_list_insert(&refs, ref_map->peer_ref->name);
- item->util = ref_map;
}
string_list_clear(&refs, 0);
}
--
1.8.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 08/15] ref_remove_duplicates(): improve documentation comment
Signed-off-by: Michael Haggerty --- remote.h | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/remote.h b/remote.h index 131130a..40293c0 100644 --- a/remote.h +++ b/remote.h @@ -149,7 +149,14 @@ int resolve_remote_symref(struct ref *ref, struct ref *list); int ref_newer(const unsigned char *new_sha1, const unsigned char *old_sha1); /* - * Removes and frees any duplicate refs in the map. + * Remove and free all but the first of any entries in the input list + * that map the same remote reference to the same local reference. If + * there are two entries that map different remote references to the + * same local reference, die. + * + * Note that the first entry is never removed; therefore, the pointer + * passed in as argument still points to the head of the list after + * the function returns. */ void ref_remove_duplicates(struct ref *ref_map); -- 1.8.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 01/15] t5510: use the correct tag name in test
Fix an apparent copy-paste error: A few lines earlier, a tag "refs/tags/sometag" is created. Check for the (non-)existence of that tag, not "somebranch", which is otherwise never mentioned in the script. Signed-off-by: Michael Haggerty --- t/t5510-fetch.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 1f0f8e6..c5e5dfc 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -121,7 +121,7 @@ test_expect_success 'fetch --prune --tags does not delete the remote-tracking br git fetch --prune --tags origin && git rev-parse origin/master && - test_must_fail git rev-parse somebranch + test_must_fail git rev-parse sometag ' test_expect_success 'fetch --prune --tags with branch does not delete other remote-tracking branches' ' -- 1.8.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
Re: [PATCH v2 2/2] Update documentation for http.continue option
Jonathan Nieder writes: > Wouldn't a natural fix be to *always* use "Expect: 100-continue" when > and only when the probe_rpc() revealed a server supporting > GSS-Negotiate authentication? A stupid question. Is GSS-Negotiate the only special case? -- 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] remote-hg: unquote C-style paths when exporting
Antoine Pelisse writes:
>> def c_style_unescape(string):
>> if string[0] == string[-1] == '"':
>> return string.decode('string-escape')[1:-1]
>> return string
>>
>> It's in git-remote-bzr.py.
>
> Yeah, that's certainly better,
>
> Thanks,
OK, so an amended one will look like this?
-- >8 --
From: Antoine Pelisse
Subject: remote-hg: unquote C-style paths when exporting
git-fast-import documentation says that paths can be C-style quoted.
Unfortunately, the current remote-hg helper doesn't unquote quoted
path and pass them as-is to Mercurial when the commit is created.
This result in the following situation:
- clone a mercurial repository with git
- Add a file with space: `mkdir dir/foo\ bar`
- Commit that new file, and push the change to mercurial
- The mercurial repository as now a new directory named '"dir', which
contains a file named 'foo bar"'
Use python ast.literal_eval to unquote the string if it starts with ".
It has been tested with quotes, spaces, and utf-8 encoded file-names.
Signed-off-by: Antoine Pelisse
Signed-off-by: Junio C Hamano
---
contrib/remote-helpers/git-remote-hg | 6 ++
1 file changed, 6 insertions(+)
diff --git a/contrib/remote-helpers/git-remote-hg
b/contrib/remote-helpers/git-remote-hg
index 0194c67..85abbed 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -678,6 +678,11 @@ def get_merge_files(repo, p1, p2, files):
f = { 'ctx' : repo[p1][e] }
files[e] = f
+def c_style_unescape(string):
+if string[0] == string[-1] == '"':
+return string.decode('string-escape')[1:-1]
+return string
+
def parse_commit(parser):
global marks, blob_marks, parsed_refs
global mode
@@ -720,6 +725,7 @@ def parse_commit(parser):
f = { 'deleted' : True }
else:
die('Unknown file command: %s' % line)
+path = c_style_unescape(path).decode('utf-8')
files[path] = f
# only export the commits if we are on an internal proxy repo
--
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/2] entry.c: convert checkout_entry to use strbuf
On Wed, Oct 23, 2013 at 3:04 PM, Duy Nguyen wrote:
> On Wed, Oct 23, 2013 at 7:58 PM, Antoine Pelisse wrote:
>>> diff --git a/entry.c b/entry.c
>>> index acc892f..d955af5 100644
>>> --- a/entry.c
>>> +++ b/entry.c
>>> @@ -237,16 +237,18 @@ static int check_path(const char *path, int len,
>>> struct stat *st, int skiplen)
>>> int checkout_entry(struct cache_entry *ce,
>>>const struct checkout *state, char *topath)
>>> {
>>> - static char path[PATH_MAX + 1];
>>> + static struct strbuf path_buf = STRBUF_INIT;
>>> + char *path;
>>> struct stat st;
>>> - int len = state->base_dir_len;
>>> + int len;
>>>
>>> if (topath)
>>> return write_entry(ce, topath, state, 1);
>>>
>>> - memcpy(path, state->base_dir, len);
>>> - strcpy(path + len, ce->name);
>>> - len += ce_namelen(ce);
>>> + strbuf_reset(&path_buf);
>>
>> I think this is not required
>
> If you mean strbuf_reset, I think it is. path_buf is still static (I
> don't want to remove that because it'll add a lot more strbuf_release)
> so we can't be sure what it contains from the second checkout_entry()
> call.
Of course, I forgot about the static,
Thanks :-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] entry.c: convert checkout_entry to use strbuf
On Wed, Oct 23, 2013 at 7:58 PM, Antoine Pelisse wrote:
>> diff --git a/entry.c b/entry.c
>> index acc892f..d955af5 100644
>> --- a/entry.c
>> +++ b/entry.c
>> @@ -237,16 +237,18 @@ static int check_path(const char *path, int len,
>> struct stat *st, int skiplen)
>> int checkout_entry(struct cache_entry *ce,
>>const struct checkout *state, char *topath)
>> {
>> - static char path[PATH_MAX + 1];
>> + static struct strbuf path_buf = STRBUF_INIT;
>> + char *path;
>> struct stat st;
>> - int len = state->base_dir_len;
>> + int len;
>>
>> if (topath)
>> return write_entry(ce, topath, state, 1);
>>
>> - memcpy(path, state->base_dir, len);
>> - strcpy(path + len, ce->name);
>> - len += ce_namelen(ce);
>> + strbuf_reset(&path_buf);
>
> I think this is not required
If you mean strbuf_reset, I think it is. path_buf is still static (I
don't want to remove that because it'll add a lot more strbuf_release)
so we can't be sure what it contains from the second checkout_entry()
call.
--
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] Clear fd after closing to avoid double-close error
On Tue, Oct 22, 2013 at 8:36 PM, Jens Lindström wrote: > From: Jens Lindstrom > > In send_pack(), clear the fd passed to pack_objects() by setting > it to -1, since pack_objects() closes the fd (via a call to > run_command()). Likewise, in get_pack(), clear the fd passed to > run_command(). > > Not doing so risks having git_transport_push(), caller of > send_pack(), closing the fd again, possibly incorrectly closing > some other open file; or similarly with fetch_refs_from_pack(), > indirect caller of get_pack(). > > Signed-off-by: Jens Lindström > --- > fetch-pack.c | 4 > send-pack.c | 4 > 2 files changed, 8 insertions(+) > > diff --git a/fetch-pack.c b/fetch-pack.c > index f5d99c1..29b711a 100644 > --- a/fetch-pack.c > +++ b/fetch-pack.c > @@ -776,6 +776,10 @@ static int get_pack(struct fetch_pack_args *args, > close(cmd.out); > } > > + if (!use_sideband) > + /* Closed by start_command() */ > + xd[0] = -1; > + Further up demux.out still holds this handle. But we don't actually start_async when use_sideband is false so demux.out can't be used anywhere else. So it's good. Acked-by: me > ret = finish_command(&cmd); > if (!ret || (args->check_self_contained_and_connected && ret == 1)) > args->self_contained_and_connected = > diff --git a/send-pack.c b/send-pack.c > index 7d172ef..edbfd07 100644 > --- a/send-pack.c > +++ b/send-pack.c > @@ -300,8 +300,12 @@ int send_pack(struct send_pack_args *args, > shutdown(fd[0], SHUT_WR); > if (use_sideband) > finish_async(&demux); > + fd[1] = -1; > return -1; > } > + if (!args->stateless_rpc) > + /* Closed by pack_objects() via start_command() */ > + fd[1] = -1; > } > if (args->stateless_rpc && cmds_sent) > packet_flush(out); > -- > 1.8.1.2 > -- 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 1/2] entry.c: convert checkout_entry to use strbuf
On Wed, Oct 23, 2013 at 2:55 PM, Nguyễn Thái Ngọc Duy wrote:
> The old code does not do boundary check so any paths longer than
> PATH_MAX can cause buffer overflow. Replace it with strbuf to handle
> paths of arbitrary length.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy
> ---
> To get this topic going again. These two patches kill PATH_MAX in
> entry.c and builtin/checkout-index.c
Thanks !
> diff --git a/entry.c b/entry.c
> index acc892f..d955af5 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -237,16 +237,18 @@ static int check_path(const char *path, int len, struct
> stat *st, int skiplen)
> int checkout_entry(struct cache_entry *ce,
>const struct checkout *state, char *topath)
> {
> - static char path[PATH_MAX + 1];
> + static struct strbuf path_buf = STRBUF_INIT;
> + char *path;
> struct stat st;
> - int len = state->base_dir_len;
> + int len;
>
> if (topath)
> return write_entry(ce, topath, state, 1);
>
> - memcpy(path, state->base_dir, len);
> - strcpy(path + len, ce->name);
> - len += ce_namelen(ce);
> + strbuf_reset(&path_buf);
I think this is not required
--
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: [msysGit] [PATCH] Prevent buffer overflows when path is too big
On Tue, Oct 22, 2013 at 2:02 AM, Johannes Sixt wrote: >> (or maybe at higher level to skip checking out those paths). > > More like this, yeah. The good thing is we do not stop checking out if one entry fails. But due to the lack of worktree entries, one may accidentally remove files in new commits. So setting CE_VALID on failed-to-checkout entries might help. I'm not sure. But I won't pursue because Windows is not really my itch. -- 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 2/2] entry.c: convert write_entry to use strbuf
The strcpy call in open_output_fd() implies that the output buffer
must be at least 25 chars long. And it's true. The only caller that
can trigger that code is checkout-index, which has the buffer of
PATH_MAX chars (and any systems that have PATH_MAX shorter than 25
chars are just insane).
But in order to say that, one has to walk through a dozen of
functions. Just convert it to strbuf to avoid the constraint and
confusion.
Although my original motivation was simpler than that. I just wanted
to change "char *path" to "const char *path" in checkout_entry() to
make sure no funny business regarding "path" in that function.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/checkout-index.c | 19 ---
cache.h | 2 +-
entry.c | 29 -
3 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 69e167b..6d88c0c 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -14,7 +14,12 @@
static int line_termination = '\n';
static int checkout_stage; /* default to checkout stage0 */
static int to_tempfile;
-static char topath[4][PATH_MAX + 1];
+static struct strbuf topath[4] = {
+ STRBUF_INIT,
+ STRBUF_INIT,
+ STRBUF_INIT,
+ STRBUF_INIT
+};
static struct checkout state;
@@ -26,19 +31,19 @@ static void write_tempfile_record(const char *name, int
prefix_length)
for (i = 1; i < 4; i++) {
if (i > 1)
putchar(' ');
- if (topath[i][0])
- fputs(topath[i], stdout);
+ if (topath[i].len)
+ fputs(topath[i].buf, stdout);
else
putchar('.');
}
} else
- fputs(topath[checkout_stage], stdout);
+ fputs(topath[checkout_stage].buf, stdout);
putchar('\t');
write_name_quoted(name + prefix_length, stdout, line_termination);
for (i = 0; i < 4; i++) {
- topath[i][0] = 0;
+ strbuf_reset(&topath[i]);
}
}
@@ -65,7 +70,7 @@ static int checkout_file(const char *name, int prefix_length)
continue;
did_checkout = 1;
if (checkout_entry(ce, &state,
- to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)
+ to_tempfile ? &topath[ce_stage(ce)] : NULL) < 0)
errs++;
}
@@ -109,7 +114,7 @@ static void checkout_all(const char *prefix, int
prefix_length)
write_tempfile_record(last_ce->name,
prefix_length);
}
if (checkout_entry(ce, &state,
- to_tempfile ? topath[ce_stage(ce)] : NULL) < 0)
+ to_tempfile ? &topath[ce_stage(ce)] : NULL) < 0)
errs++;
last_ce = ce;
}
diff --git a/cache.h b/cache.h
index 51d6602..276182f 100644
--- a/cache.h
+++ b/cache.h
@@ -962,7 +962,7 @@ struct checkout {
refresh_cache:1;
};
-extern int checkout_entry(struct cache_entry *ce, const struct checkout
*state, char *topath);
+extern int checkout_entry(struct cache_entry *ce, const struct checkout
*state, struct strbuf *topath);
struct cache_def {
char path[PATH_MAX + 1];
diff --git a/entry.c b/entry.c
index d955af5..a76942d 100644
--- a/entry.c
+++ b/entry.c
@@ -92,15 +92,15 @@ static void *read_blob_entry(const struct cache_entry *ce,
unsigned long *size)
return NULL;
}
-static int open_output_fd(char *path, const struct cache_entry *ce, int
to_tempfile)
+static int open_output_fd(struct strbuf *path, const struct cache_entry *ce,
int to_tempfile)
{
int symlink = (ce->ce_mode & S_IFMT) != S_IFREG;
if (to_tempfile) {
- strcpy(path, symlink
- ? ".merge_link_XX" : ".merge_file_XX");
- return mkstemp(path);
+ strbuf_reset(path);
+ strbuf_addstr(path, symlink ? ".merge_link_XX" :
".merge_file_XX");
+ return mkstemp(path->buf);
} else {
- return create_file(path, !symlink ? ce->ce_mode : 0666);
+ return create_file(path->buf, !symlink ? ce->ce_mode : 0666);
}
}
@@ -115,7 +115,7 @@ static int fstat_output(int fd, const struct checkout
*state, struct stat *st)
return 0;
}
-static int streaming_write_entry(const struct cache_entry *ce, char *path,
+static int streaming_write_entry(const struct cache_entry *ce, struct strbuf
*path,
struct stream_filter *filter,
const struct checkout *state, int to_tempfile,
int *fstat_done, struct stat *statbuf)
@@ -132,12 +132,12 @@ static in
[PATCH 1/2] entry.c: convert checkout_entry to use strbuf
The old code does not do boundary check so any paths longer than
PATH_MAX can cause buffer overflow. Replace it with strbuf to handle
paths of arbitrary length.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
To get this topic going again. These two patches kill PATH_MAX in
entry.c and builtin/checkout-index.c
entry.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/entry.c b/entry.c
index acc892f..d955af5 100644
--- a/entry.c
+++ b/entry.c
@@ -237,16 +237,18 @@ static int check_path(const char *path, int len, struct
stat *st, int skiplen)
int checkout_entry(struct cache_entry *ce,
const struct checkout *state, char *topath)
{
- static char path[PATH_MAX + 1];
+ static struct strbuf path_buf = STRBUF_INIT;
+ char *path;
struct stat st;
- int len = state->base_dir_len;
+ int len;
if (topath)
return write_entry(ce, topath, state, 1);
- memcpy(path, state->base_dir, len);
- strcpy(path + len, ce->name);
- len += ce_namelen(ce);
+ strbuf_reset(&path_buf);
+ strbuf_addf(&path_buf, "%.*s%s", state->base_dir_len, state->base_dir,
ce->name);
+ path = path_buf.buf;
+ len = path_buf.len;
if (!check_path(path, len, &st, state->base_dir_len)) {
unsigned changed = ce_match_stat(ce, &st,
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
--
1.8.2.83.gc99314b
--
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: Finding the repository
On Wed, Oct 23, 2013 at 2:52 PM, Perry Hutchison wrote: > At least in version 1.7.0.4, it seems git does not like being run > from outside the repository, even if the file(s) being operated > on are inside the repository, unless it is given a pointer to the > repository via the --git-dir= option or the GIT_DIR enironment > variable. > > For example, suppose /foo/bar is a local repository and baz.c is a > file in the outermost directory that I want to remove. This works: > > $ cd /foo/bar > $ git rm baz.c > > but this, which intuitively should mean exactly the same thing, > fails: > > $ cd /foo > $ git rm bar/baz.c > fatal: Not a git repository (or any of the parent directories): .git I share your pain. In my case I hate to go inside a directory just to grep something. In my opinion git should be flexible and work at least in unambiguous cases. But it's not easy to determine ambiguity here, especially when the repo finding code does not know anything about "bar/barz.c" (is it a pathname or an argument to an option?). There are more cases to consider, like what if you do "git rm bar/baz.c and rab/zab.c" where bar and rab are two different repositories.. And the setup code is not exactly easy to add these stuff in.. -- 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] remote-hg: unquote C-style paths when exporting
On Wed, Oct 23, 2013 at 2:45 AM, Felipe Contreras
wrote:
> On Tue, Oct 22, 2013 at 3:49 PM, Antoine Pelisse wrote:
>
>> It is true that I have expected "valid output" from git-fast-export.
>> And I don't have in mind any easy solution to detect that the output
>> is broken, yet still accepted as a valid string by python. We could
>> obviously write a unquote_c_style() equivalent in python if needed.
>
> Something like this?
>
> def c_style_unescape(string):
> if string[0] == string[-1] == '"':
> return string.decode('string-escape')[1:-1]
> return string
>
> It's in git-remote-bzr.py.
Yeah, that's certainly better,
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
Finding the repository
At least in version 1.7.0.4, it seems git does not like being run from outside the repository, even if the file(s) being operated on are inside the repository, unless it is given a pointer to the repository via the --git-dir= option or the GIT_DIR enironment variable. For example, suppose /foo/bar is a local repository and baz.c is a file in the outermost directory that I want to remove. This works: $ cd /foo/bar $ git rm baz.c but this, which intuitively should mean exactly the same thing, fails: $ cd /foo $ git rm bar/baz.c fatal: Not a git repository (or any of the parent directories): .git I've written a wrapper script that solves this problem, but it is more an illustration or proof of concept than a real "solution" -- the command line parsing may well be imperfect, and it would be semantically incorrect in such cases as committing multiple (individually specified) files: it would do a separate commit of each pathname rather than a single commit of all pathnames. Has anyone considered enhancing the automatic repository search in git itself to look in the directory where the specified file(s) is/are located, as a last resort before failing? (Yes, this does present the potential for operating on multiple repositories with a single invocation of git; would that be a bad thing?) #!/usr/local/bin/bash # smarter git: if the current directory has no .git subdirectory # (i.e. is not in a repository), try running git in the directory # where each file is located instead of in the current directory. [ "$1" == "--version" -o "$1" == "--help" -o "$1" == "--exec-path" \ -o "x$GIT_DIR" != "x" -o -d .git ] && exec git "$@" # Set defaults flags="" dirSet=0 # Collect flag params while [[ "$1" == -?* ]] ; do case "$1" in --git-dir=* ) dirSet=1 ;; * ) esac flags="$flags $1" shift done [ "$dirSet" == "1" ] && exec git $flags "$@" # next word must be the command gitCmd="$1" shift # remaining words must be pathnames for f in "$@" do ( cd $(dirname "$f") && git $flags $gitCmd $(basename "$f") ) done -- 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
git grep: search whole tree by default?
Hi, it would be nice if grep searched not only in current directory and subdirectories, but in whole tree. I know I can use ":/" as a pathspec, but since most git commands work tree, I got used to this and forgot that grep is different. It's easy to make a mistake and believe that your code does not contain searched string XXX - because you have searched from a subdirectory, not from the top level of your working tree. OTOH, if grep searches whole tree, you'll notice you get results from outside of CWD and if you don't want that, you will be able to limit the search to '.' I think there were discussion about how there are several git commands which do not search in whole tree by default and that it's going to be changed. I think "add" is one of such commands. Is 'grep' left unchanged? Last discussion I found is from 2011 March (http://thread.gmane.org/gmane.comp.version-control.git/168063/focus=168188) and it says it's not going to be changed :( -- Piotr Krukowiecki -- 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] Clear fd after closing to avoid double-close error
On Tue, Oct 22, 2013 at 8:42 PM, Junio C Hamano wrote: > Duy Nguyen writes: >> Life would have been simpler if fd[1] was _always_ closed by >> send_pack(), like in c20181e (start_command(), if .in/.out > 0, closes >> file descriptors, not the callers - 2008-02-21). > > Yeah, that was also my first reaction when I saw the above three > lines after reading the discussion that led to the diagnosis. If send_pack() always closes fd[1], then I believe "git send-pack --stateless-rpc --helper-status" would die in print_helper_status(), called after send_pack(), since fd[1] would be 1, to which print_helper_status() will try to write. (I don't know what either --stateless-rpc or --helper-status mean, other than what's obvious from the code, or if the combination of them makes any sense.) I don't really have any more time to spend on this issue, so if a more thorough fix is required, I'm afraid someone else will have to work on it. / Jens -- 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
