Re: [PATCH v2 00/21] Support multiple worktrees
Duy Nguyen pclo...@gmail.com writes: I am not happy with the choice of main/HEAD that would squat on a good name for remote-tracking branch (i.e. s/origin/main/), though. $GIT_DIR/COMMON_HEAD perhaps? It's not just about HEAD. Anything worktree-specific of the main checkout can be accessed this way, e.g. main/index, main/FETCH_HEAD and it's not exactly common because it's worktree info. Maybe 1ST_ as the prefix (e.g. 1ST_HEAD, 1ST_index...) ? Do we even need to expose them as ref-like things as a part of the external API/UI in the first place? For end-user scripts that want to operate in a real or borrowing worktree, there should be no reason to touch this other repository directly. Things like if one of the wortrees tries to check out a branch that is already checked out elsewhere, error out policy may need to consult the other worktrees via $GIT_COMMON_DIR mechanism but at that level we have all the control without contaminating end-user facing ref namespace in a way main/FETCH_HEAD... do. You said This makes it possible for a linked checkout to detach HEAD of the main one. but I think that is misguided---it just makes it easier to confuse users, if done automatically and without any policy knob. It instead should error out, while saying which worktree has the branch in question checked out. After all, checking out a branch that is checked out in another worktree (not the common one) needs the same caution, so main/HEAD is not the only special one. And if your updated git checkout 'frotz' gives a clear report of which worktree has the branch 'frotz' checked out, the user can go there to detach the HEAD in that worktree to detach with git -C $the_other_one checkout HEAD^0 if he chooses to. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: questions / suggestions about history simplification
Adam Spiers g...@adamspiers.org writes: I doubt it. 75% of the work for such a person to understand the behaviour from such an example is to understand what kind of history the example is building. Agreed. And that's precisely why I wanted a real repository manifesting the given example: being able to view it in gitk makes that a lot easier to understand, for obvious reasons. ... Well I didn't roll my own; I just copied the example from the man page. So it only tells you that I was spending a fair amount of effort trying to understand the man page ;-) Oh, that part I would agree, but then ... Not if the man page says if you wish to experiment with these options yourself, you can easily recreate the repository in this example by running the script contrib/foo bundled in the source distribution. ... The goal of sharing the series of commands is not to educate users through reading them, but simply to save them the time they would have to spend manually recreating the example scenario given in the man page. ... this part could be even easier if distro ships a sample repository, not a recipe to create such a sample repository, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-cherry.txt: cross reference git log --cherry
Samuel Bronson naes...@gmail.com writes: I learned of git cherry some days ago, but only learned of --cherry and related options to git log today[1] (more-or-less by chance). If the git-cherry(1) manpage had mentioned --cherry, I would have learned of these options sooner. This proposed log message is of an unusual style. It is curious that somebody learn cherry first before log. SEE ALSO -linkgit:git-patch-id[1] +linkgit:git-patch-id[1], +the `--cherry` option to linkgit:git-log[1] The description text talks about changeset (or diff), compares the changeset rather than the commit id, diffs are compared, etc., which is a hint that a lone reference to patch-id would be a page to read about that comparison, which is a good motivation that may entice the readers to follow that reference. I am not sure what value the readers of this man page would see to this addition, though. Unlike the reference to patch-id, it is not so obvious what gives them motivation to follow this new reference to log --cherry. A new sentence in DESCRIPTION section to mention it (e.g. 'log --cherry' gives similar information) may be needed at the same time. -- 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: questions / suggestions about history simplification
Adam Spiers g...@adamspiers.org writes: OTOH, including a sample repository embedded within the git repository is either impossible or very ugly (e.g. having a non-default value of GIT_DIR for the embedded repository). But I doubt you were suggesting that ;-) No. By the way, t/t1200-tutorial.sh was meant to follow what the tutorial manual tells the reader to try. We may want to update and/or enhance it to cover more materials. 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 v2 00/21] Support multiple worktrees
Duy Nguyen pclo...@gmail.com writes: On Sun, Dec 22, 2013 at 1:38 PM, Junio C Hamano gits...@pobox.com wrote: Do we even need to expose them as ref-like things as a part of the external API/UI in the first place? For end-user scripts that want to operate in a real or borrowing worktree, there should be no reason to touch this other repository directly. Things like if one of the wortrees tries to check out a branch that is already checked out elsewhere, error out policy may need to consult the other worktrees via $GIT_COMMON_DIR mechanism but at that level we have all the control without contaminating end-user facing ref namespace in a way main/FETCH_HEAD... do. No, external API/UI is just extra bonus. We need to (or should) do so in order to handle $GIT_COMMON_DIR/HEAD exactly like how we do normal refs. And that is what I consider a confusion-trigger, not a nice bonus. I do not think it is particularly a good idea to contaminate end-user namespace for this kind of peek another repository special operation. In order to handle your multiple worktrees manipulating the same branch case sanely, you need to be aware of not just the real repository your worktree is borrowing from, but also _all_ the other worktrees that borrow from that same real repository, so a single main virtual namespace will not cut it. You will be dealing with an unbounded set of HEADs, one for each such worktree. Can't we do this by adding a with this real repository layer, e.g. resolve HEAD wrt that repository, somewhat similar to how we peek into submodule namespaces? -- 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] add: don't complain when adding empty project root
Jonathan Nieder jrnie...@gmail.com writes: How about something like the following, for squashing in? With or without the tweaks below, Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks, both. Regarding git add --refresh (no other arguments), it would say Nothing specified, nothing added., and that is unrelated to the breakage reported and fixed in this thread, I think. It is the same message git add (no other arguments) gives, which I think is a mistake. git add --refresh is like git add -u in that the affected paths are determined by the index, and running these commands while your index is still empty can just be a silent no-op. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 07/10] builtin/replace: teach listing using short, medium or full formats
Christian Couder christian.cou...@gmail.com writes: On Thu, Dec 19, 2013 at 7:58 PM, Junio C Hamano gits...@pobox.com wrote: Christian Couder christian.cou...@gmail.com writes: I think this last one might be useful for people replacing objects with objects that have another type. ... which IIUC is strongly discouraged---didn't you have to tighten it recently? And that makes it useful primarily for debugging unusual situations. Ok, so would you prefer the following: - NAME_ONLY_REPLACE_FMT and --format=name_only instead of SHORT_REPLACE_FMT and --format=short - NAME_AND_VALUE_REPLACE_FMT and --format=name_and_value instead of MEDIUM_REPLACE_FMT and --format=medium - DEBUG_REPLACE_FMT and --format=debug instead of FULL _REPLACE_FMT and --format=full The end-user facing names are probably fine with short, medium, full, as long as what they show are clearly explained in the end-user documentation (patch 10/10 covers this). I have a hunch that we may later regret full when somebody wants to add even fuller information, though. It might be better spelled long instead; I'd rather see REPLACE_FMT_ as a prefix, not suffix. Do we use common suffix for enum values elsewhere? -- 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] add: don't complain when adding empty project root
Junio C Hamano gits...@pobox.com writes:
Regarding git add --refresh (no other arguments), it would say
Nothing specified, nothing added., and that is unrelated to the
breakage reported and fixed in this thread, I think. It is the same
message git add (no other arguments) gives, which I think is a
mistake. git add --refresh is like git add -u in that the
affected paths are determined by the index, and running these
commands while your index is still empty can just be a silent no-op.
Something like this...
builtin/add.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/add.c b/builtin/add.c
index d7e3e44..84e8a3e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -483,8 +483,10 @@ int cmd_add(int argc, const char **argv, const char
*prefix)
(implicit_dot ? ADD_CACHE_IMPLICIT_DOT : 0);
if (require_pathspec argc == 0) {
- fprintf(stderr, _(Nothing specified, nothing added.\n));
- fprintf(stderr, _(Maybe you wanted to say 'git add .'?\n));
+ if (!refresh_only) {
+ fprintf(stderr, _(Nothing specified, nothing
added.\n));
+ fprintf(stderr, _(Maybe you wanted to say 'git add
.'?\n));
+ }
return 0;
}
--
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/12] Convert starts_with() to skip_prefix() for option parsing
Duy Nguyen pclo...@gmail.com writes: On Sat, Dec 21, 2013 at 4:31 AM, Junio C Hamano gits...@pobox.com wrote: Jeff King p...@peff.net writes: /* here we care if we saw the prefix, as above */ if (parse_prefix(foo, prefix, the_rest)) ... /* * and here we do not care, and just want to optionally strip the * prefix, and take the full value otherwise; we just have to ignore * the return value in this case. */ parse_prefix(foo, prefix, foo); Sounds fine. I recall earlier somebody wanting to have a good name for this thing, and I think foo_gently is *not* it (the name is about adding a variant that does not die outright to foo that checks and dies if condition is not right). starts_with(foo, prefix); strip_prefix(foo, prefix, foo); perhaps? I still need consensus on the name here guys, parse_prefix. remove_prefix or strip_prefix? If no other opinions i'll go with strip_prefix (Jeff's comment before parse_prefix() also uses strip) Yup, that comment is where I took strip from. When you name your thing as X, using too generic a word X, and then need to explain what X does using a bit more specific word Y, you are often better off naming it after Y. -- 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] sha1_object_info_extended: provide delta base sha1s
Jeff King p...@peff.net writes:
@@ -1824,6 +1856,22 @@ static int packed_object_info(struct packed_git *p,
off_t obj_offset,
}
}
+ if (oi-delta_base_sha1) {
+ if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
+ const unsigned char *base;
+
+ base = get_delta_base_sha1(p, w_curs, curpos,
+type, obj_offset);
+ if (!base) {
+ type = OBJ_BAD;
+ goto out;
+ }
+
+ hashcpy(oi-delta_base_sha1, base);
+ } else
+ hashclr(oi-delta_base_sha1);
+ }
+
Makes sense.
--
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: [WIP/PATCH 0/5] git checkout --recurse-submodules
Jonathan Nieder jrnie...@gmail.com writes: Hi, This patch series comes from https://github.com/jlehmann/git-submod-enhancements branch recursive_submodule_checkout. It needed some tiny tweaks to apply to current master and build without warnings, but nothing major, and I haven't sanity checked it much beyond that and letting the kind folks that use Debian experimental play with it. I'm sending it out now to get review and ideas for what needs to happen next to get this series in shape to be included in git.git. Thoughts of all kinds welcome. Thanks, Jonathan Jens Lehmann (5): submodule: prepare for recursive checkout of submodules submodule: teach unpack_trees() to remove submodule contents submodule: teach unpack_trees() to repopulate submodules submodule: teach unpack_trees() to update submodules Teach checkout to recursively checkout submodules Documentation/git-checkout.txt | 8 ++ builtin/checkout.c | 14 +++ entry.c| 19 +++- submodule.c| 217 - submodule.h| 11 +++ t/t2013-checkout-submodule.sh | 215 +++- unpack-trees.c | 94 ++ unpack-trees.h | 1 + wrapper.c | 3 + 9 files changed, 556 insertions(+), 26 deletions(-) Looks reasonably clean from a cursory read. 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
What's cooking in git.git (Dec 2013, #05; Thu, 26)
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 -- [New Topics] * bc/log-decoration (2013-12-20) 1 commit - log: properly handle decorations with chained tags git log --decorate did not handle a tag pointed by another tag nicely. Will merge to 'next'. * jh/rlimit-nofile-fallback (2013-12-18) 1 commit - get_max_fd_limit(): fall back to OPEN_MAX upon getrlimit/sysconf failure When we figure out how many file descriptors to allocate for keeping packfiles open, a system with non-working getrlimit() could cause us to die(), but because we make this call only to get a rough estimate of how many is available and we do not even attempt to use up all file descriptors available ourselves, it is nicer to fall back to a reasonable low value rather than dying. Will merge to 'next'. * rt/bfg-ad-in-filter-branch-doc (2013-12-18) 1 commit - docs: add filter-branch notes on The BFG Will merge to 'next'. * sb/diff-orderfile-config (2013-12-18) 3 commits - diff: add diff.orderfile configuration variable - diff: let git diff -O read orderfile from any file and fail properly - t4056: add new tests for git diff -O Allow git diff -Ofile to be configured with a new configuration variable. Will merge to 'next'. * jc/graph-post-root-gap (2013-12-20) 1 commit - graph: give an extra gap after showing root commit This was primarily a RFH ($gmane/239580). * nd/daemon-informative-errors-typofix (2013-12-20) 1 commit - daemon: be strict at parsing parameters --[no-]informative-errors Will merge to 'next'. * tm/fetch-prune (2013-12-20) 2 commits - fetch --prune: run prune before fetching - fetch --prune: always print header url Fetching 'frotz' branch with git fetch, while having 'frotz/nitfol' remote-tracking branch from an earlier fetch, would error out, primarily because the command has not been told to remove anything on our side. In such a case, git fetch --prune can be used to remove 'frotz/nitfol' to make room to fetch and store 'frotz' remote-tracking branch. Will merge to 'next'. * jk/oi-delta-base (2013-12-26) 2 commits - cat-file: provide %(deltabase) batch format - sha1_object_info_extended: provide delta base sha1s Teach cat-file --batch to show delta-base object name for a packed object that is represented as a delta. Will merge to 'next'. * jk/sha1write-void (2013-12-26) 1 commit - do not pretend sha1write returns errors Code clean-up. Will merge to 'next'. * jl/submodule-recursive-checkout (2013-12-26) 5 commits - Teach checkout to recursively checkout submodules - submodule: teach unpack_trees() to update submodules - submodule: teach unpack_trees() to repopulate submodules - submodule: teach unpack_trees() to remove submodule contents - submodule: prepare for recursive checkout of submodules * mh/safe-create-leading-directories (2013-12-26) 5 commits - rename_ref(): fix a mkdir()/rmdir() race - safe_create_leading_directories(): fix a mkdir/rmdir race - safe_create_leading_directories(): add slash pointer - safe_create_leading_directories(): reduce scope of local variable - safe_create_leading_directories(): modernize format of if chaining Code clean-up and protection against concurrent write access to the ref namespace. Will merge to 'next'. * nd/add-empty-fix (2013-12-26) 1 commit - add: don't complain when adding empty project root git add -A (no other arguments) in a totally empty working tree used to emit an error. Will merge to 'next'. * nd/commit-tree-constness (2013-12-26) 1 commit - commit.c: make tree a const pointer in commit_tree*() Code clean-up. Will merge to 'next'. -- [Stalled] * mo/subtree-split-updates (2013-12-10) 3 commits - subtree: add --edit option - subtree: allow --squash and --message with push - subtree: support split --rejoin --squash Comments? * hv/submodule-ignore-fix (2013-12-06) 4 commits - disable complete ignorance of submodules for index - HEAD diff - always show committed submodules in summary after commit - teach add -f option for ignored submodules - fix 'git add' to skip submodules configured as ignored Teach git add to be consistent with git status when changes to submodules are set to be ignored, to avoid surprises after checking with git status to see there isn't any change to be further added and then see that git add . adds changes to them. I think a reroll is coming, so this may need to be replaced, but I needed some practice with heavy conflict resolution. It conflicts with two changes to git add that have been scheduled for Git 2.0 quite badly, and I think I got the resolution right
Re: [PATCH] Remove the line length limit for graft files
Johannes Schindelin johannes.schinde...@gmx.de writes:
Support for grafts predates Git's strbuf, and hence it is understandable
that there was a hard-coded line length limit of 1023 characters (which
was chosen a bit awkwardly, given that it is *exactly* one byte short of
aligning with the 41 bytes occupied by a commit name and the following
space or new-line character).
While regular commit histories hardly win comprehensibility in general
if they merge more than twenty-two branches in one go, it is not Git's
business to limit grafts in such a way.
In this particular developer's case, the use case that requires
substantially longer graft lines to be supported is the visualization of
the commits' order implied by their changes: commits are considered to
have an implicit relationship iff exchanging them in an interactive
rebase would result in merge conflicts.
Thusly implied branches tend to be very shallow in general, and the
resulting thicket of implied branches is usually very wide; It is
actually quite common that *most* of the commits in a topic branch have
not even one implied parent, so that a final merge commit has about as
many implied parents as there are commits in said branch.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
builtin/blame.c | 8
commit.c| 10 +-
2 files changed, 9 insertions(+), 9 deletions(-)
Makes sense. It is in line with the spirit of ef98c5cafb3e
(commit-tree: lift completely arbitrary limit of 16 parents,
2008-06-27), too ;-)
Thanks, will queue.
diff --git a/builtin/blame.c b/builtin/blame.c
index 1407ae7..9047b6e 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1804,17 +1804,17 @@ static int prepare_lines(struct scoreboard *sb)
static int read_ancestry(const char *graft_file)
{
FILE *fp = fopen(graft_file, r);
- char buf[1024];
+ struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
- while (fgets(buf, sizeof(buf), fp)) {
+ while (!strbuf_getwholeline(buf, fp, '\n')) {
/* The format is just Commit Parent1 Parent2 ...\n */
- int len = strlen(buf);
- struct commit_graft *graft = read_graft_line(buf, len);
+ struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
if (graft)
register_commit_graft(graft, 0);
}
fclose(fp);
+ strbuf_release(buf);
return 0;
}
diff --git a/commit.c b/commit.c
index de16a3c..57ebea2 100644
--- a/commit.c
+++ b/commit.c
@@ -196,19 +196,19 @@ bad_graft_data:
static int read_graft_file(const char *graft_file)
{
FILE *fp = fopen(graft_file, r);
- char buf[1024];
+ struct strbuf buf = STRBUF_INIT;
if (!fp)
return -1;
- while (fgets(buf, sizeof(buf), fp)) {
+ while (!strbuf_getwholeline(buf, fp, '\n')) {
/* The format is just Commit Parent1 Parent2 ...\n */
- int len = strlen(buf);
- struct commit_graft *graft = read_graft_line(buf, len);
+ struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
if (!graft)
continue;
if (register_commit_graft(graft, 1))
- error(duplicate graft data: %s, buf);
+ error(duplicate graft data: %s, buf.buf);
}
fclose(fp);
+ strbuf_release(buf);
return 0;
}
--
1.8.4.msysgit.0.1109.g3c58b16
--
--
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: What's cooking in git.git (Dec 2013, #05; Thu, 26)
Eric Sunshine sunsh...@sunshineco.com writes: On Thu, Dec 26, 2013 at 4:08 PM, Junio C Hamano gits...@pobox.com wrote: 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 -- [New Topics] Would $gmane/239575 [1] be of interest for New Topics? [1]: http://article.gmane.org/gmane.comp.version-control.git/239575/ Actually I was planning to scoop it up directly to master but forgot to do so. Running git diff maint pu -- name-hash.c shows that we have added a comment that mentions index_name_exists---that needs to be adjusted, too, by the way. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git:// protocol over SSL/TLS
Konstantin Khomoutov flatw...@users.sourceforge.net writes: On Fri, 27 Dec 2013 18:59:00 +0600 Sergey Sharybin sergey@gmail.com wrote: Quick question is, is it possible to use git:// protocol over SSL/TLS/other secure transport? The Git protocol does not implement it itself but you can channel it over a TLS tunnel (via stunnel for instance). Unfortunately, this means a specialized software and setup on both ends so if the question was about a general client using stock Git then the answer is no, it's impossible. Hmph, I somehow had an impression that you wouldn't need anything more complex than a simple helper that uses git-remote-ext on the client side. On the remote end, you'd need to have something that terminates the incoming SSL/TLS and plugs it to your git daemon. Or the recommended way to do secure anonymous checkout is to simply use https:// ? Yes, but it will only be secure if you've managed to verify the server's certificate and do trust its issuer (or a CA higher up the cert's trust chain) -- people tend to confuse encrypted with secure which is not at all the same thing. -- 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] git-svn: workaround for a bug in svn serf backend
Eric Wong normalper...@yhbt.net writes: Jonathan Nieder jrnie...@gmail.com wrote: Roman Kagan wrote: Subversion serf backend in versions 1.8.5 and below has a bug that the function creating the descriptor of a file change -- add_file() -- doesn't make a copy of its third argument when storing it on the returned descriptor. As a result, by the time this field is used (in transactions of file copying or renaming) it may well be released, and the memory reused. One of its possible manifestations is the svn assertion triggering on an invalid path, with a message svn_fspath__skip_ancestor: Assertion `svn_fspath__is_canonical(child_fspath)' failed. [...] Makes sense. Perhaps also worth mentioning that this is fixed by r1553376, but no need to reroll just for that. Thanks all, I noted this in an addendum to the commit: Subversion serf backend in versions 1.8.5 and below has a bug(*) that the ... * [ew: fixed in Subversion r1553376 as noted by Jonathan Nieder] Cc: Benjamin Pabst benjamin.pabs...@gmail.com Cc: Eric Wong normalper...@yhbt.net Cc: Jonathan Nieder jrnie...@gmail.com No need for these lines --- the mail header already keeps track of who is being cc-ed. I don't mind seeing it in history. At least I've gotten accustomed to it from the Linux kernel and tracking patch flow between dev - stable trees. Signed-off-by: Roman Kagan rka...@mail.ru Reviewed-by: Jonathan Nieder jrnie...@gmail.com Signed-off-by: Eric Wong normalper...@yhbt.net The following changes since commit 7794a680e63a2a11b73cb1194653662f2769a792: Sync with 1.8.5.2 (2013-12-17 14:12:17 -0800) are available in the git repository at: git://git.bogomips.org/git-svn.git master for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace: git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +) Roman Kagan (1): git-svn: workaround for a bug in svn serf backend perl/Git/SVN/Editor.pm | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Thanks. I almost missed this pull-request, though. Will pull. -- 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] Remove the line length limit for graft files
Jonathan Nieder jrnie...@gmail.com writes: Johannes Schindelin wrote: On Fri, 27 Dec 2013, Jonathan Nieder wrote: Is this easy to reproduce so some interested but lazy person could write a test? Yep. Make 25 orphan commits, add a graft line to make the first a merge of the rest. Thanks. Here's a pair of tests doing that. Signed-off-by: Jonathan Nieder jrnie...@gmail.com --- t/annotate-tests.sh | 21 + t/t6101-rev-parse-parents.sh | 16 +++- 2 files changed, 36 insertions(+), 1 deletion(-) Makes sense. Thanks, both. Small lint-picking like this change to perfect the system, as opposed to earth-shattering new shinies, tend to often get neglected but are very much appreciated. diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh index c9d105d..304c7b7 100644 --- a/t/annotate-tests.sh +++ b/t/annotate-tests.sh @@ -116,6 +116,27 @@ test_expect_success 'blame evil merge' ' check_count A 2 B 1 B1 2 B2 1 A U Thor 1 ' +test_expect_success 'blame huge graft' ' + test_when_finished git checkout branch2 + test_when_finished rm -f .git/info/grafts + graft= + for i in 0 1 2 + do + for j in 0 1 2 3 4 5 6 7 8 9 + do + git checkout --orphan $i$j + printf %s\n $i $j file + test_tick + GIT_AUTHOR_NAME=$i$j GIT_AUTHOR_EMAIL=$i$j...@test.git \ + git commit -a -m $i$j + commit=$(git rev-parse --verify HEAD) + graft=$graft$commit + done + done + printf %s $graft .git/info/grafts + check_count -h 00 01 1 10 1 +' + test_expect_success 'setup incomplete line' ' echo incomplete | tr -d \\012 file GIT_AUTHOR_NAME=C GIT_AUTHOR_EMAIL=c...@test.git \ diff --git a/t/t6101-rev-parse-parents.sh b/t/t6101-rev-parse-parents.sh index 7ea14ce..10b1452 100755 --- a/t/t6101-rev-parse-parents.sh +++ b/t/t6101-rev-parse-parents.sh @@ -20,7 +20,17 @@ test_expect_success 'setup' ' test_commit start2 git checkout master git merge -m next start2 - test_commit final + test_commit final + + test_seq 40 | + while read i + do + git checkout --orphan b$i + test_tick + git commit --allow-empty -m $i + commit=$(git rev-parse --verify HEAD) + printf $commit .git/info/grafts + done ' test_expect_success 'start is valid' ' @@ -79,6 +89,10 @@ test_expect_success 'final^1^! = final^1 ^final^1^1 ^final^1^2' ' test_cmp expect actual ' +test_expect_success 'large graft octopus' ' + test_cmp_rev_output b31 git rev-parse --verify b1^30 +' + test_expect_success 'repack for next test' ' git repack -a -d ' -- 1.8.5.1 -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 9/9] trailer: add tests for git interpret-trailers
Christian Couder chrisc...@tuxfamily.org writes: +# Do not remove trailing spaces below! +cat complex_message_trailers 'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF Just a hint. I think it is far safer and robust over time to do something like this: sed -e 's/ Z$/ /' -\EOF Fixes: Z Acked-by: Z EOF instead of a comment, which can warn human developers but does not do anything to prevent their editors' auto-fix features from kicking in. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] t0000 cleanups
Jonathan Nieder jrnie...@gmail.com writes: Jeff King wrote: When I want to debug a failing test, I often end up doing: cd t ./t4107-tab -v -i cd tratab The test names are long, so tab-completing on the trash directory is very helpful. Lately I've noticed that there are a bunch of crufty trash directories in my t/ directory, which makes my tab-completion more annoying. Ah, and if I'd read this then I wouldn't have had to be confused at all. Would it work to replace the commit message with something like this? The third paragraph of 1/3 sufficiently covers it, no? We could add It makes it less convenient to use tab completion 'cd t/traTAB' to go to the trash directory of the failed test to inspect the situation after ... left in the t/ directory., though. Once upon a time, the test-lib library would create trash directories in the current working directory, unless we were explicitly told to put it elsewhere via --root. As a result, t created the sub-test trash directories inside its own trash directory. However, we noticed that this did not cover all cases, since we would need to respect $TEST_OUTPUT_DIRECTORY even if --root is not given (or is relative). Commit 38b074d fixed this to consistently use the full path. As a result, trash directories used by t's sub-tests are now created in git's original test output directory rather than in our trash directory. Furthermore, since some of the sub-tests simulate failures, the trash directories do not get cleaned up, and the cruft is left in the t/ directory. We could fix this by passing a new --root=$TRASH_DIRECTORY option to the sub-test. However, we do not want the sub-tests to write anything at all to git's directory (e.g., they should not be writing to t/test-results, either, although this is already handled by separate code). So the best solution is to simply reset $TEST_OUTPUT_DIRECTORY entirely in the sub-test, which covers this case, as well as any future ones. -- 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] merge-base: fix duplicates and not best ancestors in output
Василий Макаров einmal...@gmail.com writes:
Hi there!
First of all: I'm new to mailing-lists, sorry if I'm doing it wrong.
I've found a bug in git merge-base, causing it to show not best common
ancestors and duplicates under some circumstances (example is given in
attached test case).
Attached???
Problem cause is algorithm used in get_octopus_merge_bases(), it
iteratively concatenates merge bases, and don't care if there are
duplicates or decsendants/ancestors in result.
What I suggest as a solution is to simply reduce bases list after
get_octopus_merge_bases().
I do not offhand remember if it was deliberate that we do not dedup
the result from the underlying get_octopus_merge_bases() (the most
likely reason for not deduping is because the caller is expected to
do that if it wants to).
Whether it is an improvement to force deduping here or it is an
regression to do so, I think we should split that helper function
handle_octopus(). It does two totally unrelated things (one is only
to list independent heads without showing merge bases, the other is
to show one or more merge bases across all the heads given).
Perhaps if we split the independent codepath introduced by
a1e0ad78 (merge-base --independent to print reduced parent list in a
merge, 2010-08-17) into its own helper function, like this, it would
make it clear what is going on.
Thanks.
builtin/merge-base.c | 31 +--
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index e88eb93..a00e8f5 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -44,19 +44,36 @@ static struct commit *get_commit_reference(const char *arg)
return r;
}
-static int handle_octopus(int count, const char **args, int reduce, int
show_all)
+static int handle_independent(int count, const char **args)
{
struct commit_list *revs = NULL;
struct commit_list *result;
int i;
- if (reduce)
- show_all = 1;
+ for (i = count - 1; i = 0; i--)
+ commit_list_insert(get_commit_reference(args[i]), revs);
+
+ result = reduce_heads(revs);
+ if (!result)
+ return 1;
+
+ while (result) {
+ printf(%s\n, sha1_to_hex(result-item-object.sha1));
+ result = result-next;
+ }
+ return 0;
+}
+
+static int handle_octopus(int count, const char **args, int show_all)
+{
+ struct commit_list *revs = NULL;
+ struct commit_list *result;
+ int i;
for (i = count - 1; i = 0; i--)
commit_list_insert(get_commit_reference(args[i]), revs);
- result = reduce ? reduce_heads(revs) : get_octopus_merge_bases(revs);
+ result = get_octopus_merge_bases(revs);
if (!result)
return 1;
@@ -114,8 +131,10 @@ int cmd_merge_base(int argc, const char **argv, const char
*prefix)
if (reduce (show_all || octopus))
die(--independent cannot be used with other options);
- if (octopus || reduce)
- return handle_octopus(argc, argv, reduce, show_all);
+ if (octopus)
+ return handle_octopus(argc, argv, show_all);
+ else if (reduce)
+ return handle_independent(argc, argv);
rev = xmalloc(argc * sizeof(*rev));
while (argc-- 0)
--
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: aborted 'git fetch' leaves workspace unusable
stephen_le...@stephe-leake.org writes: That left the workspace unusable: - .git/FETCH_HEAD is empty that causes 'git rev-parse FETCH_HEAD' to fail with a confusing error message. This is not limited to your Cygwin environment. I can see that we leave an empty file there after a failed fetch with $ git fetch ssh://no.such.place/ But I would not call it leaving the workspace unusable. If you ask git rev-parse What is in FETCH_HEAD?, you would get that is not even a revision, which is what you would get. Similar operations that try to use FETCH_HEAD as if there is a valid revision, e.g. git merge FETCH_HEAD, would also not work, which is very much expected. I wouldn't think that needs something drastic as this workspace is unusable, let's start from a new clone. If it really bothers you, you can always safely do $ rm -f .git/FETCH_HEAD but of course, after that, nothing that tries to use FETCH_HEAD as if there is a valid revision, e.g. git show FETCH_HEAD, would not work until you fetch from somewhere, so there isn't that much to be gained by doing so. - 'git fetch' just hangs after outputting: remote: Counting objects: 15, done. remote: Compressing objects: 100% (8/8), done. remote: Total 9 (delta 5), reused 0 (delta 0) This looks more serious, but I suspect it is totally unrelated to your previous fetch failing and leaving FETCH_HEAD there. Is this 'git fetch' hangs reproduce in a clean clone _without_ first encountering the failure (due to the forgotton ssh-add)? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-svn: workaround for a bug in svn serf backend
Roman Kagan rka...@mail.ru writes: 2013/12/28 Junio C Hamano gits...@pobox.com: Eric Wong normalper...@yhbt.net writes: git://git.bogomips.org/git-svn.git master for you to fetch changes up to 2394e94e831991348688831a384b088a424c7ace: git-svn: workaround for a bug in svn serf backend (2013-12-27 20:22:19 +) Roman Kagan (1): git-svn: workaround for a bug in svn serf backend perl/Git/SVN/Editor.pm | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) Thanks. I almost missed this pull-request, though. Will pull. Thanks! That's redundant; the project should thank you for contributing, not the other way around. I'd like to note that it's IMO worth including in the 'maint' branch as it's a crasher. Especially so since the real fix has been merged in the subversion upstream and nominated for 1.8 branch, so the workaround may soon lose its relevance. I do not quite get this part, though. If they refused to fix it for real, it would make it likely that this workaround will stay relevant for a long time, in which case it would be worth cherry-picking to an older maintenance track. But if this workaround is expected to lose its relevance shortly, I see it as one less reason to cherry-pick it to an older maintenance track. Confused... -- 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] merge-base: fix duplicates and not best ancestors in output
Junio C Hamano gits...@pobox.com writes: I do not offhand remember if it was deliberate that we do not dedup the result from the underlying get_octopus_merge_bases() (the most likely reason for not deduping is because the caller is expected to do that if it wants to). Whether it is an improvement to force deduping here or it is an regression to do so, I think we should split that helper function handle_octopus(). It does two totally unrelated things (one is only to list independent heads without showing merge bases, the other is to show one or more merge bases across all the heads given). Perhaps if we split the independent codepath introduced by a1e0ad78 (merge-base --independent to print reduced parent list in a merge, 2010-08-17) into its own helper function, like this, it would make it clear what is going on. And assuming that deduping is the right thing to do here, here is a follow-up on top of the spliting patch. -- 8 -- Subject: [PATCH] merge-base --octopus: reduce the result from get_octopus_merge_bases() Scripts that use merge-base --octopus could do the reducing themselves, but most of them are expected to want to get the reduced results without having to do any work themselves. Tests are taken from a message by Василий Макаров einmal...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- We might want to vet the existing callers of the underlying get_octopus_merge_bases() and find out if _all_ of them are doing anything extra (like deduping) because the machinery can return duplicate results. And if that is the case, then we may want to move the dedupling down the callchain instead of having it here. builtin/merge-base.c | 2 +- t/t6010-merge-base.sh | 39 +++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/builtin/merge-base.c b/builtin/merge-base.c index daa96c7..87f4dbc 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -73,7 +73,7 @@ static int handle_octopus(int count, const char **args, int show_all) for (i = count - 1; i = 0; i--) commit_list_insert(get_commit_reference(args[i]), revs); - result = get_octopus_merge_bases(revs); + result = reduce_heads(get_octopus_merge_bases(revs)); if (!result) return 1; diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh index f80bba8..abb5728 100755 --- a/t/t6010-merge-base.sh +++ b/t/t6010-merge-base.sh @@ -230,4 +230,43 @@ test_expect_success 'criss-cross merge-base for octopus-step' ' test_cmp expected.sorted actual.sorted ' +test_expect_success 'merge-base --octopus --all for complex tree' ' + # Best common ancestor for JE, JAA and JDD is JC + # JE + #/ | + # / | + # / | + # JAA/| + # |\ / | + # | \ | JDD | + # | \ |/ | | + # | JC JD | + # || /| | + # ||/ | | + # JA| | | + # |\ /| | | + # X JB | X X + # \ \ | / / + #\__\|/___/ + #J + test_commit J + test_commit JB + git reset --hard J + test_commit JC + git reset --hard J + test_commit JTEMP1 + test_merge JA JB + test_merge JAA JC + git reset --hard J + test_commit JTEMP2 + test_merge JD JB + test_merge JDD JC + git reset --hard J + test_commit JTEMP3 + test_merge JE JC + git rev-parse JC expected + git merge-base --all --octopus JAA JDD JE actual + test_cmp expected actual +' + test_done -- 1.8.5.2-311-g6427a96 -- 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] for-each-ref: remove unused variable
Ramkumar Ramachandra artag...@gmail.com writes:
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Interesting. As far as I can tell, no code ever used this symbol
since the command was introduced at 9f613ddd (Add git-for-each-ref:
helper for language bindings, 2006-09-15).
builtin/for-each-ref.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6551e7b..51798b4 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -92,7 +92,7 @@ static struct {
*/
static const char **used_atom;
static cmp_type *used_atom_type;
-static int used_atom_cnt, sort_atom_limit, need_tagged, need_symref;
+static int used_atom_cnt, need_tagged, need_symref;
static int need_color_reset_at_eol;
/*
@@ -1105,7 +1105,6 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
if (!sort)
sort = default_sort();
- sort_atom_limit = used_atom_cnt;
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
--
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 v4 00/10] teach replace objects to sha1_object_info_extended()
Christian Couder chrisc...@tuxfamily.org writes:
The only changes compared to version 3 are the following:
I'll queue this instead on top, as the series is already in 'next'.
Thanks.
-- 8 --
From: Christian Couder chrisc...@tuxfamily.org
Date: Sat, 28 Dec 2013 12:00:05 +0100
Subject: [PATCH] replace info: rename 'full' to 'long' and clarify in-code
symbols
Enum names SHORT/MEDIUM/FULL were too broad to be descriptive. And
they clashed with built-in symbols on platforms like Windows.
Clarify by giving them REPLACE_FORMAT_ prefix.
Rename 'full' format in git replace --format=name to 'long', to
match others (i.e. 'short' and 'medium').
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Documentation/git-replace.txt | 4 ++--
builtin/replace.c | 24 ++--
t/t6050-replace.sh| 4 ++--
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 7a07828..0a02f70 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -72,7 +72,7 @@ OPTIONS
--format=format::
When listing, use the specified format, which can be one of
- 'short', 'medium' and 'full'. When omitted, the format
+ 'short', 'medium' and 'long'. When omitted, the format
defaults to 'short'.
FORMATS
@@ -84,7 +84,7 @@ The following format are available:
replaced sha1
* 'medium':
replaced sha1 - replacement sha1
-* 'full'
+* 'long':
replaced sha1 (replaced type) - replacement sha1 (replacement
type)
CREATING REPLACEMENT OBJECTS
diff --git a/builtin/replace.c b/builtin/replace.c
index 1672870..2336325 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -20,11 +20,15 @@ static const char * const git_replace_usage[] = {
NULL
};
-enum repl_fmt { SHORT, MEDIUM, FULL };
+enum replace_format {
+ REPLACE_FORMAT_SHORT,
+ REPLACE_FORMAT_MEDIUM,
+ REPLACE_FORMAT_LONG
+};
struct show_data {
const char *pattern;
- enum repl_fmt fmt;
+ enum replace_format format;
};
static int show_reference(const char *refname, const unsigned char *sha1,
@@ -33,11 +37,11 @@ static int show_reference(const char *refname, const
unsigned char *sha1,
struct show_data *data = cb_data;
if (!fnmatch(data-pattern, refname, 0)) {
- if (data-fmt == SHORT)
+ if (data-format == REPLACE_FORMAT_SHORT)
printf(%s\n, refname);
- else if (data-fmt == MEDIUM)
+ else if (data-format == REPLACE_FORMAT_MEDIUM)
printf(%s - %s\n, refname, sha1_to_hex(sha1));
- else { /* data-fmt == FULL */
+ else { /* data-format == REPLACE_FORMAT_LONG */
unsigned char object[20];
enum object_type obj_type, repl_type;
@@ -64,14 +68,14 @@ static int list_replace_refs(const char *pattern, const
char *format)
data.pattern = pattern;
if (format == NULL || *format == '\0' || !strcmp(format, short))
- data.fmt = SHORT;
+ data.format = REPLACE_FORMAT_SHORT;
else if (!strcmp(format, medium))
- data.fmt = MEDIUM;
- else if (!strcmp(format, full))
- data.fmt = FULL;
+ data.format = REPLACE_FORMAT_MEDIUM;
+ else if (!strcmp(format, long))
+ data.format = REPLACE_FORMAT_LONG;
else
die(invalid replace format '%s'\n
- valid formats are 'short', 'medium' and 'full'\n,
+ valid formats are 'short', 'medium' and 'long'\n,
format);
for_each_replace_ref(show_reference, (void *) data);
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d0c62f7..719a116 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -306,7 +306,7 @@ test_expect_success 'test --format medium' '
test_cmp expected actual
'
-test_expect_success 'test --format full' '
+test_expect_success 'test --format long' '
{
echo $H1 (commit) - $BLOB (blob)
echo $BLOB (blob) - $REPLACED (blob)
@@ -314,7 +314,7 @@ test_expect_success 'test --format full' '
echo $PARA3 (commit) - $S (commit)
echo $MYTAG (tag) - $HASH1 (commit)
} | sort expected
- git replace --format=full | sort actual
+ git replace --format=long | sort actual
test_cmp expected actual
'
--
1.8.5.2-311-g6427a96
--
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 9/9] trailer: add tests for git interpret-trailers
Josh Triplett j...@joshtriplett.org writes: On Mon, Dec 30, 2013 at 09:19:55AM -0800, Junio C Hamano wrote: Christian Couder chrisc...@tuxfamily.org writes: +# Do not remove trailing spaces below! +cat complex_message_trailers 'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF Just a hint. I think it is far safer and robust over time to do something like this: sed -e 's/ Z$/ /' -\EOF Fixes: Z Acked-by: Z EOF instead of a comment, which can warn human developers but does not do anything to prevent their editors' auto-fix features from kicking in. This, but for simplicity, since every line needs the trailing space, why not just use 's/$/ /' and drop the ' Z' on every line? /bikeshed - Josh Triplett A few reasons: - The everybody will have a single SP at the end may or may not last forever; - With your scheme, if you already had _one_ trailing SPs in the input, it would be hard to spot in the source; - It makes it visually very clear that we expect a SP after these colons. This is especially true if you replace 'Z' with something more readable (e.g. '|'). -- 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 9/9] trailer: add tests for git interpret-trailers
Josh Triplett j...@joshtriplett.org writes: - The everybody will have a single SP at the end may or may not last forever; Trivially fixed if that ever changes, but given the nature of all of these, that seems unlikely. Why? Because we encourage to write tests that are expected to find breakages, some of these test vector lines may have to show a broken line that lacks SP after label + colon. - With your scheme, if you already had _one_ trailing SPs in the input, it would be hard to spot in the source; Git makes them quite difficult to miss. :) That is irrelevant, isn't it? This is about protecting the source in the editor, before you run git show --whitespace=trailing-space, git diff --check, etc. -- 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] drop unnecessary copying in credential_ask_one
Jeff King p...@peff.net writes:
... But the test suite, of course, always uses askpass because it
cannot rely on accessing a terminal (we'd have to do some magic with
lib-terminal, I think).
So it doesn't detect the problem in your patch, but I wonder if it is
worth applying the patch below anyway, as it makes the test suite
slightly more robust.
Sounds like a good first step in the right direction. Thanks.
-- 8 --
Subject: use distinct username/password for http auth tests
The httpd server we set up to test git's http client code
knows about a single account, in which both the username and
password are user@host (the unusual use of the @ here is
to verify that we handle the character correctly when URL
escaped).
This means that we may miss a certain class of errors in
which the username and password are mixed up internally by
git. We can make our tests more robust by having distinct
values for the username and password.
In addition to tweaking the server passwd file and the
client URL, we must teach the askpass harness to accept
multiple values. As a bonus, this makes the setup of some
tests more obvious; when we are expecting git to ask
only about the password, we can seed the username askpass
response with a bogus value.
Signed-off-by: Jeff King p...@peff.net
---
t/lib-httpd.sh| 15 ---
t/lib-httpd/passwd| 2 +-
t/t5540-http-push.sh | 4 ++--
t/t5541-http-push.sh | 6 +++---
t/t5550-http-fetch.sh | 10 +-
t/t5551-http-fetch.sh | 6 +++---
6 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index c470784..bfdff2a 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -129,7 +129,7 @@ prepare_httpd() {
HTTPD_DEST=127.0.0.1:$LIB_HTTPD_PORT
HTTPD_URL=$HTTPD_PROTO://$HTTPD_DEST
HTTPD_URL_USER=$HTTPD_PROTO://user%40host@$HTTPD_DEST
- HTTPD_URL_USER_PASS=$HTTPD_PROTO://user%40host:user%40host@$HTTPD_DEST
+ HTTPD_URL_USER_PASS=$HTTPD_PROTO://user%40host:pass%40host@$HTTPD_DEST
if test -n $LIB_HTTPD_DAV -o -n $LIB_HTTPD_SVN
then
@@ -217,7 +217,15 @@ setup_askpass_helper() {
test_expect_success 'setup askpass helper' '
write_script $TRASH_DIRECTORY/askpass -\EOF
echo $TRASH_DIRECTORY/askpass-query askpass: $*
- cat $TRASH_DIRECTORY/askpass-response
+ case $* in
+ *Username*)
+ what=user
+ ;;
+ *Password*)
+ what=pass
+ ;;
+ esac
+ cat $TRASH_DIRECTORY/askpass-$what
EOF
GIT_ASKPASS=$TRASH_DIRECTORY/askpass
export GIT_ASKPASS
@@ -227,7 +235,8 @@ setup_askpass_helper() {
set_askpass() {
$TRASH_DIRECTORY/askpass-query
- echo $* $TRASH_DIRECTORY/askpass-response
+ echo $1 $TRASH_DIRECTORY/askpass-user
+ echo $2 $TRASH_DIRECTORY/askpass-pass
}
expect_askpass() {
diff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd
index f2fbcad..99a34d6 100644
--- a/t/lib-httpd/passwd
+++ b/t/lib-httpd/passwd
@@ -1 +1 @@
-user@host:nKpa8pZUHx/ic
+user@host:xb4E8pqD81KQs
diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh
index 01d0d95..5b0198c 100755
--- a/t/t5540-http-push.sh
+++ b/t/t5540-http-push.sh
@@ -154,7 +154,7 @@ test_http_push_nonff
$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git \
test_expect_success 'push to password-protected repository (user in URL)' '
test_commit pw-user
- set_askpass user@host
+ set_askpass user@host pass@host
git push $HTTPD_URL_USER/auth/dumb/test_repo.git HEAD
git rev-parse --verify HEAD expect
git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/auth/dumb/test_repo.git \
@@ -168,7 +168,7 @@ test_expect_failure 'user was prompted only once for
password' '
test_expect_failure 'push to password-protected repository (no user in URL)'
'
test_commit pw-nouser
- set_askpass user@host
+ set_askpass user@host pass@host
git push $HTTPD_URL/auth/dumb/test_repo.git HEAD
expect_askpass both user@host
git rev-parse --verify HEAD expect
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 470ac54..bfd241e 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -274,7 +274,7 @@ test_expect_success 'push over smart http with auth' '
cd $ROOT_PATH/test_repo_clone
echo push-auth-test expect
test_commit push-auth-test
- set_askpass user@host
+ set_askpass user@host pass@host
git push $HTTPD_URL/auth/smart/test_repo.git
git --git-dir=$HTTPD_DOCUMENT_ROOT_PATH/test_repo.git \
log -1 --format=%s actual
@@ -286,7 +286,7 @@ test_expect_success 'push to auth-only-for-push repo' '
cd $ROOT_PATH/test_repo_clone
echo push-half-auth expect
test_commit
Re: [PATCH v2 3/4] Speed up is_git_command() by checking early for internal commands
Sebastian Schuberth sschube...@gmail.com writes:
Since 2dce956 is_git_command() is a bit slow as it does file I/O in the
call to list_commands_in_dir(). Avoid the file I/O by adding an early
check for internal commands.
I think it is a good thing to check with the list of built-in's
first, but in order to see if one name we already have at hand is a
git command, I have to wonder if it is the best we can do to collect
all the possible command names with load_command_list() and check
the membership.
There are two callers of is_in_cmdlist(), and the way they use the
function looks both very backwards.
- builtin/help.c has a user supplied string that it wants to see if
it is a git command (either on-disk in exec-path or a builtin),
either to see if an alias in a config is really in effect, or to
see if it is a command (i.e. invoke 'man' with git-string) or a
concept (i.e. invoke 'man' with gitstring). It feels that it
should be a lot less work to just check with the builtin table
and stat(exec-path + string) for either of these purposes (on
Windows you may have to append .exe before checking and may also
have to check with .com so you may have to do more than one
stat(), but still).
Even though help is not a performance critical codepath,
optimizing this further so that we do not load the full set of
commands unnecessarily may be in line with the theme of your
series, I think.
- builtin/merge.c is the same, but it is conceptually even worse.
It has the end-user supplied string and wants to see if it is a
valid strategy. If the user wants to use a custom strategy, a
single stat() to make sure if it exists should suffice, and the
error codepath should load the command list to present the names
of available ones in the error message.
Based on the above observation, I have a feeling that we will be
better off to aim for getting rid of is_in_cmdlist(), reimplement
is_git_command() to check with builtin and then do a stat (or two)
inside the exec-path, and update builtin/merge.c codepath to use
is_git_command() to see if the given name is indeed a strategy.
So in short, I very much like the part that moves the built-in
command list out of the main() function and makes is_git_command()
use it, but I think the primary implementation of is_git_command()
to check if the given string names an on-disk command is still not
very nice.
Thanks.
Signed-off-by: Sebastian Schuberth sschube...@gmail.com
---
Documentation/technical/api-builtin.txt | 4 +-
builtin.h | 2 +
builtin/help.c | 3 +
git.c | 242
+---
4 files changed, 134 insertions(+), 117 deletions(-)
diff --git a/Documentation/technical/api-builtin.txt
b/Documentation/technical/api-builtin.txt
index 150a02a..e3d6e7a 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -14,8 +14,8 @@ Git:
. Add the external declaration for the function to `builtin.h`.
-. Add the command to `commands[]` table in `handle_builtin()`,
- defined in `git.c`. The entry should look like:
+. Add the command to the `commands[]` table defined in `git.c`.
+ The entry should look like:
{ foo, cmd_foo, options },
+
diff --git a/builtin.h b/builtin.h
index d4afbfe..c47c110 100644
--- a/builtin.h
+++ b/builtin.h
@@ -27,6 +27,8 @@ extern int fmt_merge_msg(struct strbuf *in, struct strbuf
*out,
extern int textconv_object(const char *path, unsigned mode, const unsigned
char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);
+extern int is_builtin(const char *s);
+
extern int cmd_add(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix);
diff --git a/builtin/help.c b/builtin/help.c
index b6fc15e..1fdefeb 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -288,6 +288,9 @@ static struct cmdnames main_cmds, other_cmds;
static int is_git_command(const char *s)
{
+ if (is_builtin(s))
+ return 1;
+
load_command_list(git-, main_cmds, other_cmds);
return is_in_cmdlist(main_cmds, s) ||
is_in_cmdlist(other_cmds, s);
diff --git a/git.c b/git.c
index 89ab5d7..bba4378 100644
--- a/git.c
+++ b/git.c
@@ -332,124 +332,136 @@ static int run_builtin(struct cmd_struct *p, int
argc, const char **argv)
return 0;
}
+static struct cmd_struct commands[] = {
+ { add, cmd_add, RUN_SETUP | NEED_WORK_TREE },
+ { annotate, cmd_annotate, RUN_SETUP },
+ { apply, cmd_apply, RUN_SETUP_GENTLY },
+ { archive, cmd_archive },
+ { bisect--helper, cmd_bisect__helper, RUN_SETUP },
+ { blame, cmd_blame, RUN_SETUP },
+ { branch, cmd_branch, RUN_SETUP },
+ { bundle,
Re: [PATCH v2 4/4] Move builtin-related implementations to a new builtin.c file
Sebastian Schuberth sschube...@gmail.com writes:
Signed-off-by: Sebastian Schuberth sschube...@gmail.com
---
Documentation/technical/api-builtin.txt | 2 +-
Makefile| 1 +
builtin.c | 225 ++
builtin.h | 21 +++
git.c | 238
5 files changed, 248 insertions(+), 239 deletions(-)
create mode 100644 builtin.c
I'm sorry but I do not see a point in this.
It is not like builtin.c can be used outside the context of the main
Git program, and many helper functions you moved out of git.c that
used to be static want to be called from other places.
diff --git a/Documentation/technical/api-builtin.txt
b/Documentation/technical/api-builtin.txt
index e3d6e7a..d1d946c 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -14,7 +14,7 @@ Git:
. Add the external declaration for the function to `builtin.h`.
-. Add the command to the `commands[]` table defined in `git.c`.
+. Add the command to the `commands[]` table defined in `builtin.c`.
The entry should look like:
{ foo, cmd_foo, options },
diff --git a/Makefile b/Makefile
index b4af1e2..2d947e8 100644
--- a/Makefile
+++ b/Makefile
@@ -763,6 +763,7 @@ LIB_OBJS += base85.o
LIB_OBJS += bisect.o
LIB_OBJS += blob.o
LIB_OBJS += branch.o
+LIB_OBJS += builtin.o
LIB_OBJS += bulk-checkin.o
LIB_OBJS += bundle.o
LIB_OBJS += cache-tree.o
diff --git a/builtin.c b/builtin.c
new file mode 100644
index 000..6bdeb7c
--- /dev/null
+++ b/builtin.c
@@ -0,0 +1,225 @@
+#include builtin.h
+
+static struct cmd_struct commands[] = {
+ { add, cmd_add, RUN_SETUP | NEED_WORK_TREE },
+ { annotate, cmd_annotate, RUN_SETUP },
+ { apply, cmd_apply, RUN_SETUP_GENTLY },
+ { archive, cmd_archive },
+ { bisect--helper, cmd_bisect__helper, RUN_SETUP },
+ { blame, cmd_blame, RUN_SETUP },
+ { branch, cmd_branch, RUN_SETUP },
+ { bundle, cmd_bundle, RUN_SETUP_GENTLY },
+ { cat-file, cmd_cat_file, RUN_SETUP },
+ { check-attr, cmd_check_attr, RUN_SETUP },
+ { check-ignore, cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
+ { check-mailmap, cmd_check_mailmap, RUN_SETUP },
+ { check-ref-format, cmd_check_ref_format },
+ { checkout, cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
+ { checkout-index, cmd_checkout_index,
+ RUN_SETUP | NEED_WORK_TREE},
+ { cherry, cmd_cherry, RUN_SETUP },
+ { cherry-pick, cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+ { clean, cmd_clean, RUN_SETUP | NEED_WORK_TREE },
+ { clone, cmd_clone },
+ { column, cmd_column, RUN_SETUP_GENTLY },
+ { commit, cmd_commit, RUN_SETUP | NEED_WORK_TREE },
+ { commit-tree, cmd_commit_tree, RUN_SETUP },
+ { config, cmd_config, RUN_SETUP_GENTLY },
+ { count-objects, cmd_count_objects, RUN_SETUP },
+ { credential, cmd_credential, RUN_SETUP_GENTLY },
+ { describe, cmd_describe, RUN_SETUP },
+ { diff, cmd_diff },
+ { diff-files, cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
+ { diff-index, cmd_diff_index, RUN_SETUP },
+ { diff-tree, cmd_diff_tree, RUN_SETUP },
+ { fast-export, cmd_fast_export, RUN_SETUP },
+ { fetch, cmd_fetch, RUN_SETUP },
+ { fetch-pack, cmd_fetch_pack, RUN_SETUP },
+ { fmt-merge-msg, cmd_fmt_merge_msg, RUN_SETUP },
+ { for-each-ref, cmd_for_each_ref, RUN_SETUP },
+ { format-patch, cmd_format_patch, RUN_SETUP },
+ { fsck, cmd_fsck, RUN_SETUP },
+ { fsck-objects, cmd_fsck, RUN_SETUP },
+ { gc, cmd_gc, RUN_SETUP },
+ { get-tar-commit-id, cmd_get_tar_commit_id },
+ { grep, cmd_grep, RUN_SETUP_GENTLY },
+ { hash-object, cmd_hash_object },
+ { help, cmd_help },
+ { index-pack, cmd_index_pack, RUN_SETUP_GENTLY },
+ { init, cmd_init_db },
+ { init-db, cmd_init_db },
+ { log, cmd_log, RUN_SETUP },
+ { ls-files, cmd_ls_files, RUN_SETUP },
+ { ls-remote, cmd_ls_remote, RUN_SETUP_GENTLY },
+ { ls-tree, cmd_ls_tree, RUN_SETUP },
+ { mailinfo, cmd_mailinfo },
+ { mailsplit, cmd_mailsplit },
+ { merge, cmd_merge, RUN_SETUP | NEED_WORK_TREE },
+ { merge-base, cmd_merge_base, RUN_SETUP },
+ { merge-file, cmd_merge_file, RUN_SETUP_GENTLY },
+ { merge-index, cmd_merge_index, RUN_SETUP },
+ { merge-ours, cmd_merge_ours, RUN_SETUP },
+ { merge-recursive, cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
+ { merge-recursive-ours, cmd_merge_recursive, RUN_SETUP |
NEED_WORK_TREE },
+ { merge-recursive-theirs, cmd_merge_recursive, RUN_SETUP |
NEED_WORK_TREE },
+ { merge-subtree, cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
+ { merge-tree, cmd_merge_tree, RUN_SETUP },
+ { mktag, cmd_mktag, RUN_SETUP },
+ { mktree,
Re: [PATCH] Fix safe_create_leading_directories() for Windows
Johannes Schindelin johannes.schinde...@gmx.de writes: Hi, On Thu, 2 Jan 2014, Sebastian Schuberth wrote: See https://github.com/msysgit/git/pull/80. Thanks Sebastian! However, since the git.git project is not comfortable with the concept of pull requests (which is why you submitted this patch via mail), I believe that we have to explain the rationale in the commit message. So here goes my attempt: Thanks; the conclusion is correct --- you need a good commit message in the recorded history. That does not have anything to do with integrating with pulling from subsystem maintainers, which we regularly do. On Linux, we can get away with assuming that the directory separator is a forward slash, but that is wrong in general. For that purpose, the is_dir_sep() function was introduced a long time ago. By using it in safe_create_leading_directories(), we proof said function for use on platforms where the directory separator is different from Linux'. Perhaps with s|Linux|POSIX|, but more importantly, was there a specific error scenario that triggered this change? My quick reading of git grep suggests that the callsites of this function all assume that they are to use slashes as directory separators, and it may be that it is a bug in the specific caller that throws a backslash-separated paths to it. -- 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/RFC] Introduce git submodule add|update --attach
Francesco Pretto cez...@gmail.com writes: by default git submodule performs its add or update operations on a detached HEAD. This works well when using an existing full-fledged/indipendent project as the submodule, as there's less frequent need to update it or commit back changes. When the submodule is actually a large portion of shareable code between different projects, and the superproject needs to track very closely the evolution of the submodule (or the other way around), I feel more confortable to reattach the HEAD of the submodule with an existing branch. I may be missing some fundamental assumption in your mind when you did this change, but in a workflow where somebody wants submodule checkout to be on branches (as opposed to detached), wouldn't it make more sense not to detach in the first place, rather than introducing yet another option to re-attach? The documentation of submodule update seems to say that its merge and rebase modes do not detach in the first place (and it alludes to --checkout but it is unclear what it does purely from the documentation, as git submodule --help does not even list it as one of the options). And if there is a good reason why detaching to update and then (perhaps after verifying the result or cleaning it up? I dunno what the expected use case is, so I am purely guessing) attaching the result to a specific branch in separate steps, does it make sense to give --attach option to update in the first place? That makes the whole thing into a single step, not giving the user a chance to do anything in between, which I am guessing is the whole point of your not using the existing do not detach, work on a branch modes. Puzzled... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] blame: new option to better handle merged cherry-picks
Bernhard R. Link brl+...@mail.brlink.eu writes:
Allows to disable the git blame optimization of assuming that if there is a
parent of a merge commit that has the exactly same file content, then
only this parent is to be looked at.
I think this is what we usually call --full-history in git log
family, but more importantly, I do not think this is solving a valid
problem.
This optimization, while being faster in the usual case, means that in
the case of cherry-picks the blamed commit depends on which other commits
touched a file.
If for example one commit A modified both files b and c. And there are
commits B and C, B only modifies file b and C only modifies file c
(so that no conflicts happen), and assume A is cherry-picked as A'
and the two branches then merged:
--o-B---A
\ \
---C---A'--M---
So the contents of b at M is as the same as in A, so following 'b'
will see A and B changed that path, which is correct.
The contents of c at M is? It is different from A because at A c
lacks the change made to it at C. The merged result at M would
match C in A', no? So following 'c' will see A' and C changed that
path, no?
So what is wrong about it? If the original history were like this
instead, and A' were a cherry-pick of A, then what should happen?
--o-B---A'
\ \
---C---A---M---
Don't we want to see c blamed the same way?
Also, when handling a merge, we have to handle parents sequencially,
checking the difference between M with its first parent first, and
then passing blame for the remaining common lines to the remaining
parents. If you flip the order of parents of M when you merge A and
A' in your original history, and with your patch, what would you
see when you blame c? Wouldn't it notice that M:c is identical to c
in its first parent (now A') and pass the whole blame to A' anyway
with or without your change?
Then without this new option git blame blames the A|A' changes of
file b to A while blaming the changes of c to A'.
With the new option --no-parent-shortcut it blames both changes to the
same commit.
Signed-off-by: Bernhard R. Link brl...@debian.org
---
Documentation/blame-options.txt | 6 ++
builtin/blame.c | 5 -
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 0cebc4f..55dd12b 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -48,6 +48,12 @@ include::line-range-format.txt[]
Show the result incrementally in a format designed for
machine consumption.
+--no-parent-shortcut::
+ Always look at all parents of a merge and do not shortcut
+ to the first parent with no changes to the file looked at.
+ This takes more time but produces more reliable results
+ if branches with cherry-picked commits were merged.
+
--encoding=encoding::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/builtin/blame.c b/builtin/blame.c
index 4916eb2..dab2c36 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -45,6 +45,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int no_parent_shortcut;
static enum date_mode blame_date_mode = DATE_ISO8601;
static size_t blame_date_width;
@@ -1248,7 +1249,8 @@ static void pass_blame(struct scoreboard *sb, struct
origin *origin, int opt)
porigin = find(sb, p, origin);
if (!porigin)
continue;
- if (!hashcmp(porigin-blob_sha1, origin-blob_sha1)) {
+ if (!no_parent_shortcut
+ !hashcmp(porigin-blob_sha1, origin-blob_sha1)) {
pass_whole_blame(sb, origin, porigin);
origin_decref(porigin);
goto finish;
@@ -2247,6 +2249,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
static const char *contents_from = NULL;
static const struct option options[] = {
OPT_BOOL(0, incremental, incremental, N_(Show blame entries
as we find them, incrementally)),
+ OPT_BOOL(0, no-parent-shortcut, no_parent_shortcut,
N_(Don't take shortcuts in some merges but handle cherry-picks better)),
OPT_BOOL('b', NULL, blank_boundary, N_(Show blank SHA-1 for
boundary commits (Default: off))),
OPT_BOOL(0, root, show_root, N_(Do not treat root commits
as boundaries (Default: off))),
OPT_BOOL(0, show-stats, show_stats, N_(Show work cost
statistics)),
--
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] Fix safe_create_leading_directories() for Windows
Sebastian Schuberth sschube...@gmail.com writes: On 02.01.2014 20:55, Junio C Hamano wrote: Thanks; the conclusion is correct --- you need a good commit message in the recorded history. That does not have anything to do with integrating with pulling from subsystem maintainers, which we regularly do. I'll send a v2 which adds a proper commits message inline. Perhaps with s|Linux|POSIX|, but more importantly, was there a specific error scenario that triggered this change? Yes, cloning to a non-existent directory with Windows paths fails like: fatal: could not create work tree dir 'c:\foo\bar'.: No such file or directory OK. That was why I wanted to see (and Dscho correctly guessed) a good description in the proposed log message to see what problem the change is trying to address, so that we can judge if the change is tackling the right problem. Seems like the path to clone to is taken as-is from argv in cmd_clone(). So maybe another solution would be to replace all backslashes with forward slashes already there? That sounds like a workable alternative, and it might even be a preferable solution but if and only if clone's use of the function to create paths that lead to a new working tree location is the only (ab)use of the function. That was what I meant when I said it may be that it is a bug in the specific caller. AFAIK, the function was meant to be used only on paths we internally generate, and the paths we internally generate all are slash separated, in line with how paths are stored in the index. If we are going to change the meaning of the function so that it can now take any random path in platform-specific convention that may be incompatible with the internal notion of paths Git has (i.e. what is passed to safe_create_leading_directories() may have to be massaged into a slash-separated form before it can be used in the index and parsed to be stuffed into trees), it is fine to do so as long as all the codepaths understands the new world order, but my earlier git grep hits did not tell me that such a change is warranted. 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: What's cooking in git.git (Dec 2013, #05; Thu, 26)
Eric Sunshine sunsh...@sunshineco.com writes: On Fri, Dec 27, 2013 at 5:13 PM, Junio C Hamano gits...@pobox.com wrote: Eric Sunshine sunsh...@sunshineco.com writes: On Thu, Dec 26, 2013 at 4:08 PM, Junio C Hamano gits...@pobox.com wrote: [New Topics] Would $gmane/239575 [1] be of interest for New Topics? [1]: http://article.gmane.org/gmane.comp.version-control.git/239575/ Actually I was planning to scoop it up directly to master but forgot to do so. Make sense. Running git diff maint pu -- name-hash.c shows that we have added a comment that mentions index_name_exists---that needs to be adjusted, too, by the way. Oops, yes, I had noticed that too when testing atop 'pu' but then forgot about it when preparing the patch for submission on 'master'. I'm not sure how to move forward with this now that kb/fast-hashmap, with which it has a textual conflict, has graduated to 'next'. Should this become a two-patch series with one for scooping directly to 'master' and one for 'next' to sit atop kb/fast-hashmap? (But how will the textual conflict be handled?) I have a feeling that a small unused helper function is not a huge breakage that needs to be immediately fixed, so a single patch as a clean-up on top of whatever is cooking on 'next' should be the best approach, I would think. -- 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] Fix safe_create_leading_directories() for Windows
Sebastian Schuberth sschube...@gmail.com writes:
When cloning to a directory C:\foo\bar from Windows' cmd.exe where foo
does not exist yet, Git would throw an error like
fatal: could not create work tree dir 'c:\foo\bar'.: No such file or directory
Fix this by not hard-coding a platform specific directory separator into
safe_create_leading_directories().
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Sebastian Schuberth sschube...@gmail.com
---
sha1_file.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 760dd60..2114c58 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -110,12 +110,15 @@ int safe_create_leading_directories(char *path)
char *pos = path + offset_1st_component(path);
struct stat st;
- while (pos) {
- pos = strchr(pos, '/');
- if (!pos)
- break;
- while (*++pos == '/')
- ;
+ while (*pos) {
+ while (!is_dir_sep(*pos)) {
+ ++pos;
+ if (!*pos)
+ break;
+ }
+ /* skip consecutive directory separators */
+ while (is_dir_sep(*pos))
+ ++pos;
if (!*pos)
break;
*--pos = '\0';
This has a bit of conflict with another topic in flight; I think I
resolved it correctly, but please double check. The following is
how it would apply on top of 'pu'.
sha1_file.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 131ca97..9e686eb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -113,11 +113,12 @@ int safe_create_leading_directories(char *path)
while (!retval next_component) {
struct stat st;
- char *slash = strchr(next_component, '/');
-
- if (!slash)
+ char *slash = next_component;
+ while (!is_dir_sep(*slash))
+ ++slash;
+ if (!*slash)
return 0;
- while (*(slash + 1) == '/')
+ while (is_dir_sep(*(slash + 1)))
slash++;
next_component = slash + 1;
if (!*next_component)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] blame: new option to better handle merged cherry-picks
Bernhard R. Link brl+...@mail.brlink.eu writes: When giving git-blame the new option introduced with my patch, only the order of parents determines which commit is blamed. Without the option (i.e. the currently only possible behaviour) which commit is blamed depends what else touches other parts of the file. I am trying to figure out why that difference matters, in other words, when using the new option is actually useful. You give the command a scenario that can be solved in two equally valid ways (blaming to either A or A' is equally valid), and sometimes the command gives the identical result with or without the new option, and some other times the user gets a different but an equally valid result (but after traversing more history spending more cycles). I am not sure what problem the new option solves. I am trying to come up with an easy-to-understand explanation to the end users: If you want to see blame's result with the property X, use this option---it may have to spend extra cycles, but the property X is so desirable that it may be worth it. And I am having a hard time understanding what that X is. But in the example with one commit B touching also b and one commit C touching also c, there is (without the new option) always one part of the cherry-picked commit is blamed on the original and one on the cherry-picked, no matter how you order the parents. Yeah, the cherry-picked one will introduce the same change as the one that was cherry-picked, so if you look at the end result and ask where did _this_ line come from?, there are two equally plausible candidates, as blame output can give only one answer to each line. I still do not see why the one that is picked with the new option is better. At best, it looks to me that it is saying running with this option may (or may not) give a different answer, so run the command with and without it and see which one you like, which does not sound too useful to the end users. That is where my confusion comes from. (While by having your mainline always the most leftward parent, with the the new option you always get those commit blamed that is the first one this was introduced to mainline.) Yes, I vaguely recall we talked about adding --first-parent option to the command in the past. I do not remember what came out of that discussion. -- 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] Fix safe_create_leading_directories() for Windows
Junio C Hamano gits...@pobox.com writes:
This has a bit of conflict with another topic in flight; I think I
resolved it correctly, but please double check. The following is
how it would apply on top of 'pu'.
sha1_file.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 131ca97..9e686eb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -113,11 +113,12 @@ int safe_create_leading_directories(char *path)
while (!retval next_component) {
struct stat st;
- char *slash = strchr(next_component, '/');
-
- if (!slash)
+ char *slash = next_component;
+ while (!is_dir_sep(*slash))
Gaah; we need to check for the end of string here, i.e.
while (*slash !is_dir_sep(*slash))
will be what I'll queue on 'pu' for today.
+ ++slash;
+ if (!*slash)
return 0;
- while (*(slash + 1) == '/')
+ while (is_dir_sep(*(slash + 1)))
slash++;
next_component = slash + 1;
if (!*next_component)
--
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] Fix safe_create_leading_directories() for Windows
Junio C Hamano gits...@pobox.com writes:
Junio C Hamano gits...@pobox.com writes:
This has a bit of conflict with another topic in flight; I think I
resolved it correctly, but please double check. The following is
how it would apply on top of 'pu'.
sha1_file.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 131ca97..9e686eb 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -113,11 +113,12 @@ int safe_create_leading_directories(char *path)
while (!retval next_component) {
struct stat st;
-char *slash = strchr(next_component, '/');
-
-if (!slash)
+char *slash = next_component;
+while (!is_dir_sep(*slash))
Gaah; we need to check for the end of string here, i.e.
while (*slash !is_dir_sep(*slash))
will be what I'll queue on 'pu' for today.
+++slash;
+if (!*slash)
return 0;
-while (*(slash + 1) == '/')
+while (is_dir_sep(*(slash + 1)))
slash++;
next_component = slash + 1;
if (!*next_component)
Another thing I noticed (but I won't fix it up myself today, as I am
deep into today's integration cycle already) is that we temporarily
replace the slash with NUL and then restore them before we return,
but the restoration is done with the slash. If we were to go in the
direction of this patch, you may want to update that one to use
whatever dir-sep was used in the input string.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] t0000 cleanups
Jonathan Nieder jrnie...@gmail.com writes: Jeff King wrote: On Mon, Dec 30, 2013 at 10:51:25AM -0800, Jonathan Nieder wrote: These scratch areas for sub-tests should be under the t trash directory, but because the TEST_OUTPUT_DIRECTORY setting from the toplevel test leaks [...] This is not exactly true. The TEST_OUTPUT_DIRECTORY setting does not leak. t sets $TEST_DIRECTORY (which it must, so the sub-scripts can find test-lib.sh and friends), and then TEST_OUTPUT_DIRECTORY uses that as a default if it is not explicitly set. So I should have said something like the following instead: These scratch areas for sub-tests should be under the t trash directory, but because TEST_OUTPUT_DIRECTORY defaults to TEST_DIRECTORY which is exported to help sub-tests find test-lib.sh, the sub-test trash directories are created under the toplevel t/ directory instead. Because some of the sub-tests simulate failures, their trash directories are kept around. I had a private rewrite queued already, but the above is easier to read, so I'll replace it with this. Thanks. Fix it by explicitly setting TEST_OUTPUT_DIRECTORY appropriately for sub-tests. Thanks for catching it. Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] completion: introduce __gitcomp_2 ()
Ramkumar Ramachandra artag...@gmail.com writes:
There are situations where two classes of completions possible. For
example
branch.TAB
should try to complete
branch.master.
branch.autosetupmerge
branch.autosetuprebase
The first candidate has the suffix ., and the second/ third candidates
have the suffix . To facilitate completions of this kind, create a
variation of __gitcomp_nl () that accepts two sets of arguments and two
independent suffixes.
That sounds like a reasonable issue to address, but I do not quite
get why you need a new helper to do this.
If the original only knows to throw branch. + branch names +
trailing dot into COMPREPLY[] and does so by calling gitcomp_nl,
isn't it the matter of making another call to gitcomp_nl just after
the existing call to stuff branch.autosetup* with trailing SP to
append them to COMPREPLY[]?
Ahh, is that because the eventual call to __gitcompadd() starts the
iteration starting from zero, essentially forbidding you to
incrementally adding to COMPREPLY[] from multiple callers, even
though it is called comp add not replace with this single thing?
What I am wondering is if a cleaner solution that can be reused by
later needs that may have more than two data sources (or more than
two suffixes) might be to create a variant of __gitcomp_nl that does
not clear existing entries in COMPREPLY[] array, add a helper to
clear the array, which would make the existing one to:
__gitcomp_nl () {
__gitcomp_clear
__gitcomp_nl_append $@
}
and then complete branch.* using two calls to __gitcomp_*, letting
the first one clear and later one(s) accumulate:
__gitcomp_nl $(__git_heads) $pfx $cur_ .
__gitcomp_nl_append $autosetupmerge\nautosetuprebase\n $pfx $cur_
Will queue as-is.
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 3/4] completion: fix branch.autosetup(merge|rebase)
Ramkumar Ramachandra artag...@gmail.com writes:
branch.*.*)
local pfx=${cur%.*}. cur_=${cur##*.}
+ if [ $pfx == branch.autosetupmerge. ] ||
+ [ $pfx == branch.autosetuprebase. ]; then
+ return
+ fi
__gitcomp remote pushremote merge mergeoptions rebase $pfx
$cur_
return
;;
I do not quite understand this change.
If we are looking at branch.autosetupmerge. followed by something,
who typed that final dot? If you are working on a topic about
auto-setup-merge and named your branch autosetupmerge, don't you
want to be able to configure various aspect of that branch via
branch.autosetupmerge.{remote,merge} etc., just like you can do so
for your topic branch via branch.topic.{remote,merge} etc.,
regardless of your use of autosetupmerge option across branches?
Besides, it smells fishy to me that you need to enumerate and
special case these two here, and then you have to repeat them below
in a separate case arm.
branch.*)
local pfx=${cur%.*}. cur_=${cur#*.}
- __gitcomp_nl $(__git_heads) $pfx $cur_ .
+ __gitcomp_2 $(__git_heads)
+ autosetupmerge autosetuprebase
+ $pfx $cur_ .
return
;;
guitool.*.*)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)
Ramkumar Ramachandra artag...@gmail.com writes:
Junio C Hamano wrote:
If we are looking at branch.autosetupmerge. followed by something,
who typed that final dot?
I admit that it's a very unlikely case. The user did:
$ branch.autosetupmerTAB
hit backspace to delete the trailing space, inserted a dot, and hit TAB
again.
If you are working on a topic about
auto-setup-merge and named your branch autosetupmerge, don't you
want to be able to configure various aspect of that branch via
branch.autosetupmerge.{remote,merge} etc., just like you can do so
for your topic branch via branch.topic.{remote,merge} etc.,
regardless of your use of autosetupmerge option across branches?
My reasoning was that being correct was more important that being
complete. So, if by some horrible chance, the user names her branch
autosetupmerge, we don't aid her in completions.
Besides, it smells fishy to me that you need to enumerate and
special case these two here, and then you have to repeat them below
in a separate case arm.
I'm not too irked about correctness in this odd case; seeing that you
aren't either, I'll resubmit the series without this hunk (+ the hunk
in remote.pushdefault).
You seem to be calling it incorrect to give the same degree of
completion for a branch the user named autosetupmerge as another
branch topic, but I think it is incorrect not to, so I cannot tell
if we are agreeing or disagreeing.
Puzzled...
--
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/4] completion: introduce __gitcomp_2 ()
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: __gitcomp_nl $(__git_heads) $pfx $cur_ . __gitcomp_nl_append $autosetupmerge\nautosetuprebase\n $pfx $cur_ This is not a bad idea at all. I'm just afraid that we might be leaving open ends: What happens if the $pfx isn't the same in both cases? Who keeps track of the index i of COMPREPLY (it's currently a local variable)? If we make it global, doesn't every function that deals with COMPREPLY be careful to reset it? I am not sure what you are worried about $pfx; what does it do when you have strings with different prefix in COMPREPLY? If it breaks, then the answer is don't do it then. Doesn't an array know its own length and give you a way to ask? More importantly, can you see a usecase for more than two completion classes? I am not sure why you think it is more important. Somebody lacked forsight and designed an interface that would allow only one completion classes (whatever that means) and called the result sufficient. It has been sufficient for a long time. Later you came, found that one was not enough, and added an inteface that would allow only two, and called the result sufficient. See a pattern? Anticipating unforseen possibilities for enhancement and preparing an easy way to support them without overengineering is what the prepare an appending variant suggestion is about, and by definition, unforseen possibilities have not been seen yet ;-) Imagine if one is flipping 47 topic branches from 6 contributors whose names all begin with 'j'. I can see that such a person would appreciate if git config branch.jTAB did not dump all 47 topics at once but offered jc/ jk/ jl/ jm/ jn/ js/ instead, and then a follow-up completion of git config branch.jk/TAB expanded to names of topics from that single contributor jk. Wouldn't the way to give these be either to return these two-letter hierarchy names with slash as the suffix or to return list of two-letter plus a slash with an empty suffix? Either way, that is using something different from a dot or a space, so that may count as the third, I guess. -- 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: aborted 'git fetch' leaves workspace unusable
Stephen Leake stephen_le...@stephe-leake.org writes: Junio C Hamano gits...@pobox.com writes: stephen_le...@stephe-leake.org writes: However, in this case, even running the fetch was a mistake; I would have prefered that it leave FETCH_HEAD in its previous state. I think the clearing of leftover FETCH_HEAD is one of the early things git fetch does, unless --append is in effect. I haven't looked at the code for a long time, but it may be possible to move the logic of doing so around so that this clearing is done as lazily as possible. I however suspect that such a change may have fallouts on other people who are writing tools like yours; they may be depending on seeing FETCH_HEAD cleared after a failed fetch, and be surprised to see a stale contents after they (attempt to) run git fetch in it. So it is not so clear if it is a good thing to change the behaviour of git fetch not to touch FETCH_HEAD upon a failure. Ok; backwards compatibility is important. Perhaps FETCH_HEAD could be copied to FETCH_HEAD_prev or some such, to allow recovering in an error case? As FETCH_HEAD is purely ephemeral (so are other ephemeral markers like MERGE_HEAD and friends), and the promise between git fetch and its users has always been that an invocation of git fetch clears FETCH_HEAD (logical consequence of which is that the user runs git fetch only when s/he are _done_ with the old FETCH_HEAD), I doubt FETCH_HEAD_prev would add much value to the system and only introduce more things we have to worry about, like when will it be cleaned?, what happens to the old value in FETCH_HEAD_prev?. It is like asking Should 'rm -f foo' save the original 'foo' to somewhere else just in case?. If your emacs wrapper for some reason needs to know what happened in the last fetch, I imagine that it can check and record what was in FETCH_HEAD before issuing git fetch, so... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/4] Speed up is_git_command() by checking early for internal commands
Jeff King p...@peff.net writes: On Thu, Jan 02, 2014 at 11:41:05AM -0800, Junio C Hamano wrote: - builtin/merge.c is the same, but it is conceptually even worse. It has the end-user supplied string and wants to see if it is a valid strategy. If the user wants to use a custom strategy, a single stat() to make sure if it exists should suffice, and the error codepath should load the command list to present the names of available ones in the error message. Is it a single stat()? I think we would need to check each element of $PATH. Yeah, load_command_list() iterates over the members of env_path. Though in practice, the exec-dir would be the first thing we check, and where we would find the majority of hits. So it would still be a win, as we would avoid touching anything but the exec-dir in the common case. Exactly. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 1/2] fetch --prune: Always print header url
Tom Miller jacker...@gmail.com writes:
If fetch --prune is run with no new refs to fetch, but it has refs
to prune. Then, the header url is not printed as it would if there were
new refs to fetch.
Output before this patch:
$ git fetch --prune remote-with-no-new-refs
x [deleted] (none) - origin/world
Output after this patch:
$ git fetch --prune remote-with-no-new-refs
From https://github.com/git/git
x [deleted] (none) - origin/test
Signed-off-by: Tom Miller jacker...@gmail.com
And (needless to say) when there are refs to be fetched and also
pruned, the shown_url logic prevents us from giving two identical
headers From there. Sounds sane.
---
I decided it is not worth writing a function to format the header url
that is printed by fetch. Instead, I will duplicate the formatting logic
for now because I plan on following up this patch set with another patch
to stop striping the trailing / and .git from the url. When that
patch is finished it will remove the majority of the duplicated logic.
OK, let's see how it goes.
Will replace what has been queued on 'pu'. The incremental change
relative to the old one looked sensible and I think this one is
ready for 'next'.
Thanks.
builtin/fetch.c | 32 +++-
t/t5510-fetch.sh | 12
2 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1e7d617..1b81cf9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -44,6 +44,7 @@ static struct transport *gtransport;
static struct transport *gsecondary;
static const char *submodule_prefix = ;
static const char *recurse_submodules_default;
+static int shown_url = 0;
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -535,7 +536,7 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
{
FILE *fp;
struct commit *commit;
- int url_len, i, shown_url = 0, rc = 0;
+ int url_len, i, rc = 0;
struct strbuf note = STRBUF_INIT;
const char *what, *kind;
struct ref *rm;
@@ -708,17 +709,36 @@ static int fetch_refs(struct transport *transport,
struct ref *ref_map)
return ret;
}
-static int prune_refs(struct refspec *refs, int ref_count, struct ref
*ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref
*ref_map,
+ const char *raw_url)
{
- int result = 0;
+ int url_len, i, result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count,
ref_map);
+ char *url;
const char *dangling_msg = dry_run
? _( (%s will become dangling))
: _( (%s has become dangling));
+ if (raw_url)
+ url = transport_anonymize_url(raw_url);
+ else
+ url = xstrdup(foreign);
+
+ url_len = strlen(url);
+ for (i = url_len - 1; url[i] == '/' 0 = i; i--)
+ ;
+
+ url_len = i + 1;
+ if (4 i !strncmp(.git, url + i - 3, 4))
+ url_len = i - 3;
+
for (ref = stale_refs; ref; ref = ref-next) {
if (!dry_run)
result |= delete_ref(ref-name, NULL, 0);
+ if (verbosity = 0 !shown_url) {
+ fprintf(stderr, _(From %.*s\n), url_len, url);
+ shown_url = 1;
+ }
if (verbosity = 0) {
fprintf(stderr, x %-*s %-*s - %s\n,
TRANSPORT_SUMMARY(_([deleted])),
@@ -726,6 +746,7 @@ static int prune_refs(struct refspec *refs, int
ref_count, struct ref *ref_map)
warn_dangling_symref(stderr, dangling_msg, ref-name);
}
}
+ free(url);
free_refs(stale_refs);
return result;
}
@@ -854,11 +875,12 @@ static int do_fetch(struct transport *transport,
* don't care whether --tags was specified.
*/
if (ref_count) {
- prune_refs(refs, ref_count, ref_map);
+ prune_refs(refs, ref_count, ref_map, transport-url);
} else {
prune_refs(transport-remote-fetch,
transport-remote-fetch_refspec_nr,
-ref_map);
+ref_map,
+transport-url);
}
}
free_refs(ref_map);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..87e896d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,16 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack 3
'
+test_expect_success 'fetch --prune prints the remotes url' '
+ git branch goodbye
Re: [PATCH] get_octopus_merge_bases(): cleanup redundant variable
Vasily Makarov einmal...@gmail.com writes:
pptr is needless. Some related code got cleaned as well
Signed-off-by: Vasily Makarov einmal...@gmail.com
---
commit.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/commit.c b/commit.c
index de16a3c..4a7a192 100644
--- a/commit.c
+++ b/commit.c
@@ -834,26 +834,23 @@ static struct commit_list *merge_bases_many(struct
commit *one, int n, struct co
struct commit_list *get_octopus_merge_bases(struct commit_list *in)
{
struct commit_list *i, *j, *k, *ret = NULL;
- struct commit_list **pptr = ret;
- for (i = in; i; i = i-next) {
- if (!ret)
- pptr = commit_list_insert(i-item, pptr)-next;
- else {
- struct commit_list *new = NULL, *end = NULL;
-
- for (j = ret; j; j = j-next) {
- struct commit_list *bases;
- bases = get_merge_bases(i-item, j-item, 1);
- if (!new)
- new = bases;
- else
- end-next = bases;
- for (k = bases; k; k = k-next)
- end = k;
- }
- ret = new;
+ commit_list_insert(in-item, ret);
I suspect that the original code would have behaved well (and I also
suspect that it was designed to) even if in==NULL upon entry, but
this version will crash here. Nothing a simple
if (!in)
return NULL;
upfront cannot fix, though.
And if we add these three lines back, the patch will become 18
insertions with 18 deletions but the result is very much more
readable than the original ;-).
+
+ for (i = in-next; i; i = i-next) {
+ struct commit_list *new = NULL, *end = NULL;
+
+ for (j = ret; j; j = j-next) {
+ struct commit_list *bases;
+ bases = get_merge_bases(i-item, j-item, 1);
+ if (!new)
+ new = bases;
+ else
+ end-next = bases;
+ for (k = bases; k; k = k-next)
+ end = k;
}
+ ret = new;
}
return ret;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] completion: fix branch.autosetup(merge|rebase)
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: You seem to be calling it incorrect to give the same degree of completion for a branch the user named autosetupmerge as another branch topic, but I think it is incorrect not to, so I cannot tell if we are agreeing or disagreeing. No, what's incorrect is providing completions for $ git config branch.autosetupmerge.TAB when no branch called autosetupmerge exists. The purpose of the hunk (which I now removed) was to prevent such completions, ... Hmph, but in a repository without 'foo', I just did $ git config branch.foo.TAB branch.foo.merge branch.foo.rebase branch.foo.mergeoptions branch.foo.remote and got offered the above. How would that removed hunk that special cased those autosetupmerge etc. helped such case? If it _were_ about correctness, and the definition of correctness were that completing branch.foo.TAB to offer these four variables is wrong until refs/heads/foo materializes, the fix would have checked if there already is such a branch and refused to complete otherwise, not special case a few known names such as autosetup*. As there is no reason to forbid setting configuration variables for a branch 'foo' you are going to create before you actually create it with git branch foo, I do not necessarily agree with the above definition of correctness, either. So it was completely bogus hunk and it is good we noticed and decided to remove it, I guess. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/4] completion: introduce __gitcomp_nl_append ()
Ramkumar Ramachandra artag...@gmail.com writes:
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 51c2dd4..bf358d6 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -233,6 +233,19 @@ __gitcomp_nl ()
__gitcompadd $1 ${2-} ${3-$cur} ${4- }
}
+# Variation of __gitcomp_nl () that appends to the existing list of
+# completion candidates, COMPREPLY.
+__gitcomp_nl_append ()
+{
+ local IFS=$'\n'
+ local i=${#COMPREPLY[@]}
+ for x in $1; do
+ if [[ $x == $3* ]]; then
+ COMPREPLY[i++]=$2$x$4
+ fi
+ done
+}
Hmph. Why so much duplication with __gitcompadd, though.
I would have expected that this append behaviour to be done at the
lower level by introducing __gitcompappend that does not forcibly
truncate by starting from a hard-coded i=0, i.e. a collection of
small helper functions plus a single implementation of the logic to
push elements into COMPREPLY[] in __gitcompappend, perhaps like
these:
__gitcompappend () {
local i=${#COMPREPLY[@]}
for x in $1; do
if [[ $x == $3* ]]; then
COMPREPLY[i++]=$2$x$4
fi
done
}
__gitcompadd () {
COMPREPLY=()
__gitcompappend $@
}
__gitcomp_nl_append () {
local IFS=$'\n'
__gitcompappend $1 ${2-} ${3-$cur} ${4- }
}
__gitcomp_nl () {
COMPREPLY=()
__gitcomp_nl_append $@
}
Is it because going this route and doing it at such a low level
would make zsh completion (which I have no clue about ;-)
unnecessarily complex?
+
# Generates completion reply with compgen from newline-separated possible
# completion filenames.
# It accepts 1 to 3 arguments:
diff --git a/contrib/completion/git-completion.zsh
b/contrib/completion/git-completion.zsh
index 6fca145..6b77968 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -76,6 +76,14 @@ __gitcomp_nl ()
compadd -Q -S ${4- } -p ${2-} -- ${=1} _ret=0
}
+__gitcomp_nl_append ()
+{
+ emulate -L zsh
+
+ local IFS=$'\n'
+ compadd -Q -S ${4- } -p ${2-} -- ${=1} _ret=0
+}
+
__gitcomp_file ()
{
emulate -L zsh
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] completion: fix remote.pushdefault
Ramkumar Ramachandra artag...@gmail.com writes:
When attempting to complete
$ git config remote.pushTAB
'pushdefault' doesn't come up. This is because $cur is matched with
remote.* and a list of remotes are completed. Add 'pushdefault' to the
list of remotes using __gitcomp_nl_append ().
Add ... to the list of remotes (same for list of branches in
3/4) sounds somewhat funny, doesn't it? How about
Add remote.pushdefault as a candidate for completion, too.
or something like that instead?
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
contrib/completion/git-completion.bash | 1 +
1 file changed, 1 insertion(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 75c7302..345ceff 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1883,6 +1883,7 @@ _git_config ()
remote.*)
local pfx=${cur%.*}. cur_=${cur#*.}
__gitcomp_nl $(__git_remotes) $pfx $cur_ .
+ __gitcomp_nl_append pushdefault $pfx $cur_
return
;;
url.*.*)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] completion: prioritize ./git-completion.bash
brian m. carlson sand...@crustytoothpaste.net writes:
On Fri, Jan 03, 2014 at 01:30:28PM +0530, Ramkumar Ramachandra wrote:
To ease development, prioritize ./git-completion.bash over other
standard system paths.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
contrib/completion/git-completion.zsh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.zsh
b/contrib/completion/git-completion.zsh
index fac5e71..6fca145 100644
--- a/contrib/completion/git-completion.zsh
+++ b/contrib/completion/git-completion.zsh
@@ -30,10 +30,10 @@ if [ -z $script ]; then
local -a locations
local e
locations=(
+$(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
'/etc/bash_completion.d/git' # fedora, old debian
'/usr/share/bash-completion/completions/git' # arch, ubuntu,
new debian
'/usr/share/bash-completion/git' # gentoo
-$(dirname ${funcsourcetrace[1]%:*})/git-completion.bash
)
for e in $locations; do
test -f $e script=$e break
I'm not clear on this change. It looks like this loads
git-completion.bash from the same directory as git-completion.zsh. Is
this correct?
I think the idea is to help those who have installed a closer to
bleeding-edge version of completion scripts --- they will not be
reading their zsh completions from the system default locations,
and the place next to it would be a place more likely to have the
matching bleeding-edge version of bash completion than the system
default place.
Your commit message says ./, and if that's the case, it
has the same security problems as putting . first in your PATH.
A correct observation. I think
Subject: zsh completion: find matching custom bash completion
If zsh completion is being read from a location that is
different from system-wide default, it is likely that the
user is trying to use a custom version, perhaps closer to
the bleeding edge, installed in her own directory. We will
more likely to find the matching bash completion script in
the same directory than in those system default places.
would probably a lot closer to what the patch really does.
--
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: Bug report: stash in upstream caused remote fetch to fail
Jeff King p...@peff.net writes: On Fri, Jan 03, 2014 at 04:12:51PM -0500, Matt Burke wrote: + git init -q + git fetch -q -fu ../../../other '+refs/*:refs/*' fatal: bad object 9b985fbe6a2b783c16756077a8be261c94b6c197 error: ../../../other did not send all necessary objects I was going to ask you to send your repository, but I can easily reproduce here. I guess people don't run into it because it's uncommon to fetch the whole refs/ namespace from a non-bare repo (and bare repos do not tend to have stashes). Here's a minimal reproduction recipe: git init repo cd repo echo content foo git add . git commit -m foo echo more foo git stash git init --bare sub cd sub git fetch .. 'refs/*:refs/*' It looks like we are not feeding refs/stash properly to pack-objects. I'll try to take a closer look later today. I looked at this in the past and I vaguely recall that we reject it in the for-each-ref loop with check-ref-format saying eh, that is a single-level name. At that point I stopped digging, thinking it was a feature ;-) based on your exact observation about stash vs bare/non-bare. -- 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] [RFC] Making use of bitmaps for thin objects
Jeff King p...@peff.net writes: We could probably teach index-pack an --assume-refs-are-thin option to optimize for this case, and have fetch-pack/receive-pack pass it whenever they know that delta-base-offset was negotiated. I thought the existing negotiation merely means I understand offset encoded bases, so you are allowed to use that encoding, not I will not accept encoding other than the offset format, so you must use that encoding for everything. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
Heiko Voigt hvo...@hvoigt.net writes: On Sun, Jan 05, 2014 at 10:27:19PM +0100, Francesco Pretto wrote: 2014/1/5 W. Trevor King wk...@tremily.us: On Sun, Jan 05, 2014 at 04:53:12AM +0100, Francesco Pretto wrote: Also it could break some users that rely on the current behavior. The current code always has a detached HEAD after an initial-clone update, regardless of submodule.name.update, which doesn't match those docs either. I perfectly agree with you that the documentation is a bit contradictory with regard to update command and detached HEAD. That's why it's so hard to add a feature and keep the same spirit of those that coded submodules at first. Also, I think that submodules didn't get much feedback with regards to these pitfalls because many people try to setup them, they see that update detaches the HEAD and they think hmmm, maybe submodules are not what I was looking for. I am not so sure about that. Why should detached HEAD make you think like that? For us at $dayjob we have a pre-commit hook that denies you to commit on a detached HEAD and asks you to create a branch first. Perception is irrational ;-) We long-timers think detached is a perfect starting point for both users of submodule who merely want to use the specified commit and developers who want to work on the submodule to match the need of the superproject. The former do not have to do anything, and the latter will have to chdir to the submodule working tree and create a branch (or update the branch with rebase or pull on top of the specified commit) as the first thing before doing anything. Not everybody is a long-timer, but the saving grace is that nobody stays a newcomer. BUT. - developers checkout that commit and don't pull (you can't do git pull in a detached HEAD); Exactly. We consider pull evil ;-) Seriously: To update we only do fast forward merges of local stable branches. ... Yes, why would you do a git pull in a submodule? Don't you want to do something like git checkout -t -b dev/my-topic origin/master to start your development? As long as origin/master contains the commit specified by the superproject, that would work, but it may be a good thing to use a mode of submodule.*.update that does not have to drop the user into a detached state in the first place. I somehow thought that was what rebase (or merge) was about, that is, starting from the state where a branch is checked out in the submodule working tree, an update in the superproject would cause that branch checked out in the submodule brought up to date with respect to the commit specified in the superproject tree. If that is not how it is supposed to work, please correct me---and we may have to add another mode that does so (or even make rebase/merge do so as a bugfix). And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? -- 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] git-submodule.sh: Support 'checkout' as a valid update command
W. Trevor King wk...@tremily.us writes:
On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
+case $update_module in
+'')
+;; # Unset update mode
+checkout | rebase | merge | none)
+;; # Known update modes
+!*)
+;; # Custom update command
+*)
+update_module=
+echo 2 warning: invalid update mode for
submodule '$name'
+;;
+esac
I'd prefer `die …` to `echo 2 …`. It's hard to know if mapping
the user's preferred (unknown) update mechanism to 'checkout' is
serious or not.
This commit also makes me think that --rebase, --merge, and --checkout
should be replaced with a single --update={rebase|merge|checkout|!…}
option, but that's probably food for another commit (and a long
finger-breaking deprecation period).
All of the above points sound sensible to 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: [GIT PULL] l10n update for maint branch
Jiang Xin worldhello@gmail.com writes: Please pull l10n update for maint branch. It can be merged to master without conflict. 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] Improve user-manual html and pdf formatting
Thomas Ackermann th.ac...@arcor.de writes: Use asciidoc style 'article' instead of 'book' and change asciidoc title level. This removes blank first page and superfluous Part I page (there is no Part II) in pdf output. Also pdf size is decreased by this from 77 to 67 pages. In html output this removes unnecessary sub-tocs and chapter numbering. Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/Makefile| 2 +- Documentation/user-manual.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 91a12c7..36c58fc 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -324,7 +324,7 @@ manpage-base-url.xsl: manpage-base-url.xsl.in user-manual.xml: user-manual.txt user-manual.conf $(QUIET_ASCIIDOC)$(RM) $@+ $@ \ - $(ASCIIDOC) $(ASCIIDOC_EXTRA) -b docbook -d book -o $@+ $ \ + $(ASCIIDOC) $(ASCIIDOC_EXTRA) -b docbook -d article -o $@+ $ \ mv $@+ $@ technical/api-index.txt: technical/api-index-skel.txt \ diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt index cbb01a1..130c2f4 100644 --- a/Documentation/user-manual.txt +++ b/Documentation/user-manual.txt @@ -1,5 +1,5 @@ -Git User Manual -___ +#65279;Git User Manual +=== Will queue after dropping the extra. 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] git-submodule.sh: Support 'checkout' as a valid update command
Junio C Hamano gits...@pobox.com writes:
W. Trevor King wk...@tremily.us writes:
On Sun, Jan 05, 2014 at 03:50:48AM +0100, Francesco Pretto wrote:
+ case $update_module in
+ '')
+ ;; # Unset update mode
+ checkout | rebase | merge | none)
+ ;; # Known update modes
+ !*)
+ ;; # Custom update command
+ *)
+ update_module=
+ echo 2 warning: invalid update mode for
submodule '$name'
+ ;;
+ esac
I'd prefer `die …` to `echo 2 …`. It's hard to know if mapping
the user's preferred (unknown) update mechanism to 'checkout' is
serious or not.
This commit also makes me think that --rebase, --merge, and --checkout
should be replaced with a single --update={rebase|merge|checkout|!…}
option, but that's probably food for another commit (and a long
finger-breaking deprecation period).
All of the above points sound sensible to me.
I'll tentatively queue this on 'pu' (with the suggested die
update), with some rewording of the log message. The patch needs to
be signed-off, though.
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 v2 10/17] lock_ref_sha1_basic(): on SCLD_VANISHED, retry
Michael Haggerty mhag...@alum.mit.edu writes:
If safe_create_leading_directories() fails because a file along the
path unexpectedly vanished, try again (up to 3 times).
This can occur if another process is deleting directories at the same
time as we are trying to make them. For example, git pack-refs
--all tries to delete the loose refs and any empty directories that
are left behind. If a pack-refs process is running, then it might
delete a directory that we need to put a new loose reference in.
If safe_create_leading_directories() thinks this might have happened,
then take its advice and try again (maximum three attempts).
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 3926136..6eb8a02 100644
--- a/refs.c
+++ b/refs.c
@@ -2039,6 +2039,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int type, lflags;
int mustexist = (old_sha1 !is_null_sha1(old_sha1));
int missing = 0;
+ int attempts = 3;
lock = xcalloc(1, sizeof(struct ref_lock));
lock-lock_fd = -1;
@@ -2093,7 +2094,15 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
if ((flags REF_NODEREF) (type REF_ISSYMREF))
lock-force_write = 1;
- if (safe_create_leading_directories(ref_file)) {
+ retry:
+ switch (safe_create_leading_directories(ref_file)) {
+ case SCLD_OK:
+ break; /* success */
+ case SCLD_VANISHED:
+ if (--attempts 0)
+ goto retry;
+ /* fall through */
Hmph.
Having no backoff/sleep at all might be OK here as long as the other
side that removes does not retry (and I do not think the other side
would, even though I haven't read through the series to the end yet
;-)).
This may be just a style thing, but I find that the variable name
attempts that starts out as 3 quite misleading, as its value is
not the number of attempts made but the remaining number of
attempts allowed. Starting it from 0 and then
if (attempts++ MAX_ATTEMPTS)
goto retry;
would be one way to clarify it. Renaming it to remaining_attempts
would be another.
+ default:
last_errno = errno;
error(unable to create directory for %s, ref_file);
goto error_return;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/17] remove_dir_recurse(): handle disappearing files and directories
Michael Haggerty mhag...@alum.mit.edu writes:
If a file or directory that we are trying to remove disappears (e.g.,
because another process has pruned it), do not consider it an error.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
dir.c | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 11e1520..716b613 100644
--- a/dir.c
+++ b/dir.c
@@ -1476,7 +1476,9 @@ static int remove_dir_recurse(struct strbuf *path, int
flag, int *kept_up)
flag = ~REMOVE_DIR_KEEP_TOPLEVEL;
dir = opendir(path-buf);
if (!dir) {
- if (errno == EACCES !keep_toplevel)
+ if (errno == ENOENT)
+ return keep_toplevel ? -1 : 0;
If we were told that foo/bar must go, but do not bother removing
foo/, is it an error if foo itself did not exist? I think this
step does not change the behaviour from the original (we used to say
oh, we were told to keep_toplevel, and the top-level cannot be read
for whatever reason, so it is an error), but because this series is
giving us a finer grained error diagnosis, we may want to think
about it further, perhaps as a follow-up.
I also wonder why the keep-toplevel logic is in this recurse part
of the callchain. If everything in foo/bar/ can be removed but
foo/bar/ is unreadable, it is OK, when opendir(foo/bar) failed
with EACCESS, to attempt to rmdir(foo/bar) whether we were told
not to attempt removing foo/, no?
+ else if (errno == EACCES !keep_toplevel)
That is, I am wondering why this part just checks !keep_toplevel,
not
(!keep_toplevel || has_dir_separator(path-buf))
or something.
/*
* An empty dir could be removable even if it
* is unreadable:
@@ -1496,13 +1498,21 @@ static int remove_dir_recurse(struct strbuf *path,
int flag, int *kept_up)
strbuf_setlen(path, len);
strbuf_addstr(path, e-d_name);
- if (lstat(path-buf, st))
- ; /* fall thru */
- else if (S_ISDIR(st.st_mode)) {
+ if (lstat(path-buf, st)) {
+ if (errno == ENOENT)
+ /*
+ * file disappeared, which is what we
+ * wanted anyway
+ */
+ continue;
+ /* fall thru */
+ } else if (S_ISDIR(st.st_mode)) {
if (!remove_dir_recurse(path, flag, kept_down))
continue; /* happy */
- } else if (!only_empty !unlink(path-buf))
+ } else if (!only_empty
+(!unlink(path-buf) || errno == ENOENT)) {
continue; /* happy, too */
+ }
/* path too long, stat fails, or non-directory still exists */
ret = -1;
@@ -1512,7 +1522,7 @@ static int remove_dir_recurse(struct strbuf *path, int
flag, int *kept_up)
strbuf_setlen(path, original_len);
if (!ret !keep_toplevel !kept_down)
- ret = rmdir(path-buf);
+ ret = (!rmdir(path-buf) || errno == ENOENT) ? 0 : -1;
else if (kept_up)
/*
* report the uplevel that it is not an error that we
--
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 17/17] rename_tmp_log(): on SCLD_VANISHED, retry
Michael Haggerty mhag...@alum.mit.edu writes:
If safe_create_leading_directories() fails because a file along the
path unexpectedly vanished, try again from the beginning. Try at most
3 times.
As the previous step bumped it from 3 to 4 without explanation, the
above no longer reflects reality ;-)
The series mostly looked sane from a cursory read.
Will re-queue. Thanks.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 490525a..810f802 100644
--- a/refs.c
+++ b/refs.c
@@ -2533,7 +2533,14 @@ static int rename_tmp_log(const char *newrefname)
int attempts = 4;
retry:
- if (safe_create_leading_directories(git_path(logs/%s, newrefname))) {
+ switch (safe_create_leading_directories(git_path(logs/%s,
newrefname))) {
+ case SCLD_OK:
+ break; /* success */
+ case SCLD_VANISHED:
+ if (--attempts 0)
+ goto retry;
+ /* fall through */
+ default:
error(unable to create directory for %s, newrefname);
return -1;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 03/17] safe_create_leading_directories(): add explicit slash pointer
Michael Haggerty mhag...@alum.mit.edu writes:
Keep track of the position of the slash character independently of
pos, thereby making the purpose of each variable clearer and
working towards other upcoming changes.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
This step has an interaction with $gmane/239878 where Windows folks
want it to pay attention to is_dir_sep()---over there, a backslash
could separate directory path components.
AFAIK, the function was meant to be used only on paths we internally
generate, and the paths we internally generate all are slash
separated, so it could be argued that feeding a path, whose path
components are separated by backslashes, that we obtained from the
end user without converting it to the internal form in some
codepaths (e.g. $there in git clone $url $there) are bugs we
acquired over time that need to be fixed, but it is easy enough to
use is_dir_sep() here to work it around, and doing so will
not negatively affect
1. UNIX-only projects by forbidding use of a byte with backslash in
it as a path component character (yes, I am imagining using
Shift-JIS that can use a backslash as the second byte of
two-byte character in the pathname on UNIX); and
2. UNIX-and-Windows mixed projects, as you cannot sanely use such a
pathname with backslash as part of a path component if its tree
needs to be checked out on Windows.
sha1_file.c | 20 +++-
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index cc9957e..197766d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -111,19 +111,21 @@ int safe_create_leading_directories(char *path)
while (pos) {
struct stat st;
+ char *slash = strchr(pos, '/');
- pos = strchr(pos, '/');
- if (!pos)
+ if (!slash)
break;
- while (*++pos == '/')
- ;
+ while (*(slash + 1) == '/')
+ slash++;
+ pos = slash + 1;
if (!*pos)
break;
- *--pos = '\0';
+
+ *slash = '\0';
if (!stat(path, st)) {
/* path exists */
if (!S_ISDIR(st.st_mode)) {
- *pos = '/';
+ *slash = '/';
return -3;
}
} else if (mkdir(path, 0777)) {
@@ -131,14 +133,14 @@ int safe_create_leading_directories(char *path)
!stat(path, st) S_ISDIR(st.st_mode)) {
; /* somebody created it since we checked */
} else {
- *pos = '/';
+ *slash = '/';
return -1;
}
} else if (adjust_shared_perm(path)) {
- *pos = '/';
+ *slash = '/';
return -2;
}
- *pos++ = '/';
+ *slash = '/';
}
return 0;
}
--
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] format-patch: introduce format.defaultTo
Ramkumar Ramachandra artag...@gmail.com writes:
A very common workflow for preparing patches involves working off a
topic branch and generating patches against 'master' to send off to the
maintainer. However, a plain
$ git format-patch -o outgoing
is a no-op on a topic branch,...
Two points.
- why is a single branch name sufficient?
- is it a better option to simply default to @{u}, if one exists,
instead of failing?
--
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: [BUG] rebase --onto doesn't abort properly
Ramkumar Ramachandra artag...@gmail.com writes: Hi, On the latest git, I noticed that a rebase --onto doesn't abort properly. Steps to reproduce: # on some topic branch $ git rebase --onto master @~10 ^C # quickly! $ git rebase --abort # HEAD is still detached I do not think --abort was designed to abort an uncontrolled stop like ^C in the first place. To allow that kind of recovery, you need to teach rebase to first record the state you would want to go back to before doing anything, but then what happens if the ^C comes when you are still in the middle of doing so? -- 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] format-patch: introduce format.defaultTo
Ramkumar Ramachandra artag...@gmail.com writes:
Junio C Hamano wrote:
- why is a single branch name sufficient?
It does accept a revision, so any form is allowed; but why would
anyone want that in a format.defaultTo? I'm not sure we want to impose
an artificial restriction on the configuration variable though.
I meant a single branch as opposed to depending on what branch
you are sending out, you may have to use a different upstream
starting point, and a single format.defaultTo that does not read
what your HEAD currently points at may not be enough.
Unless you set @{u} to this new configuration, in which case the
choice becomes dynamic depending on the current branch, but
- if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?
- Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?
After all, format-patch to send things out to upstream is like
asking the other side to do a rebase you would do in your
repository, so whatever git rebase that were too lazy to specify
what the fork point was when applying may be a reasonable type-saver
default. Yes, sometimes people need to rebase onto somewhere they
did not fork from, but that is why they can give explicit $upstream
and $onto to the command---I do not think it is any different for
format-patch.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug report: stash in upstream caused remote fetch to fail
Jeff King p...@peff.net writes: On Mon, Jan 06, 2014 at 08:16:31AM -0800, Junio C Hamano wrote: I was going to ask you to send your repository, but I can easily reproduce here. I guess people don't run into it because it's uncommon to fetch the whole refs/ namespace from a non-bare repo (and bare repos do not tend to have stashes). Here's a minimal reproduction recipe: git init repo cd repo echo content foo git add . git commit -m foo echo more foo git stash git init --bare sub cd sub git fetch .. 'refs/*:refs/*' It looks like we are not feeding refs/stash properly to pack-objects. I'll try to take a closer look later today. I looked at this in the past and I vaguely recall that we reject it in the for-each-ref loop with check-ref-format saying eh, that is a single-level name. At that point I stopped digging, thinking it was a feature ;-) based on your exact observation about stash vs bare/non-bare. I am fine with rejecting it with a warning, but we should not then complain that the other side did not send us the object, since we should not be asking for it at all. I also do not see us complaining about the funny ref anywhere. So there is definitely _a_ bug here. :) Oh, no question about that. I was just pointing somebody who already has volunteered to take a look in a direction I recall was where the issue was ;-) Thanks. I think somebody else mentioned recently that we do not handle malformed refs consistently. I think it was: http://article.gmane.org/gmane.comp.version-control.git/239381 which might or might not be related. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] format-patch: introduce format.defaultTo
Jeff King p...@peff.net writes:
On Mon, Jan 06, 2014 at 12:06:51PM -0800, Junio C Hamano wrote:
Unless you set @{u} to this new configuration, in which case the
choice becomes dynamic depending on the current branch, but
- if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?
- Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?
Yeah, I had similar thoughts. I personally use branch.*.merge as
forkedFrom, and it seems like we are going that way anyway with things
like git rebase and git merge defaulting to upstream. But then there
is git push -u and push.default = upstream, which treats the
upstream config as something else entirely.
So it seems like there is already some confusion, and either way we go,
thisis making it worse to some degree (I do not blame Ram, but rather he
has stumbled into a hidden sand pit that we have been building for the
past few years... :).
I wonder if it is too late to try to clarify this dual usage. It kind of
seems like the push config is this is the place I publish to. Which,
in many workflows, just so happens to be the exact same as the place you
forked from. Could we introduce a new branch.*.pushupstream variable
that falls back to branch.*.merge? Or is that just throwing more fuel on
the fire (more sand in the pit in my analogy, I guess).
I admit I haven't thought it through yet, though. And even if it does
work, it may throw a slight monkey wrench in the proposed push.default
transition.
Yeah, when I say upstream, I never mean it as where I publish.
Your upstream is where you get others' work from.
For a push to somewhere for safekeeping or other people to look at
triangular workflow, it does not make any sense to treat that I
publish there place as an upstream (hence having branch.*.remote
pointing at that publishing point). Once you stop doing that, and
instead using branch.*.remote = origin, and branch.*.merge = master,
where 'origin' is not your publishing point, @{u} will again start
making sense, I think.
And I thought that is what setting remote.pushdefault to the
publishing point repository was about.
--
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] format-patch: introduce format.defaultTo
Jeff King p...@peff.net writes:
On Mon, Jan 06, 2014 at 12:38:30PM -0800, Junio C Hamano wrote:
I wonder if it is too late to try to clarify this dual usage. It kind of
seems like the push config is this is the place I publish to. Which,
in many workflows, just so happens to be the exact same as the place you
forked from. Could we introduce a new branch.*.pushupstream variable
that falls back to branch.*.merge? Or is that just throwing more fuel on
the fire (more sand in the pit in my analogy, I guess).
I admit I haven't thought it through yet, though. And even if it does
work, it may throw a slight monkey wrench in the proposed push.default
transition.
Yeah, when I say upstream, I never mean it as where I publish.
Your upstream is where you get others' work from.
That's my thinking, as well, but it means the upstream push.default is
nonsensical. I've thought that all along, but it seems like other people
find it useful. I guess because they are in a non-triangular,
non-feature-branch setup (I suppose you could think of a central-repo
feature-branch workflow as a special form of triangular setup, where
the remote is bi-directional, but the branch names are triangular).
If we want to declare push -u and push.default=upstream as
tools for certain simple bi-directional workflows, that makes
sense. But I suspect it may cause extra confusion when people
make the jump to using a triangular workflow.
I do not think there is no want to declare involved. If I
correctly recall how push -u came about, it was merely a way to
appease those who complained that their new branch created by
running checkout -b branch origin/branch has already set up the
branch.*.remote and branch.*.merge configurations nicely for them
and allow them to immediately go ahead and start using the
centralized I merge from their 'branch' and push to that, but when
they create a new branch on their own and want to make the branch of
the same name at the origin to be the upstream, they have to futz
with the configuration. The declaration was made long time ago when
we started using @{upstream}, and there is no new extra confusion.
For a push to somewhere for safekeeping or other people to look at
triangular workflow, it does not make any sense to treat that I
publish there place as an upstream (hence having branch.*.remote
pointing at that publishing point).
You _might_ treat it the same way we treat the upstream, in some special
cases. For example, when you say git status, it is useful to see how
your topic and the upstream have progressed (i.e., do I need to pull
from upstream?). But you may _also_ want to know how your local branch
differs from its pushed counterpart (i.e., do I have unsaved commits
here that I want to push up?).
Correct; I am not saying where do I publish is never relevant. It
is just it is not something useful for format-patch to use as the
default fork-point.
So having two config options might help with that. Of course, your push
upstream (or whatever you want to call it) does not logically have one
value. You may push to several places, and would want to compare to
each.
Yes. But most likely, if you always push a single branch to
multiple places, it won't be like you push it to only one of the
places today and another one tomorrow, leaving everybody out of
sync. Unless there is a site that is temporarily down, in which
case that one may stay behind, the normal state would be that all of
them point at the same commit (that is how I publish to multiple
places anyway).
Once you stop doing that, and
instead using branch.*.remote = origin, and branch.*.merge = master,
where 'origin' is not your publishing point, @{u} will again start
making sense, I think.
And I thought that is what setting remote.pushdefault to the
publishing point repository was about.
If that were sufficient, then we would just need push.default =
current, and not upstream (nor simple). I lobbied for that during
the discussion, but people seemed to think that upstream was
better/more useful. Maybe it was just because remote.pushdefault did not
exist then.
Yeah, I think in the 2.0 world with pushdefault (i.e. triangular),
the default 'simple' turns into 'current'.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
W. Trevor King wk...@tremily.us writes: And wouldn't it make it unnecessary to have a new re-attach option if such a mode that never have to detach is used? I think so, but we currently don't have a never detached route for folks that are cloning submodules via update (instead of via 'submodule add'). Currently, new clone-updates will always leave you with a detached HEAD (unless you have branch-creation in your update !command). My patch aims to close this detached-HEAD gap, for folks we expect will be doing local development, by creating an initial branch at clone-update time. I am not a submodule expert so I may be missing some other gaps, but what your change does sounds sensible to 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 2/2] format-patch: introduce format.defaultTo
John Szakmeister j...@szakmeister.net writes:
Am I missing something? If there is something other than @{u} to
represent this latter concept, I think `git push` should default to
that instead. But, at least with my current knowledge, that doesn't
exist--without explicitly saying so--or treating @{u} as that branch.
If there's a better way to do this, I'd love to hear it!
I see Ram who worked on landing the remote.pushdefault feature is
CC'ed; this work was started in early April 2013 and your config and
workflow may not have been adjusted to it.
--
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] format-patch: introduce format.defaultTo
Ramkumar Ramachandra artag...@gmail.com writes:
Junio C Hamano wrote:
I meant a single branch as opposed to depending on what branch
you are sending out, you may have to use a different upstream
starting point, and a single format.defaultTo that does not read
what your HEAD currently points at may not be enough.
Unless you set @{u} to this new configuration, in which case the
choice becomes dynamic depending on the current branch, but
- if that is the only sane choice based on the current branch, why
not use that as the default without having to set the
configuration?
- Or if that is still insufficient, don't we need branch.*.forkedFrom
that is different from branch.*.merge, so that different branches
you want to show format-patch output can have different
reference points?
Ah, I was going for an equivalent of merge.defaultToUpstream, but I
guess branch.*.forkedFrom is a good way to go.
As I said in the different subthread, I am not convinced that you
would need the complexity of branch.*.forkedFrom. If you set your
upstream to the true upstream (not your publishing point), and
have remote.pushdefault set to 'publish', you can expect
git push
to do the right thing, and then always say
git show-branch publish/topic topic
to see where your last published branch is relative to what you
have, no? Mapping topic@{publish} to refs/remotes/publish/topic
(or when you have 'topic' checked out, mapping @{publish} to it)
is a trivial but is a separate usefulness, I would guess.
--
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
What's cooking in git.git (Jan 2014, #01; Mon, 6)
Welcome to the first issue of What's cooking report for the new year. 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] * fc/remote-helper-fixes (2013-12-26) 5 commits (merged to 'next' on 2013-12-26 at ce5f872) + remote-hg: test 'shared_path' in a moved clone (merged to 'next' on 2013-12-17 at aa4dc07) + remote-hg: add tests for special filenames + remote-hg: fix 'shared path' path + remote-helpers: add extra safety checks + remote-hg: avoid buggy strftime() * jc/push-refmap (2013-12-04) 3 commits (merged to 'next' on 2013-12-12 at 71e358f) + push: also use upstream mapping when pushing a single ref + push: use remote.$name.push as a refmap + builtin/push.c: use strbuf instead of manual allocation Make git push origin master update the same ref that would be updated by our 'master' when git push origin (no refspecs) is run while the 'master' branch is checked out, which makes git push more symmetric to git fetch and more usable for the triangular workflow. * jk/cat-file-regression-fix (2013-12-12) 2 commits (merged to 'next' on 2013-12-13 at 3713e01) + cat-file: handle --batch format with missing type/size + cat-file: pass expand_data to print_object_or_die git cat-file --batch=, an admittedly useless command, did not behave very well. * jk/name-pack-after-byte-representation (2013-12-16) 3 commits (merged to 'next' on 2013-12-17 at 0bc385c) + pack-objects doc: treat output filename as opaque (merged to 'next' on 2013-12-09 at 247b2d0) + pack-objects: name pack files after trailer hash + sha1write: make buffer const-correct (this branch is tangled with jk/pack-bitmap.) Two packfiles that contain the same set of objects have traditionally been named identically, but that made repacking a repository that is already fully packed without any cruft with a different packing parameter cumbersome. Update the convention to name the packfile after the bytestream representation of the data, not after the set of objects in it. * jk/pull-rebase-using-fork-point (2013-12-10) 2 commits (merged to 'next' on 2013-12-13 at 1862c12) + rebase: use reflog to find common base with upstream + pull: use merge-base --fork-point when appropriate * jk/rev-parse-double-dashes (2013-12-09) 2 commits (merged to 'next' on 2013-12-13 at d26bac7) + rev-parse: be more careful with munging arguments + rev-parse: correctly diagnose revision errors before -- git rev-parse revs -- paths did not implement the usual disambiguation rules the commands in the git log family used in the same way. * js/gnome-keyring (2013-12-16) 1 commit (merged to 'next' on 2013-12-17 at 422fd61) + contrib/git-credential-gnome-keyring.c: small stylistic cleanups Style fix. * tg/diff-no-index-refactor (2013-12-16) 4 commits (merged to 'next' on 2013-12-17 at 009d8d8) + diff: avoid some nesting + diff: add test for --no-index executed outside repo (merged to 'next' on 2013-12-13 at 523f7c4) + diff: don't read index when --no-index is given + diff: move no-index detection to builtin/diff.c git diff ../else/where/A ../else/where/B when ../else/where is clearly outside the repository, and git diff --no-index A B, do not have to look at the index at all, but we used to read the index unconditionally. * zk/difftool-counts (2013-12-16) 2 commits (merged to 'next' on 2013-12-16 at 0e0d235) + diff.c: fix some recent whitespace style violations (merged to 'next' on 2013-12-12 at ba35694) + difftool: display the number of files in the diff queue in the prompt Show the total number of paths and the number of paths shown so far when git difftool prompts to launch an external diff tool, which would give users some sense of progress. -- [New Topics] * ta/format-user-manual-as-an-article (2014-01-06) 1 commit (merged to 'next' on 2014-01-06 at 37858f6) + user-manual: improve html and pdf formatting Update the way the user-manual is formatted via AsciiDoc to save trees. Will merge to 'master'. * bm/merge-base-octopus-dedup (2013-12-30) 2 commits (merged to 'next' on 2014-01-06 at 355d62b) + merge-base --octopus: reduce the result from get_octopus_merge_bases() + merge-base: separate --independent codepath into its own helper git merge-base --octopus used to leave cleaning up suboptimal result to the caller, but now it does the clean-up itself. Will merge to 'master'. * jk/test-framework-updates (2014-01-02) 3 commits (merged to 'next' on 2014-01-06 at f81f373) + t: drop known breakage test + t: simplify HARNESS_ACTIVE hack + t: set TEST_OUTPUT_DIRECTORY for
Re: Bug report: stash in upstream caused remote fetch to fail
Jeff King p...@peff.net writes: Then we ask fetch_refs_via_pack to get the actual objects for us. And it checks our refs again, with this call chain: do_fetch fetch_refs transport_fetch_refs fetch_refs_via_pack fetch_pack do_fetch_pack everything_local filter_refs check_refname_format Here, the code looks like this: if (!memcmp(ref-name, refs/, 5) check_refname_format(ref-name + 5, 0)) ; /* trash */ I think this should feed ref-name, not ref-name + 5; an obvious alternative would be to use REFNAME_ALLOW_ONELEVEL; you are also right that is probably a misspelt ||; this is about protecting ourselves from creating garbage in our ref namespace, so accepting anything outside refs/ makes little sense. It's really not clear to me what the check in filter_refs was trying to do. It dates all the way back to 1baaae5 (Make maximal use of the remote refs, 2005-10-28), but there is not much explanation. I haven't dug into the list around that time to see if there's any discussion. I think the funny refs the log message talks about is about filtering we know we can reach these objects via our alternates, but these are not refs we actually have. -- 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: What's cooking in git.git (Jan 2014, #01; Mon, 6)
Francesco Pretto ceztk...@gmail.com writes: 2014/1/6 Junio C Hamano gits...@pobox.com: - git-submodule.sh: support 'checkout' as a valid update mode Need to pick up a rerolled one. I resent it, can you see it? I know. I saw it and that is why I left the note to self. The thing is, it takes a non trivial amount of time to run through a single day's integration cycle, and any reroll that comes later in a day once the cycle started may be too late for that day. Otherwise I have to discard the the result of earlier merges and tests and start over from scratch. -- 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] mv: better document side effects when moving a submodule
Jens Lehmann jens.lehm...@web.de writes: The Submodules section of the git mv documentation mentions what will happen when a submodule with a gitfile gets moved with newer git. But it doesn't talk about what happens when the user changes between commits before and after the move, which does not update the work tree like using the mv command did the first time. Explain what happens and what the user has to do manually to fix that. Also document this in a new test. Reported-by: George Papanikolaou g3orge@gmail.com Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Am 09.12.2013 18:49, schrieb Jens Lehmann: Am 09.12.2013 11:59, schrieb George Papanikolaou: Also after mv you need to run 'submodule update' and I think this should be documented somewhere. You're right, this should be mentioned in the mv man page. I'll prepare a patch for that. Does this new paragraph make it clearer? Don't we have bugs section that we can use to list the known limitations like this? Documentation/git-mv.txt | 10 ++ t/t7001-mv.sh| 21 + It also may make sense to express the test as this is what we would like to see happen eventually in the form of test_expect_failure; it is not a big deal though. 2 files changed, 31 insertions(+) diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt index b1f7988..c9e8568 100644 --- a/Documentation/git-mv.txt +++ b/Documentation/git-mv.txt @@ -52,6 +52,16 @@ core.worktree setting to make the submodule work in the new location. It also will attempt to update the submodule.name.path setting in the linkgit:gitmodules[5] file and stage that file (unless -n is used). +Please note that each time a superproject update moves a populated +submodule (e.g. when switching between commits before and after the +move) a stale submodule checkout will remain in the old location +and an empty directory will appear in the new location. To populate +the submodule again in the new location the user will have to run +git submodule update afterwards. Removing the old directory is +only safe when it uses a gitfile, as otherwise the history of the +submodule will be deleted too. Both steps will be obsolete when +recursive submodule update has been implemented. + GIT --- Part of the linkgit:git[1] suite diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 3bfdfed..e3c8c2c 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -442,4 +442,25 @@ test_expect_success 'mv --dry-run does not touch the submodule or .gitmodules' ' git diff-files --quiet -- sub .gitmodules ' +test_expect_success 'checking out a commit before submodule moved needs manual updates' ' + git mv sub sub2 + git commit -m moved sub to sub2 + git checkout -q HEAD^ 2actual + echo warning: unable to rmdir sub2: Directory not empty expected + test_i18ncmp expected actual + git status -s sub2 actual + echo ?? sub2/ expected + test_cmp expected actual + ! test -f sub/.git + test -f sub2/.git + git submodule update + test -f sub/.git + rm -rf sub2 + git diff-index --exit-code HEAD + git update-index --refresh + git diff-files --quiet -- sub .gitmodules + git status -s sub2 actual + ! test -s actual +' + test_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
Re: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
Francesco Pretto cez...@gmail.com writes: According to Documentation/gitmodules.txt, 'checkout' is a valid 'submodule.name.update' command. As you can see in the surrounding text, we call the value of submodule.*.update a mode, not a command. Also git-submodule.sh refers to it and processes it correctly. This present tense puzzles me. If it already refers to checkout and handles it correctly is there anything that needs to be done? Or did you mean it should refer to and process it but it doesn't, so make it so? Reflecting commit 'ac1fbb' to support this syntax and also validate property values during 'update' command, issuing an error if the value found is unknown. Sorry, but -ECANNOTPARSE. -- 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: Bug report: stash in upstream caused remote fetch to fail
Junio C Hamano gits...@pobox.com writes: It's really not clear to me what the check in filter_refs was trying to do. It dates all the way back to 1baaae5 (Make maximal use of the remote refs, 2005-10-28), but there is not much explanation. I haven't dug into the list around that time to see if there's any discussion. I think the funny refs the log message talks about is about filtering we know we can reach these objects via our alternates, but these are not refs we actually have. Actually, I think the above recollection of mine was completely bogus. The is there because we do allow things like HEAD (they are the funny ones) as well as things inside refs/, and the latter is the only thing we had a check-ref-format to dictate the format when the code was written. I do not mind tightening things a bit (e.g. outside refs/, only allow HEAD and nothing else). A good first step might be to enforce allow-onelevel outside refs/ (so that we can allow HEAD) and for those inside refs/ check without allow-onelevel but without skipping the prefix. It is a separate story if it makes much sense to allow fetching refs/stash or ignoring when running git clone. Operationally, I think it makes more sense to ignore refs/stash, not because it is a one-level name, but because what a stash is. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-submodule.sh: Support 'checkout' as a valid update command
Francesco Pretto cez...@gmail.com writes: Like you said, it already refers to checkout and handles it correctly. I think the use of the simple present tense here is correct: it's a fact. Feel free to advice another wording if you prefer. It is not about preference but what we want to convey to the readers. When you start the sentence with Oh, it already works correctly, the readers need to see this sentence finished: It already works, it is handled correctly, but we change the code nevertheless because ...?. Here is my attempt to fill that because ... part: Subject: git-submodule.sh: 'checkout' is a valid update mode 'checkout' is documented as one of the valid values for 'submodule.name.update' variable, and in a repository with the variable set to 'checkout', git submodule update command do update using the 'checkout' mode. However, it has been an accident that the implementation works this way; any unknown value would trigger the same codepath and update using the 'checkout' mode. Tighten the codepath and explicitly list 'checkout' as one of the known update modes, and error out when an unknown update mode is used. Also, teach the codepath that initializes the configuration variable from in-tree .gitmodules that 'checkout' is one of the valid values---the code since ac1fbbda (submodule: do not copy unknown update mode from .gitmodules, 2013-12-02) used to treat the value 'checkout' as unknown and mapped it to 'none', which made little sense. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pager: set LV=-c alongside LESS=FRSX
Jonathan Nieder jrnie...@gmail.com writes:
On systems with lv configured as the preferred pager (i.e.,
DEFAULT_PAGER=lv at build time, or PAGER=lv exported in the
environment) git commands that use color show control codes instead of
color in the pager:
$ git diff
^[[1mdiff --git a/.mailfilter b/.mailfilter^[[m
^[[1mindex aa4f0b2..17e113e 100644^[[m
^[[1m--- a/.mailfilter^[[m
^[[1m+++ b/.mailfilter^[[m
^[[36m@@ -1,11 +1,58 @@^[[m
less avoids this problem because git uses the LESS environment
variable to pass the -R option ('output ANSI color escapes in raw
form') by default. Use the LV environment variable to pass 'lv' the
-c option ('allow ANSI escape sequences for text decoration / color')
to fix it for lv, too.
Noticed when the default value for color.ui flipped to 'auto' in
v1.8.4-rc0~36^2~1 (2013-06-10).
Reported-by: Olaf Meeuwissen olaf.meeuwis...@avasys.jp
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
Olaf Meeuwissen wrote[1]:
Yes, it's called LV and documented in the lv(1) manual page. Simply
search for 'env' ;-)
Ah, thanks. How about this patch?
[1] http://bugs.debian.org/730527
Looks good; though I have to wonder two (and a half) things:
- Scripted Porcelains get LESS=-FRSX while C Porcelains get
LESS=FRSX as the default (the leading dash being the
difference), which looks somewhat inconsistent. Not a new
problem, though.
- Can we generalize this a bit so that a builder can pass a list
of var=val pairs and demote the existing LESS=FRSX to just a
canned setting of such a mechanism?
- Can such a code be reused to make this into a runtime setting,
even? Would it be worth the complexity?
Thanks.
Documentation/config.txt | 4
git-sh-setup.sh | 3 ++-
pager.c | 11 +--
perl/Git/SVN/Log.pm | 1 +
t/t7006-pager.sh | 12
5 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index a405806..ed59853 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -567,6 +567,10 @@ be passed to the shell by Git, which will translate the
final
command to `LESS=FRSX less -+S`. The environment tells the command
to set the `S` option to chop long lines but the command line
resets it to the default to fold long lines.
++
+Likewise, when the `LV` environment variable is unset, Git sets it
+to `-c`. You can override this setting by exporting `LV` with
+another value or setting `core.pager` to `lv +c`.
core.whitespace::
A comma separated list of common whitespace problems to
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 190a539..fffa3c7 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -159,7 +159,8 @@ git_pager() {
GIT_PAGER=cat
fi
: ${LESS=-FRSX}
- export LESS
+ : ${LV=-c}
+ export LESS LV
eval $GIT_PAGER '$@'
}
diff --git a/pager.c b/pager.c
index 345b0bc..0cc75a8 100644
--- a/pager.c
+++ b/pager.c
@@ -80,8 +80,15 @@ void setup_pager(void)
pager_process.use_shell = 1;
pager_process.argv = pager_argv;
pager_process.in = -1;
- if (!getenv(LESS)) {
- static const char *env[] = { LESS=FRSX, NULL };
+ if (!getenv(LESS) || !getenv(LV)) {
+ static const char *env[3];
+ int i = 0;
+
+ if (!getenv(LESS))
+ env[i++] = LESS=FRSX;
+ if (!getenv(LV))
+ env[i++] = LV=-c;
+ env[i] = NULL;
pager_process.env = env;
}
if (start_command(pager_process))
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index 3f8350a..34f2869 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm
@@ -117,6 +117,7 @@ sub run_pager {
}
open STDIN, '', $rfd or fatal Can't redirect stdin: $!;
$ENV{LESS} ||= 'FRSX';
+ $ENV{LV} ||= '-c';
exec $pager or fatal Can't run pager: $! ($pager);
}
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index ff25908..7fe3367 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -37,6 +37,18 @@ test_expect_failure TTY 'pager runs from subdir' '
test_cmp expected actual
'
+test_expect_success TTY 'LESS and LV envvars are set for pagination' '
+ (
+ sane_unset LESS LV
+ PAGER=env pager-env.out
+ export PAGER
+
+ test_terminal git log
+ )
+ grep ^LESS= pager-env.out
+ grep ^LV= pager-env.out
+'
+
test_expect_success TTY 'some commands do not use a pager' '
rm -f paginated.out
test_terminal git rev-list HEAD
--
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] sha1_name: don't resolve refs when core.warnambiguousrefs is false
Brodie Rao bro...@sf.io writes:
This change ensures get_sha1_basic() doesn't try to resolve full hashes
as refs when ambiguous ref warnings are disabled.
This provides a substantial performance improvement when passing many
hashes to a command (like git rev-list --stdin) when
core.warnambiguousrefs is false. The check incurs 6 stat()s for every
hash supplied, which can be costly over NFS.
---
Needs sign-off. The patch looks good.
Thanks.
sha1_name.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index e9c2999..10bd007 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -451,9 +451,9 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
int at, reflog_len, nth_prior = 0;
if (len == 40 !get_sha1_hex(str, sha1)) {
- if (warn_on_object_refname_ambiguity) {
+ if (warn_ambiguous_refs warn_on_object_refname_ambiguity) {
refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
- if (refs_found 0 warn_ambiguous_refs) {
+ if (refs_found 0) {
warning(warn_msg, len, str);
if (advice_object_name_warning)
fprintf(stderr, %s\n,
_(object_name_msg));
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/17] remove_dir_recurse(): handle disappearing files and directories
Michael Haggerty mhag...@alum.mit.edu writes: I'm not sure I understand your point. Please note that the REMOVE_DIR_KEEP_TOPLEVEL bit is cleared from flags before this function recurses. So in recursive invocations, keep_toplevel will always be false, and the rmdir(path-buf) codepath will be permitted. Does that answer your question? Yes; 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 v2 03/17] safe_create_leading_directories(): add explicit slash pointer
Michael Haggerty mhag...@alum.mit.edu writes: I agree that it would be reasonable to use is_dir_sep() in the implementation of this function, at least unless/until somebody does the work to figure out whether callers should really only be passing it forward-slash-normalized paths. Please be careful, though, because I don't think this function is capable of handling arbitrary Windows paths, like for example //host/path format, either before or after my change. Let me know if you would like me to merge or rebase the is_dir_sep() changes into this patch series. I'd want SSchuberth and windows folks to be at least aware of this series and preferrably see that they offer inputs to this series, making their is_dir_sep() change just one step in this series. That way I have one less series to worry about ;-) 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] sha1_name: don't resolve refs when core.warnambiguousrefs is false
Jeff King p...@peff.net writes: Alternatively, I guess cat-file --batch could just turn off warn_ambiguous_refs itself. Sounds like a sensible way to go, perhaps on top of this change? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
Francesco Pretto cez...@gmail.com writes: 2014/1/7 Francesco Pretto cez...@gmail.com: To not break the existing behavior what it's really needed here, IMO, is a submodule.name.attached property that says two things: - at the first clone on git submodule update stay attached to submodule.name.branch; - implies --remote, as it's the only thing that makes sense when the submodules are attached. Unless you decide to go with the proposed approach of Trevor, where submodule.name.branch set means attached (if it's not changed: this thread is quite hard to follow...). To this end, Junio could sync with more long-timers (Heiko?) submodule users/devs to understand if this breaks too much or not. It is not immediately obvious to me why anybody who specifies the submodule.*.branch variable to say I want _that_ branch not to want to be on that branch but in a detached state, so from that perspective, submodule.*.attach feels superfluous. But I'd mostly defer the design discussion to Heiko (and Jens), WTK and you. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
W. Trevor King wk...@tremily.us writes:
From: W. Trevor King wk...@tremily.us
The previous code only checked out the requested branch in cmd_add.
This commit moves the branch-checkout logic into module_clone, where
it can be shared by cmd_add and cmd_update. I also update the initial
checkout command to use 'rebase' to preserve branches setup during
module_clone.
I want to see the log message explain the motivation behind it
(i.e. instead of stopping after saying We used to do X, now we do
Y, but also explain why we consider that Y is better than X). Here
is my attempt.
submodule: respect requested branch on all clones
The previous code only checked out the requested branch in cmd_add
but not in cmd_update; this left the user on a detached HEAD after
an update initially cloned, and subsequent updates using rebase or
merge mode will kept the HEAD detached, unless the user moved to the
desired branch himself.
Move the branch-checkout logic into module_clone, where it can be
shared by cmd_add and cmd_update. Also update the initial checkout
command to use 'rebase' to preserve branches setup during
module_clone. This way, unless the user explicitly asks to work on
a detached HEAD, subsequent updates all happen on the specified
branch, which matches the end-user expectation much better.
Signed-off-by: W. Trevor King wk...@tremily.us
Signed-off-by: Junio C Hamano gits...@pobox.com
Please correct me if I misunderstood the intention.
Having writing all the above and then looking at the patch again, it
is not immediately obvious to me where you use rebase when doing
the initial checkout, though.
The current Documentation/git-submodule.txt has:
update::
Update the registered submodules, i.e. clone missing submodules
and checkout the commit specified in the index of the containing
repository. This will make the submodules HEAD be detached unless
`--rebase` or `--merge` is specified or the key
`submodule.$name.update` is set to `rebase`, `merge` or `none`.
Side note but doesn't Francesco's 'checkout' is a valid update mode
need to update this part of the documentation as well?
git-submodule.sh | 34 --
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..167d4fa 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -253,6 +253,7 @@ module_clone()
url=$3
reference=$4
depth=$5
+ branch=$6
quiet=
if test -n $GIT_QUIET
then
@@ -306,7 +307,15 @@ module_clone()
echo gitdir: $rel/$a $sm_path/.git
rel=$(echo $a | sed -e 's|[^/][^/]*|..|g')
- (clear_local_git_env; cd $sm_path GIT_WORK_TREE=. git config
core.worktree $rel/$b)
+ (
+ clear_local_git_env
+ cd $sm_path
+ GIT_WORK_TREE=. git config core.worktree $rel/$b
+ case $branch in
+ '') git checkout -f -q ;;
+ ?*) git checkout -f -q -B $branch origin/$branch ;;
+ esac
+ ) || die $(eval_gettext Unable to setup cloned submodule
'\$sm_path')
}
isnumber()
@@ -469,16 +478,7 @@ Use -f if you really want to add it. 2
echo $(eval_gettext Reactivating local git
directory for submodule '\$sm_name'.)
fi
fi
- module_clone $sm_path $sm_name $realrepo $reference
$depth || exit
- (
- clear_local_git_env
- cd $sm_path
- # ash fails to wordsplit ${branch:+-b $branch...}
- case $branch in
- '') git checkout -f -q ;;
- ?*) git checkout -f -q -B $branch origin/$branch ;;
- esac
- ) || die $(eval_gettext Unable to checkout submodule
'\$sm_path')
+ module_clone $sm_path $sm_name $realrepo $reference
$depth $branch || exit
fi
git config submodule.$sm_name.url $realrepo
@@ -787,7 +787,8 @@ cmd_update()
fi
name=$(module_name $sm_path) || exit
url=$(git config submodule.$name.url)
- branch=$(get_submodule_config $name branch master)
+ config_branch=$(get_submodule_config $name branch)
+ branch=${config_branch:-master}
if ! test -z $update
then
update_module=$update
@@ -815,7 +816,7 @@ Maybe you want to use 'update --init'?)
if ! test -d $sm_path/.git -o -f $sm_path/.git
then
- module_clone $sm_path $name $url $reference
$depth || exit
+ module_clone $sm_path $name $url $reference
$depth $config_branch || exit
cloned_modules=$cloned_modules;$name
subsha1
Re: [PATCH v3] stash: handle specifying stashes with spaces
Øystein Walle oys...@gmail.com writes:
When trying to pop/apply a stash specified with an argument containing
spaces git-stash will throw an error:
$ git stash pop 'stash@{two hours ago}'
Too many revisions specified: stash@{two hours ago}
This happens because word splitting is used to count non-option
arguments. Make use of rev-parse's --sq option to quote the arguments
for us to ensure a correct count. Add quotes where necessary.
Also add a test that verifies correct behaviour.
Helped-by: Thomas Rast t...@thomasrast.ch
Signed-off-by: Øystein Walle oys...@gmail.com
---
v3 uses the same eval/--sq technique as v2, suggested by Thomas Rast.
This is basically a resend except that I added a missing '' in the
test that Eric Sunshine noticed when reading v1.
The change looks good.
An unrelated tangent, but here is a tip-of-the-day for the
approximate parser. You could have just said
git stash pop stash@{2.hours}
and it would have been interpreted just the same ;-)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index debda7a..7eb011c 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -673,4 +673,15 @@ test_expect_success 'store updates stash ref and reflog'
'
grep quux bazzy
'
+test_expect_success 'handle stash specification with spaces' '
+ git stash clear
+ echo pig file
Style: no SP between redirection operator and its target, i.e.
echo pig file
+ git stash
+ test_tick
+ echo cow file
+ git stash
+ git stash apply stash@{Thu Apr 7 15:17:13 2005 -0700}
This is brittle. If new tests are added before this, the test_tick
will give you different timestamp and this test will start failing.
Perhaps grab the timestamp out of the stash that was created, e.g.
...
test_tick
git stash
stamp=$(git log -g --format=%cd -1 refs/stash)
test_tick
echo cow file
git stash
git stash apply stash@{$stamp}
or something?
+ grep pig file
+'
+
test_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
Re: [PATCH 2/2] Introduce git submodule attached update
Francesco Pretto cez...@gmail.com writes: - In which situations does the developer or maintainer switch between your attached/detached mode? The developer/maintainer does so optionally and voluntarily and it effects only its private working tree. This does not answer my question. I would like to find out the reason why one would do the switch. The developer does it voluntarily, at his responsibility, because he may decide to partecipate more actively to the development of the submodule and still want to use a simple git submodule update to updates his submodules, overriding its configuration as it can be done for other properties like, for example, branch. It is still unclear to me why we need attached/detached mode for that. The developer may want to do an exploratory development, whose result is unknown to deserve to be committed on the specified branch at the beginning, and choose to build on a detached HEAD, which is a perfectly normal thing to do. But the standard way to do so, whether the developer is working in the top-level superproject or in a submodule, would be to just do: cd $there git checkout HEAD^0 or use whatever commit the state to be detached is at instead of HEAD in the above example, no? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Introduce git submodule attached update
Francesco Pretto cez...@gmail.com writes: My bottom line: - For what I understand, detached HEAD it's a way to say hey, you have to stay on this commit. Also don't even think you can push to the upstream branch. This sometimes can't be spurious, as in the use case I wrote above: access control on the remote repositories should be enough. I think maintainers should have the option to make developers to clone a repository starting with an attached HEAD on the branch suggested in submodule.$name.branch; - git submodule update is missing a property to do automatically --remote. I think in the use case I wrote it's really handy to have a git submodule update to act like this. The short version I read in the message is that your workflow, in which partipants want to work on a branch, gets frustrating with the current system only because the default update/initial cloning detaches HEAD and will stay in that state until the user gets out of the detached state manually. Once that initial detachment is fixed, there is no more major issue, as update will stay on that branch. Am I reading you correctly? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
W. Trevor King wk...@tremily.us writes: On Tue, Jan 07, 2014 at 10:15:25AM -0800, Junio C Hamano wrote: submodule: respect requested branch on all clones The previous code only checked out the requested branch in cmd_add but not in cmd_update; this left the user on a detached HEAD after an update initially cloned, and subsequent updates using rebase or merge mode will kept the HEAD detached, unless the user moved to the desired branch himself. Move the branch-checkout logic into module_clone, where it can be shared by cmd_add and cmd_update. Also update the initial checkout command to use 'rebase' to preserve branches setup during module_clone. This way, unless the user explicitly asks to work on a detached HEAD, subsequent updates all happen on the specified branch, which matches the end-user expectation much better. This looks reasonable to me, but there are still changes I'd like to make for a v3 (e.g. using submodule.name.update to trigger local branch checkout). However, I'm currently leaning towards a new 'git submodule checkout' command with explicit preferred local submodule branches (see [1]). Maybe this should all wait until Jens rolls out his update implementation [2]? Sounds good. I'll backburner this one, then. Having writing all the above and then looking at the patch again, it is not immediately obvious to me where you use rebase when doing the initial checkout, though. It's used to shift the local branch reference from from the (arbitrary) cloned remote branch tip to the explicit submodule $sha1. The objective is not what I was questioning. In the patch I see git checkout -f -q -B $branch origin/$branch used in the module_clone (which I think makes sense), and then cmd_update uses git reset --hard -q to make sure that the working tree matches the commit $sha1 obtained from the superproject's gitlink (which may make $branch diverged from origin/$branch, but still I think it makes sense). But there is no 'rebase' I can see in the codepath, which was what I was puzzled about. -- 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] sha1_name: don't resolve refs when core.warnambiguousrefs is false
Jeff King p...@peff.net writes: On Tue, Jan 07, 2014 at 09:51:07AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: Alternatively, I guess cat-file --batch could just turn off warn_ambiguous_refs itself. Sounds like a sensible way to go, perhaps on top of this change? The downside is that we would not warn about ambiguous refs anymore, even if the user was expecting it to. I don't know if that matters much. That is true already with or without Brodie's change, isn't it? With warn_on_object_refname_ambiguity, cat-file --batch makes us ignore core.warnambigousrefs setting. If we redo 25fba78d (cat-file: disable object/refname ambiguity check for batch mode, 2013-07-12) to unconditionally disable warn_ambiguous_refs in cat-file --batch and get rid of warn_on_object_refname_ambiguity, the end result would be the same, no? I kind of feel in the --batch situation that it is somewhat useless (I wonder if rev-list --stdin should turn it off, too). I think doing the same as cat-file --batch in rev-list --stdin makes sense. Both interfaces are designed to grok extended SHA-1s, and full 40-hex object names could be ambiguous and we are missing the warning for them. Or are you wondering if we should revert 25fba78d, apply Brodie's change to skip the ref resolution whose result is never used, and tell people who want to use cat-file --batch (or rev-list --stdin) to disable the ambiguity warning themselves? -- 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] drop unnecessary copying in credential_ask_one
Jeff King p...@peff.net writes: On Thu, Jan 02, 2014 at 11:08:51AM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: ... But the test suite, of course, always uses askpass because it cannot rely on accessing a terminal (we'd have to do some magic with lib-terminal, I think). So it doesn't detect the problem in your patch, but I wonder if it is worth applying the patch below anyway, as it makes the test suite slightly more robust. Sounds like a good first step in the right direction. Thanks. I took a brief look at adding real terminal tests for the credential code using our test-terminal/lib-terminal.sh setup. Unfortunately, it falls short of what we need. test-terminal only handles stdout and stderr streams as fake terminals. We could pretty easily add stdin for input, as it uses fork() to work asynchronously. But the credential code does not actually read from stdin. It opens and reads from /dev/tty explicitly. So I think we'd have to actually fake setting up a controlling terminal. And that means magic with setsid() and ioctl(TIOCSCTTY), which in turn sounds like a portability headache. I wonder if expect has already solved that for us. So it's definitely possible under Linux, and probably under most Unixes. But I'm not sure it's worth the effort, given that review already caught the potential bug here. Another option would be to instrument git_terminal_prompt with a mock-terminal interface (say, reading from a file specified in an environment variable). But I really hate polluting the code with test cruft, and it would not actually be testing an interesting segment of the code, anyway. Agreed. -- 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] sha1_name: don't resolve refs when core.warnambiguousrefs is false
Jeff King p...@peff.net writes: On Tue, Jan 07, 2014 at 11:38:15AM -0800, Junio C Hamano wrote: Alternatively, I guess cat-file --batch could just turn off warn_ambiguous_refs itself. Sounds like a sensible way to go, perhaps on top of this change? The downside is that we would not warn about ambiguous refs anymore, even if the user was expecting it to. I don't know if that matters much. That is true already with or without Brodie's change, isn't it? With warn_on_object_refname_ambiguity, cat-file --batch makes us ignore core.warnambigousrefs setting. If we redo 25fba78d (cat-file: disable object/refname ambiguity check for batch mode, 2013-07-12) to unconditionally disable warn_ambiguous_refs in cat-file --batch and get rid of warn_on_object_refname_ambiguity, the end result would be the same, no? No, I don't think the end effect is the same (or maybe we are not talking about the same thing. :) ). There are two ambiguity situations: 1. Ambiguous non-fully-qualified refs (e.g., same tag and head name). 2. 40-hex sha1 object names which might also be unqualified ref names. Prior to 25ffba78d, cat-file checked both (like all the rest of git). But checking (2) is very expensive,... Ahh, of course. Sorry for forgetting about 1. The two options I was musing over earlier today were (all on top of Brodie's patch): a. Revert 25ffba78d. With Brodie's patch, core.warnAmbiguousRefs disables _both_ warnings. So we default to safe-but-slow, but people who care about performance can turn off ambiguity warnings. The downside is that you have to know to turn it off manually (and it's a global config flag, so you end up turning it off _everywhere_, not just in big queries where it matters). Or git -c core.warnambiguousrefs=false cat-file --batch, but I think a more important point is that it is no longer automatic for known-to-be-heavy operations, and I agree with you that it is a downside. b. Revert 25ffba78d, but then on top of it just turn off warn_ambiguous_refs unconditionally in cat-file --batch-check. The downside is that we drop the safety from (1). The upside is that the code is a little simpler, as we drop the extra flag. And obviously: c. Just leave it at Brodie's patch with nothing else on top. My thinking in favor of (b) was basically does anybody actually care about ambiguous refs in this situation anyway?. If they do, then I think (c) is my preferred choice. OK. I agree with that line of thinking. Let's take it one step at a time, i.e. do c. and also use warn_on_object_refname_ambiguity in rev-list --stdin first and leave the simplification (i.e. b.) for later. I kind of feel in the --batch situation that it is somewhat useless (I wonder if rev-list --stdin should turn it off, too). I think doing the same as cat-file --batch in rev-list --stdin makes sense. Both interfaces are designed to grok extended SHA-1s, and full 40-hex object names could be ambiguous and we are missing the warning for them. I'm not sure I understand what you are saying. We _do_ have the warning for rev-list --stdin currently. We do _not_ have the warning for cat-file --batch, since my 25ffba78d. What I wanted to say was that we would be discarding the safety for rev-list --stdin with the same argument as we did for cat-file --batch. If the argument for the earlier cat-file --batch were this interface only takes raw 40-hex object names, then the situation would have been different, but that is not the case. I was wondering if rev-list should go the same way as 25ffba78d, for efficiency reasons (e.g., think piping to rev-list --no-walk --stdin). Yes, and I was trying to agree with that, but apparently I failed ;-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Ramkumar Ramachandra artag...@gmail.com writes:
A very common workflow for preparing patches involves working off a
topic branch and generating patches against 'master' to send off to the
maintainer. However, a plain
$ git format-patch -o outgoing
is a no-op on a topic branch, and the user has to remember to specify
'master' explicitly everytime. This problem is not unique to
format-patch; even a
$ git rebase -i
is a no-op because the branch to rebase against isn't specified.
To tackle this problem, introduce branch.*.forkedFrom which can specify
the parent branch of a topic branch. Future patches will build
functionality around this new configuration variable.
I do not mind allowing laziness by defaulting to something, but I am
not enthused by an approach that adds the new variable whose value
is questionable. The description does not justify at all why
@{upstream} is not a good default (unless the workflow is screwed up
and @{upstream} is set to point at somewhere that is _not_ a true
upstream, that is).
Cc: Jeff King p...@peff.net
Cc: Junio C Hamano gis...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Since -M, -C, -D are left in the argc, checking argc 2 isn't
sufficient.
I wanted to get an early reaction before wiring up checkout and
rebase.
But I wanted to discuss the overall idea of the patch.
builtin/log.c | 21 +
t/t4014-format-patch.sh | 20
2 files changed, 41 insertions(+)
diff --git a/builtin/log.c b/builtin/log.c
index b97373d..525e696 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -674,6 +674,7 @@ static int thread;
static int do_signoff;
static const char *signature = git_version_string;
static int config_cover_letter;
+static const char *config_base_branch;
enum {
COVER_UNSET,
@@ -750,6 +751,22 @@ static int git_format_config(const char *var, const char
*value, void *cb)
config_cover_letter = git_config_bool(var, value) ? COVER_ON :
COVER_OFF;
return 0;
}
+ if (starts_with(var, branch.)) {
+ const char *name = var + 7;
+ const char *subkey = strrchr(name, '.');
+ struct strbuf buf = STRBUF_INIT;
+
+ if (!subkey)
+ return 0;
+ strbuf_add(buf, name, subkey - name);
+ if (branch_get(buf.buf) != branch_get(NULL))
+ return 0;
+ strbuf_release(buf);
+ if (!strcmp(subkey, .forkedfrom)) {
+ if (git_config_string(config_base_branch, var, value))
+ return -1;
+ }
+ }
return git_log_config(var, value, cb);
}
@@ -1324,6 +1341,10 @@ int cmd_format_patch(int argc, const char **argv,
const char *prefix)
die (_(--subject-prefix and -k are mutually exclusive.));
rev.preserve_subject = keep_subject;
+ if (argc 2 config_base_branch) {
+ argv[1] = config_base_branch;
+ argc++;
+ }
argc = setup_revisions(argc, argv, rev, s_r_opt);
if (argc 1)
die (_(unrecognized argument: %s), argv[1]);
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 73194b2..2ea94af 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1370,4 +1370,24 @@ test_expect_success 'cover letter auto user override' '
test_line_count = 2 list
'
+test_expect_success 'branch.*.forkedFrom matches' '
+ mkdir -p tmp
+ test_when_finished rm -rf tmp;
+ git config --unset branch.rebuild-1.forkedFrom
+
+ git config branch.rebuild-1.forkedFrom master
+ git format-patch -o tmp list
+ test_line_count = 2 list
+'
+
+test_expect_success 'branch.*.forkedFrom does not match' '
+ mkdir -p tmp
+ test_when_finished rm -rf tmp;
+ git config --unset branch.foo.forkedFrom
+
+ git config branch.foo.forkedFrom master
+ git format-patch -o tmp list
+ test_line_count = 0 list
+'
+
test_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
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Ramkumar Ramachandra artag...@gmail.com writes:
Junio C Hamano wrote:
I do not mind allowing laziness by defaulting to something, but I am
not enthused by an approach that adds the new variable whose value
is questionable. The description does not justify at all why
@{upstream} is not a good default (unless the workflow is screwed up
and @{upstream} is set to point at somewhere that is _not_ a true
upstream, that is).
Did you find the explanation I gave in
http://article.gmane.org/gmane.comp.version-control.git/240077
reasonable?
No.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Jeff King p...@peff.net writes:
The point was that the meaning of @{upstream} (and branch.*.merge)
is _already_ forked-from, and push -u and push.default=upstream
are the odd men out. If we are going to add an option to distinguish the
two branch relationships:
1. Where you forked from
2. Where you push to
we should leave @{upstream} as (1), and add a new option to represent
(2). Not the other way around.
That matches my feeling as well.
I am not sure if push -u is truly odd man out, though. It was an
invention back in the you fetch from and push to the same place and
there is no other workflow support days, and in that context, the
upstream meant just that: the place you fetch from, which happens
to be the same as where you are pushing to right now. If push -u
suddenly stopped setting the configuration to merge back from where
it is pushing, that would regress for centralized folks, so I am not
sure how it could be extended to also support triangular folks, but
I do think @{upstream} should mean this is where I sync with to
stay abreast with others.
--
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] format-patch: introduce format.defaultTo
Jeff King p...@peff.net writes:
Yes, pushbranch is probably a better name for what I am referring to.
I agree that pushremote is probably enough for sane cases. I seem to
recall that people advocating the upstream push-default thought that
branch name mapping was a useful feature, but I might be
mis-remembering. I will let those people speak up for the feature if
they see fit; it seems somewhat crazy to me.
I think branch mapping you recall are for those who want to push
their 'topic' to 'review/topic' or something like that. With Git
post 7cdebd8a (Merge branch 'jc/push-refmap', 2013-12-27), I think
remote.*.push can be used to implement that, by the way.
Frankly, I don't use full triangular workflows myself mainly because
my prompt is compromised: when I have a branch.*.remote different from
branch.*.pushremote, I'd like to see where my branch is with respect
to @{u} and @{publish} (not yet invented);
Yes, as two separate relationships, you would theoretically want to be
able to see them separately (or simultaneously side by side). Whether
exposing that in the prompt is too clunky, I don't know (I don't even
show ahead/behind in my prompt, but rather prefer to query it when I
care; I have a separate script that queries the ahead/behind against my
publishing point, but it would be nice if git handled this itself).
Same here. I do not bother a/b in prompt and comparison with
publishing point is done with a custom script. It would be nice to
have it natively, and @{publish} would be a good first step to do
so.
--
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] format-patch: introduce format.defaultTo
Jeff King p...@peff.net writes: I think that is sensible, and only heightens my sense of the upstream push.default as useless. :) Yes, it only is good for centralized world (it was designed back in the centralized days after all, wasn't it?). -- 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] format-patch: introduce format.defaultTo
Jeff King p...@peff.net writes: And even in a centralized workflow, I see upstream creating problems. E.g., you fork a feature branch in the centralized repo; it should not get pushed straight back to master! And that is why we invented simple, to prevent such things. Oh, don't get me wrong. I personally wouldn't imagine forking 'topic' from the shared 'master', call the result perfect and push it directly back to the shared 'master'. But the 'upstream' setting was added exactly to support that. In such a case, I would have 'master' that is forked from the shared 'master', 'topic' that is forked from my 'master', and pushing back would be a two-step process, first updating my 'master' in sync with the shared 'master', merging 'topic' into it to make sure the result is sane and then push it back to the shared 'master'. And in that set-up, 'upstream' would work fine as the upstream of my 'master' is the shared 'master', even though 'current' or even 'matching' would work just as well. So in that sense, I do not see 'upstream' as so broken as you seem to be saying. One gap in that line of thought might be that I am sane enough not to attempt git push while I am on my 'topic', 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 v3] stash: handle specifying stashes with spaces
Thomas Rast t...@thomasrast.ch writes:
Junio C Hamano gits...@pobox.com writes:
Øystein Walle oys...@gmail.com writes:
+ git stash
+ test_tick
+ echo cow file
+ git stash
+ git stash apply stash@{Thu Apr 7 15:17:13 2005 -0700}
This is brittle. If new tests are added before this, the test_tick
will give you different timestamp and this test will start failing.
Perhaps grab the timestamp out of the stash that was created [...]
Hmm, now that I stare at this, why not simply use something like
git stash apply stash@{ 0 }
It seems to refer to the same as stash@{0} as one would expect, while
still triggering the bug with unpatched git-stash.
Yeah, that is fine as well. For the record, here is what I
tentatively queued.
-- 8 --
From: Øystein Walle oys...@gmail.com
Date: Tue, 7 Jan 2014 09:22:15 +0100
Subject: [PATCH] stash: handle specifying stashes with $IFS
When trying to pop/apply a stash specified with an argument
containing IFS whitespace, git-stash will throw an error:
$ git stash pop 'stash@{two hours ago}'
Too many revisions specified: stash@{two hours ago}
This happens because word splitting is used to count non-option
arguments. Make use of rev-parse's --sq option to quote the arguments
for us to ensure a correct count. Add quotes where necessary.
Also add a test that verifies correct behaviour.
Helped-by: Thomas Rast t...@thomasrast.ch
Signed-off-by: Øystein Walle oys...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
git-stash.sh | 14 +++---
t/t3903-stash.sh | 12
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index 1e541a2..f0a94ab 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -358,7 +358,7 @@ parse_flags_and_rev()
i_tree=
u_tree=
- REV=$(git rev-parse --no-flags --symbolic $@) || exit 1
+ REV=$(git rev-parse --no-flags --symbolic --sq $@) || exit 1
FLAGS=
for opt
@@ -376,7 +376,7 @@ parse_flags_and_rev()
esac
done
- set -- $REV
+ eval set -- $REV
case $# in
0)
@@ -391,13 +391,13 @@ parse_flags_and_rev()
;;
esac
- REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
+ REV=$(git rev-parse --quiet --symbolic --verify $1 2/dev/null) || {
reference=$1
die $(eval_gettext \$reference is not valid reference)
}
- i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
- set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 2/dev/null)
+ i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null)
+ set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2:
2/dev/null)
s=$1
w_commit=$1
b_commit=$2
@@ -408,8 +408,8 @@ parse_flags_and_rev()
test $ref_stash = $(git rev-parse --symbolic-full-name ${REV%@*})
IS_STASH_REF=t
- u_commit=$(git rev-parse --quiet --verify $REV^3 2/dev/null)
- u_tree=$(git rev-parse $REV^3: 2/dev/null)
+ u_commit=$(git rev-parse --quiet --verify $REV^3 2/dev/null)
+ u_tree=$(git rev-parse $REV^3: 2/dev/null)
}
is_stash_like()
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index debda7a..5b79b21 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -673,4 +673,16 @@ test_expect_success 'store updates stash ref and reflog' '
grep quux bazzy
'
+test_expect_success 'handle stash specification with spaces' '
+ git stash clear
+ echo pig file
+ git stash
+ stamp=$(git log -g --format=%cd -1 refs/stash)
+ test_tick
+ echo cow file
+ git stash
+ git stash apply stash@{$stamp}
+ grep pig file
+'
+
test_done
--
1.8.5.2-419-g5ca323a
--
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
