[PATCH v3] stash: handle specifying stashes with spaces
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.
git-stash.sh | 14 +++---
t/t3903-stash.sh | 11 +++
2 files changed, 18 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..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
+ git stash
+ test_tick
+ echo cow file
+ git stash
+ git stash apply stash@{Thu Apr 7 15:17:13 2005 -0700}
+ grep pig file
+'
+
test_done
--
1.8.5
--
To unsubscribe from this list: send the line 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
On 01/06/2014 07:32 PM, Junio C Hamano wrote: 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. 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. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 13/17] remove_dir_recurse(): handle disappearing files and directories
On 01/06/2014 07:18 PM, Junio C Hamano wrote:
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.
Indeed, this is a design choice that I should have explained. I
implemented it this way to keep the behavior the same as the old code in
this situation, and because I thought that if the caller explicitly asks
for the toplevel path to be kept, and that path doesn't even exist at
the entrance to the function, then something is screwy and it is better
to return an error than to keep going.
It would even be possible to argue that if keep_toplevel is set but path
is missing, then this function should call
safe_create_leading_directories(path).
Changing this behavior would require an audit to see which behavior
would be most useful to the callers. I think that is out of the scope
of this patch series.
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.
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?
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/17] lock_ref_sha1_basic(): on SCLD_VANISHED, retry
On 01/06/2014 06:54 PM, Junio C Hamano wrote:
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
;-)).
remove_dir_recurse() only tries deleting directories once (I haven't
changed that). And from a broader perspective, it would be pretty silly
for any tidy-up-directories function to try deleting things more than
once. So I don't think it is a problem. But even in the worst case,
this function only tries three times before giving up, so it shouldn't
be a disaster.
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.
I just renamed the variable to attempts_remaining. (I thought I was
following your suggestion, but now I see that I put the words in the
opposite order; oh well, I think it's fine either way.)
Thanks for your review! I will wait a day or so for any additional
comments, and then send a v3.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 17/17] rename_tmp_log(): on SCLD_VANISHED, retry
On 01/06/2014 07:21 PM, Junio C Hamano wrote: 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 ;-) Good catch. The increment 3 - 4 was because the first call to rename() is optimistic, and can fail once even if there is no race. I will change the commit message of 16/17 to explain this point, and of 17/17 to match reality. The series mostly looked sane from a cursory read. Will re-queue. Thanks. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] completion: complete format.coverLetter
Ramkumar Ramachandra wrote: contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) Junio: Please push this part via 'maint' independently. -- To unsubscribe from this list: send the line 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
On Thu, Jan 2, 2014 at 9:46 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Sebastian, On Thu, 2 Jan 2014, Sebastian Schuberth wrote: On 02.01.2014 18:33, Johannes Schindelin wrote: -- snip -- 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'. -- snap -- While I'd be fine with this, I do not think we really need it. I also would have been fine with your commit message. But I knew Junio wouldn't be. As you say, is_dir_sep() has been introduced a long time ago, so people should be aware of it, and it should also be immediately clear from the diff why using it is better than hard-coding '/'. That said, I see any further explanations on top of the commit message title is an added bonus, and as just a bonus a link to a pull request should be fine. You don't need to understand or appreciate the concept of pull requests in order to follow the link and read the text in there. Well, you and I both know how easy GitHub's pull request made things for us as well as for contributors. I really cannot thank Erik enough for bullying me into using and accepting them. Huh? I don't think you refer to me, because I really dislike them (and I always have IIRC). -- To unsubscribe from this list: send the line 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 2/2] format-patch: introduce format.defaultTo
John Szakmeister wrote:
I guess it's not a good idea to
set 'push.default' to 'upstream' in this triangle workflow then,
otherwise the branch name being pushed to will be 'branch.*.merge'.
Is that correct? I just want to make sure I understand what's
happening here.
push.default = upstream does not support triangular workflows. See
builtin/push.c:setup_push_upstream().
Given this new found knowledge, I'm not sure it makes sense for `git
status` to show me the status against the upstream vs. the publish
location. The latter makes a little more sense to me, though I see
the usefulness of either one.
Currently, status information is only against @{u}; we haven't
invented a @{publish} yet.
--
To unsubscribe from this list: send the line 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
On Mon, Jan 06, 2014 at 07:35:04PM -0800, Brodie Rao wrote: On Mon, Jan 6, 2014 at 7:32 PM, Brodie Rao bro...@sf.io wrote: 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. Forgot to add: Signed-off-by: Brodie Rao bro...@sf.io Looks good to me. I wonder if I should have simply gone this route instead of adding warn_on_object_refname_ambiguity, and then people who want cat-file --batch to be fast could just turn off core.warnAmbiguousRefs. I wanted it to happen automatically, though. Alternatively, I guess cat-file --batch could just turn off warn_ambiguous_refs itself. -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] 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] [RFC] Making use of bitmaps for thin objects
On Mon, Jan 06, 2014 at 10:14:30PM +, Ben Maurer wrote: It looks like for my repo the size win wasn't as big (~10%). Is it possible that with the kernel test you got extremely lucky and there was some huge binary blob that thin packing turned into a tiny delta? I don't think so. When I look at the reused-delta numbers, I see that we reused ~3000 extra deltas (and the compressing progress meter drops by the same amount. If I do a full clone (or just index-pack the result), it claims ~3000 thin objects completed with local objects (versus 0 in the normal case). So I think we really are getting a lot of little savings adding up, which makes sense. If there were thousands of changed files, a non-thin pack has to have at least _one_ full version of each file. With thin packs, we might literally have only deltas. It was a 7-week period, which might make more difference. I'm going to run some experiments with different time periods to see if that changes anything. It might also be the repo contents. I'm going to try my experiments on a few different repositories. It may be that either the kernel or your repo is unusual in some way. Or maybe I was just lucky. :) When you get a chance, it'd be handy if you could push an updated version of your change out to your public github repo. I'd like to see if folks here are interested in testing this more, and it'd be good to make sure we're testing the diff that is targeted for upstream. I've pushed it to: git://github.com/peff/git.git jk/bitmap-reuse-delta I'll continue to rebase it forward as time goes on (until a cleaned-up version gets merged upstream), but the tip of that branch should always be in a working state. -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 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] git-submodule.sh: Support 'checkout' as a valid update command
2014/1/7 Junio C Hamano gits...@pobox.com: 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. I wouldn't be able to explain the change better than your description. Also, I was under the improper assumption that the change was obvious. Thank you very much for the amended patch description. Cheers, Francesco -- To unsubscribe from this list: send the line 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)
Am 06.01.2014 23:36, schrieb Junio C Hamano: * 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 What is the doneness of this one??? It's still work in progress. Currently I'm working on a test framework so we can reuse recursive submodule checkout tests instead of rewriting them for every command that learns the --recurse-submodule option. Will reroll this series as soon as I have something presentable. -- To unsubscribe from this list: send the line 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
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. 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. -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] 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: [PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false
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. I kind of feel in the --batch situation that it is somewhat useless (I wonder if rev-list --stdin should turn it off, too). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix safe_create_leading_directories() for Windows
Hi, On Tue, 7 Jan 2014, Erik Faye-Lund wrote: On Thu, Jan 2, 2014 at 9:46 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Well, you and I both know how easy GitHub's pull request made things for us as well as for contributors. I really cannot thank Erik enough for bullying me into using and accepting them. Huh? I don't think you refer to me, because I really dislike them (and I always have IIRC). Ah yes, I misremembered. You were actually opposed to using them and I thought we should be pragmatic to encourage contributions. In any case, I do think that the contributions we got via pull requests were in general contributions we would not otherwise have gotten. Sorry for my mistake! Dscho -- To unsubscribe from this list: send the line 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
Am 06.01.2014 23:40, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Does this new paragraph make it clearer? Don't we have bugs section that we can use to list the known limitations like this? Right, will change accordingly in v2. 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. We'll get the what we would like to see happen eventually test for free from the recursive submodule update framework I'm currently implementing, so I propose we don't implement this exepected failure in this patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] pager: set LV=-c alongside LESS=FRSX
Junio C Hamano gits...@pobox.com writes: - 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. Even the less manpage is inconsistent about whether $LESS should start with a dash (there are examples with and without it). Implementation-wise, less uses the same function to process an option argument (including the leading dash) and the value of $LESS, so the form with the leading dash is probably preferred. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line 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: [RFC v2] submodule: Respect requested branch on all clones
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]? 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. Otherwise the default method for that operation is a HEAD-detaching 'checkout'. I tried to explain it here [3]. W. Trevor King wk...@tremily.us writes: 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? That would be nice. I don't think his patch changes the docs, and I don't know if mentioning the --checkout option belongs in that patch as well, or in a separate fixup ;). Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/240097 [2]: http://article.gmane.org/gmane.comp.version-control.git/240117 [3]: http://article.gmane.org/gmane.comp.version-control.git/239953 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
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
2014/1/7 Junio C Hamano gits...@pobox.com: 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. Junio, for what it concerns me I fully support this patch as, IMO, it makes cleaner the role of the property submodule.name.branch. Because with my original proposal I decided to go non-breaking Heiko and Jens could also take position on this because this patch will represent a small behavior break. Also, and important feature should be added together with this patch: a way to go --remote by default on an attached HEAD. This can be done at least in two ways: - explicit, non breaking way: add a submodule.name.remote property. When set to true it implies --remote when doing git submodule update, both on attached and detached HEAD; - implicit, breaking way: assume --remote when doing git submodule update on an attached HEAD. I am quite sure this will break a couple of submodule tests (I already tried it), probably for marginal reasons. I think this is needed because it makes little sense to having an attached HEAD and git submodule update does nothing. Thank you, Francesco -- To unsubscribe from this list: send the line 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
[PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false
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.
Signed-off-by: Brodie Rao bro...@sf.io
---
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));
--
1.8.5.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Introduce git submodule attached update
2014/1/7 Junio C Hamano gits...@pobox.com: 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? Yep, you got it 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: [PATCH 2/2] Introduce git submodule attached update
On Mon, Jan 06, 2014 at 08:21:24PM +0100, David Engster wrote: +---+ | master | -- +---++---+| Merges to/from master | CEDET | | done only by CEDET developers +---+ | +---+| | stable | -- +---+ | | | | Any Emacs developer | can push and commit | submodule +++--+ | | Emacs | -- | lisp/cedet submodule | - +++--+ This looks reasonable, and except for the detached-HEAD after the initial update-clone, I think Git already supports everything you need. If you set submodule.cedet.update to 'rebase' (or 'merge') you can easily integrate your local master changes with cedet/master (e.g. if a CEDET dev updates cedet/master before the Emacs dev has a chance to push their fix). With the non-checkout update mode, you'll also stay on your checked-out master branch during 'submodule update' calls. AFAICS the main problem with this approach is that one always has to think of committing the new SHA1 of the submodule. … However, as Heiko notes, the history must be preserved to be able to go back to earlier revisions, so there must be some kind of commit for the submodule when 'stable' changes; maybe that could be automated somehow? If an Emacs dev in the submodule makes the CEDET change, you could use a post-commit hook (in the CEDET submodule) to also commit the change to the Emacs superproject). However, commiting only the submodule bump may not be what you want. Maybe there are other superproject changes that should be committed alongside the submodule bump. Maybe there is stuff in the superprojects's staging area that should *not* be committed alongside the submodule bump. This ambiguity makes it tricky for Git to automatically do “the right thing”. If cedet/master is updated independently by the CEDET devs, there's no way for the local Emacs repo to know about the change, so it's impossible to automatically update Emacs (without polling for CEDET updates or some other transgression ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
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 2/2] Introduce git submodule attached update
2014/1/7 Junio C Hamano gits...@pobox.com: Francesco Pretto cez...@gmail.com writes: 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? Because of the overlapping change with the the other patch proposed by Trevor, and to not generate confusion, I will stop for now pursuing for an attach|detach command/switch specific for submodules, waiting for Trevors's patch possible acceptance. After that I will see it still makes sense or not. -- To unsubscribe from this list: send the line 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
On Tue, Jan 07, 2014 at 08:19:49PM +0100, Francesco Pretto wrote: 2014/1/7 Junio C Hamano gits...@pobox.com: 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. Junio, for what it concerns me I fully support this patch as, IMO, it makes cleaner the role of the property submodule.name.branch. No, submodule.name.branch is the name of the remote-tracking branch for 'update --remote'. In this patch, I'm using it as a hint for the preferred local branch name [1], which I now think was a bad idea. After [2], I think that we should just define the preferred local branch name explicitly (submodule.name.local-branch?). Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/239980 [2]: http://article.gmane.org/gmane.comp.version-control.git/240097 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 W. Trevor King wk...@tremily.us: Junio, for what it concerns me I fully support this patch as, IMO, it makes cleaner the role of the property submodule.name.branch. No. Trevor, maybe it was not clear. But I wanted to say: I fully support *Trevor's* patch... :) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] submodule: Respect requested branch on all clones
On Tue, Jan 07, 2014 at 11:21:28AM -0800, Junio C Hamano wrote: W. Trevor King wk...@tremily.us writes: On Tue, Jan 07, 2014 at 10:15:25AM -0800, Junio C Hamano wrote: 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. Ah, thanks. s/rebase/reset/ in the commit message ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 Francesco Pretto cez...@gmail.com: 2014/1/7 Junio C Hamano gits...@pobox.com: 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. Junio, for what it concerns me I fully support *this* patch as, Where this patch is Trevor's one, don't get me wrong... :) -- To unsubscribe from this list: send the line 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
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, since otherwise a 40-hex sha1 does not need to do a ref lookup at all, and something like rev-list --objects | cat-file --batch-check processes a large number of these. Detecting (1) is not nearly as expensive. You must already be doing a ref lookup to trigger it (so the relative cost is much closer), and your query size is bounded by the number of refs, not the number of objects. Commit 25ffba78d traded off some safety for a lot of performance by disabling (2), but left (1) in place because the tradeoff is different. 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). 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. 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. 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). 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? See above. :) -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] drop unnecessary copying in credential_ask_one
On Tue, Jan 07, 2014 at 11:44:00AM -0800, Junio C Hamano wrote: 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. I would not be surprised if it did. Though it introduces its own portability issues, since we cannot depend on having it. But it is probably enough to just test_lazy_prereq EXPECT 'expect --version' or something. I dunno. I have never used expect, do not have it installed, and am not excited about introducing a new tool dependency. But if you want to explore it, be my guest. -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
[RFC/PATCH] format-patch: introduce branch.*.forkedFrom
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.
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
--
1.8.5.2.234.gba2dde8.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
[Fixed typo in Junio's address]
On Wed, Jan 8, 2014 at 1:59 AM, Ramkumar Ramachandra artag...@gmail.com wrote:
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.
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
--
1.8.5.2.234.gba2dde8.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] 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
On Wed, Jan 08, 2014 at 02:00:44AM +0530, Ramkumar Ramachandra wrote:
On Wed, Jan 8, 2014 at 1:59 AM, Ramkumar Ramachandra artag...@gmail.com
wrote:
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.
Cc: Jeff King p...@peff.net
Cc: Junio C Hamano gis...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
I have not carefully read some of the later bits of the discussion from
last night / this morning, so maybe I am missing something, but this
seems backwards to me from what Junio and I were discussing earlier.
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.
Am I missing something?
-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: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
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? I don't know why label the respin-workflow as being
screwed up.
--
To unsubscribe from this list: send the line 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
On Tue, Jan 07, 2014 at 03:40:56AM +0530, Ramkumar Ramachandra wrote:
Jeff King wrote:
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.
My issue with that is that I no idea where my branch is with respect
to my forked upstream; I find that extremely useful when doing
re-spins.
Right. I think there are two separate relationships, and they are both
shoe-horned into upstream. The solution is to let them be configured
separately (and fallback on each other as appropriate to make the burden
less on the user).
push.default = upstream is a bit of a disaster, in my opinion. I've
advocated push.default = current on multiple occasions, and wrote the
initial remote.pushDefault series with that configuration in mind.
Yeah, I agree with all of that.
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).
We already have a branch.*.pushremote, and I don't see the value of
branch.*.pushbranch (what you're referring to as pushupstream, I
assume) except for Gerrit users.
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.
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).
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.
We're transitioning to push.default = simple which is even simpler
than current.
Simpler in the sense that it is less likely to do something unexpected.
But the rules are actually more complicated. Two examples:
1. Imagine I make a feature branch foo forked off of origin/master, then
git push with no arguments. The current scheme would go to
foo on origin, but upstream would go to master. Since they
don't agree, simple will punt and tell me to be more specific.
2. Imagine I have set my default push remote to publish, am on
master (forked from origin/master) and I run git push without
arguments. current would push to master on publish. But
upstream will complain, because we are not pushing to our
upstream remote. I believe simple will therefore reject this.
In both cases, I think current does the sane thing, and simple makes
things more complicated. The one saving grace it has is that it punts on
these cases rather than potentially doing something destructive that the
user did not intend.
-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: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Jeff King wrote:
I have not carefully read some of the later bits of the discussion from
last night / this morning, so maybe I am missing something, but this
seems backwards to me from what Junio and I were discussing earlier.
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.
I have a local branch 'forkedfrom' that has two sources: 'master'
and 'ram/forkedfrom'. 'ram/forkedfrom' isn't a dumb publish-point:
the relationship information I get between 'forkedfrom' and
'ram/forkedfrom' is very useful; it's what helps me tell how my
re-roll is doing with respect to the original series; I'd often want
to cherry-pick commits/ messages from my original series to prepare
the re-roll, so interaction with this source is quite high. On the
other hand, the relationship information I get between 'forkedfrom'
and 'master' is practically useless: 'forkedfrom' is always ahead of
'master', and a divergence indicates that I need to rebase; I'll never
really need to interact with this source.
I'm only thinking in terms of what infrastructure we've already built:
if @{u} is set to 'ram/forkedfrom', we get a lot of information for
free _now_. If @{u} is set to 'master', the current git-status is
unhelpful.
--
To unsubscribe from this list: send the line 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
On Tue, Jan 07, 2014 at 04:17:00AM +0530, Ramkumar Ramachandra wrote:
Junio C Hamano wrote:.
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.pushdefaultset to 'publish', you can expect
git push
to do the right thing, and then always say
git show-branch publish/topic topic
I think it's highly sub-optimal to have a local-branch @{u} for
several reasons; the prompt is almost useless in this case, and it
will always show your forked-branch ahead of 'master' (assuming that
'master' doesn't update itself in the duration of your development).
I actually use local-branch @{u} all the time to represent inter-topic
dependencies. For example, imagine I have topic bar which builds on
topic foo, which is based on master. I would have:
[branch foo]
remote = origin
merge = refs/heads/master
[branch bar]
remote = .
merge = refs/heads/foo
When I rebase foo, I want to rebase it against upstream's master. When
I rebase bar, I want to rebase it against foo. And naturally, upstream
does not necessarily have a foo, because it is my topic, not theirs (I
_may_ have published my foo somewhere, but that is orthogonal, and
anyway my local foo is the most up-to-date source, not the pushed
version).
As an aside, if you want to rebase both branches, you have to topo-sort
them to make sure you do foo first, then rebase bar on the result.
My daily procedure is something like:
all_topics |
while read topic; do echo $topic $(git rev-parse $topic@{u}); done |
topo_sort |
while read topic upstream; do
git rebase $upstream $topic || exit 1
done
While doing respins, the prompt doesn't aid you in any way. Besides,
on several occasions, I found myself working on the same forked-branch
from two different machines; then the publish-point isn't necessarily
always a publish-point: it's just another upstream for the branch.
Right, things get trickier then. But I don't think there is an automatic
way around that. Sometimes the published one is more up to date, and
sometimes the upstream thing is more up to date. You have to manually
tell git which you are currently basing your work on. I find in such a
situation that it tends to resolve itself quickly, though, as the first
step is to pull in the changes you pushed up from the other machine
anyway (either via git reset or git rebase).
-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:
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: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
On Wed, Jan 08, 2014 at 02:32:10AM +0530, Ramkumar Ramachandra wrote:
we should leave @{upstream} as (1), and add a new option to represent
(2). Not the other way around.
I have a local branch 'forkedfrom' that has two sources: 'master'
and 'ram/forkedfrom'. 'ram/forkedfrom' isn't a dumb publish-point:
the relationship information I get between 'forkedfrom' and
'ram/forkedfrom' is very useful; it's what helps me tell how my
re-roll is doing with respect to the original series; I'd often want
to cherry-pick commits/ messages from my original series to prepare
the re-roll, so interaction with this source is quite high. On the
other hand, the relationship information I get between 'forkedfrom'
and 'master' is practically useless: 'forkedfrom' is always ahead of
'master', and a divergence indicates that I need to rebase; I'll never
really need to interact with this source.
Thanks for a concrete example.
I definitely respect the desire to reuse the existing tooling we have
for @{u}. At the same time, I think you are warping the meaning of
@{u} somewhat. It is _not_ your upstream here, but rather another
version of the branch that has useful changes in it. That might be
splitting hairs a bit, but I think you will find that the differences
leak through in inconvenient spots (like format-patch, where you really
_do_ want to default to the true upstream).
If we add @{publish} (and @{pu}), then it becomes very convenient to
refer to the ram/ version of your branch. That seems like an obvious
first step to me. We don't have to add new config, because
branch.*.pushremote already handles this.
Now you can do git rebase @{pu} which is nice, but not _quite_ as nice
as git rebase, which defaults to @{u}. That first step might be
enough, and I'd hold off there and try it out for a few days or weeks
first. But if you find in your workflow that you are having to specify
@{pu} a lot, then maybe it is worth adding an option to default rebase
to @{pu} instead of @{u}.
You end up in the same place (git rebase without options does what you
want), but I think the underlying data more accurately represents what
is going on (and there is no need to teach format-patch anything
special).
-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
On Tue, Jan 07, 2014 at 01:07:08PM -0800, Junio C Hamano wrote: 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. Hmm. The top patch of that series still relies on upstream being a push destination, though. So if I have a triangular workflow where I fork topic from origin/master, my git push origin topic will go to refs/heads/master on origin under the upstream rule. So that seems broken as ever. :) But I guess what you are referring to is that in a triangular world, the second patch lets me do: git config push.default current git config remote.origin.push 'refs/heads/*:refs/review/*' And then git push, git push origin, or git push origin topic all put it in review/topic, which is what I want. I think that is sensible, and only heightens my sense of the upstream push.default as useless. :) -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 wrote:
My daily procedure is something like:
all_topics |
while read topic; do echo $topic $(git rev-parse $topic@{u}); done |
topo_sort |
while read topic upstream; do
git rebase $upstream $topic || exit 1
done
Ah, I was perhaps over-specializing for my own usecase, where
everything is based off 'master'. I never considered 'master' a true
upstream because I throw away topic branches after the maintainer
merges them. If you have long-running branches that you work on a
daily basis, the issue is somewhat different.
While doing respins, the prompt doesn't aid you in any way. Besides,
on several occasions, I found myself working on the same forked-branch
from two different machines; then the publish-point isn't necessarily
always a publish-point: it's just another upstream for the branch.
Right, things get trickier then. But I don't think there is an automatic
way around that. Sometimes the published one is more up to date, and
sometimes the upstream thing is more up to date. You have to manually
tell git which you are currently basing your work on. I find in such a
situation that it tends to resolve itself quickly, though, as the first
step is to pull in the changes you pushed up from the other machine
anyway (either via git reset or git rebase).
My primary concern is that the proposed @{publish} should be a
first-class citizen; if it has everything that @{u} has, then we're
both good: you'd primarily use @{u}, while I'd primarily use
@{publish}.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/2] better document side effects when [re]moving a submodule
Am 07.01.2014 18:57, schrieb Jens Lehmann: Am 06.01.2014 23:40, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Does this new paragraph make it clearer? Don't we have bugs section that we can use to list the known limitations like this? Right, will change accordingly in v2. Changes from v1: - Document side effects under the BUGS section - Add similar documentation for git rm Jens Lehmann (2): mv: better document side effects when moving a submodule rm: better document side effects when removing a submodule Documentation/git-mv.txt | 12 Documentation/git-rm.txt | 9 + t/t3600-rm.sh| 16 t/t7001-mv.sh| 21 + 4 files changed, 58 insertions(+) -- 1.8.5.2.231.gfc86eb1 -- To unsubscribe from this list: send the line 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
On Wed, Jan 08, 2014 at 02:55:04AM +0530, Ramkumar Ramachandra wrote:
Jeff King wrote:
My daily procedure is something like:
all_topics |
while read topic; do echo $topic $(git rev-parse $topic@{u}); done |
topo_sort |
while read topic upstream; do
git rebase $upstream $topic || exit 1
done
Ah, I was perhaps over-specializing for my own usecase, where
everything is based off 'master'. I never considered 'master' a true
upstream because I throw away topic branches after the maintainer
merges them. If you have long-running branches that you work on a
daily basis, the issue is somewhat different.
What I do is maybe somewhat gross, but I continually rebase my patches
forward as master develops. So they diverge from where Junio has forked
them upstream (which does not necessarily have any relationship with
where I forked from, anyway). The nice thing about this is that
eventually the topic becomes empty, as rebase drops patches that were
merged upstream (or resolve conflicts to end up at an empty patch).
It's a nice way of tracking the progress of the patch upstream, and it
catches any differences between what's upstream and what's in the topic
(in both directions: you see where the maintainer may have marked up
your patch, and you may see a place where you added something to be
squashed but the maintainer missed it). The downside is that sometimes
the conflicts are annoying and complicated (e.g., several patches that
touch the same spot are a pain to rebase on top of themselves; the early
ones are confused that the later changes are already in place).
My primary concern is that the proposed @{publish} should be a
first-class citizen; if it has everything that @{u} has, then we're
both good: you'd primarily use @{u}, while I'd primarily use
@{publish}.
Definitely. I think that's the world we want to work towards.
-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
[PATCH v2 1/2] mv: better document side effects when moving a submodule
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 in the new BUGS section. Also document this behavior in a new test. Reported-by: George Papanikolaou g3orge@gmail.com Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Documentation/git-mv.txt | 12 t/t7001-mv.sh| 21 + 2 files changed, 33 insertions(+) diff --git a/Documentation/git-mv.txt b/Documentation/git-mv.txt index b1f7988..e453132 100644 --- a/Documentation/git-mv.txt +++ b/Documentation/git-mv.txt @@ -52,6 +52,18 @@ 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). +BUGS + +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 -- 1.8.5.2.231.gfc86eb1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] rm: better document side effects when removing a submodule
The Submodules section of the git rm documentation mentions what will happen when a submodule with a gitfile gets removed with newer git. But it doesn't talk about what happens when the user changes between commits before and after the removal, which does not remove the submodule from the work tree like using the rm command did the first time. Explain what happens and what the user has to do manually to fix that in the new BUGS section. Also document this behavior in a new test. Signed-off-by: Jens Lehmann jens.lehm...@web.de --- Documentation/git-rm.txt | 9 + t/t3600-rm.sh| 16 2 files changed, 25 insertions(+) diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt index 9d731b4..f1efc11 100644 --- a/Documentation/git-rm.txt +++ b/Documentation/git-rm.txt @@ -170,6 +170,15 @@ of files and subdirectories under the `Documentation/` directory. (i.e. you are listing the files explicitly), it does not remove `subdir/git-foo.sh`. +BUGS + +Each time a superproject update removes a populated submodule +(e.g. when switching between commits before and after the removal) a +stale submodule checkout will remain in the old location. Removing the +old directory is only safe when it uses a gitfile, as otherwise the +history of the submodule will be deleted too. This step will be +obsolete when recursive submodule update has been implemented. + SEE ALSO linkgit:git-add[1] diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 540c49b..3d30581 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -705,6 +705,22 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc rm -rf submod ' +test_expect_success 'checking out a commit after submodule removal needs manual updates' ' + git commit -m submodule removal submod + git checkout HEAD^ + git submodule update + git checkout -q HEAD^ 2actual + git checkout -q master 2actual + echo warning: unable to rmdir submod: Directory not empty expected + test_i18ncmp expected actual + git status -s submod actual + echo ?? submod/ expected + test_cmp expected actual + rm -rf submod + git status -s -uno --ignore-submodules=none actual + ! test -s actual +' + test_expect_success 'rm of d/f when d has become a non-directory' ' rm -rf d mkdir d -- 1.8.5.2.231.gfc86eb1 -- To unsubscribe from this list: send the line 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
On Tue, Jan 07, 2014 at 09:09:19PM +0100, Francesco Pretto wrote:
2014/1/7 W. Trevor King wk...@tremily.us:
Trevor, maybe it was not clear. But I wanted to say:
I fully support *Trevor's* patch... :)
Which I appreciate ;). I still though I should point out that my
patch *confuses* the role of submodule.name.branch :p.
You are welcome. Also, at your wish, can you please reply also in
public?
Here you go.
I'd be happy to hear ideas about superproject-branch-specific local
overrides to a hypothetical submodule.name.local-branch, in the
event that a developer doesn't like a default set in .gitmodules. If
I could think of a way to do that, we could avoid this heuristic
approach, and make the local submodule.name.local-branch
vs. remote-tracking submodule.name.branch distinction more obvious.
It would also be nice if submodule.name.branch was just an initial
setup-time and detached-HEAD default. If the submodule is on a branch
it would make more sense to use the checked-out branch's @{upstream}.
Cheers,
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: [RFC/PATCH] format-patch: introduce branch.*.forkedFrom
Jeff King wrote:
I definitely respect the desire to reuse the existing tooling we have
for @{u}. At the same time, I think you are warping the meaning of
@{u} somewhat. It is _not_ your upstream here, but rather another
version of the branch that has useful changes in it. That might be
splitting hairs a bit, but I think you will find that the differences
leak through in inconvenient spots (like format-patch, where you really
_do_ want to default to the true upstream).
Thanks for the clear reasoning.
If we add @{publish} (and @{pu}), then it becomes very convenient to
refer to the ram/ version of your branch. That seems like an obvious
first step to me. We don't have to add new config, because
branch.*.pushremote already handles this.
Agreed. I'll start working on @{publish}. It's going to take quite a
bit of effort, because I won't actually start using it until my prompt
is @{publish}-aware.
Now you can do git rebase @{pu} which is nice, but not _quite_ as nice
as git rebase, which defaults to @{u}. That first step might be
enough, and I'd hold off there and try it out for a few days or weeks
first. But if you find in your workflow that you are having to specify
@{pu} a lot, then maybe it is worth adding an option to default rebase
to @{pu} instead of @{u}.
Actually, I'm not sure I'd use git rebase @{pu}; for me @{pu} is
mainly a source of information for taking apart to build a new series.
--
To unsubscribe from this list: send the line 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
On Tue, Jan 7, 2014 at 6:56 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Well, you and I both know how easy GitHub's pull request made things for us as well as for contributors. I really cannot thank Erik enough for bullying me into using and accepting them. Huh? I don't think you refer to me, because I really dislike them (and I always have IIRC). Ah yes, I misremembered. You were actually opposed to using them and I thought we should be pragmatic to encourage contributions. Not that it matters too much, but I guess it was me who talked Dscho into moving to GitHub and using / accepting pull requests :-) In any case, I do think that the contributions we got via pull requests were in general contributions we would not otherwise have gotten. I absolutely think so, too. -- Sebastian Schuberth -- To unsubscribe from this list: send the line 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
2014/1/7 W. Trevor King wk...@tremily.us: I'd be happy to hear ideas about superproject-branch-specific local overrides to a hypothetical submodule.name.local-branch, in the event that a developer doesn't like a default set in .gitmodules. If I could think of a way to do that, we could avoid this heuristic approach, and make the local submodule.name.local-branch vs. remote-tracking submodule.name.branch distinction more obvious. Uh, I think you got it wrong in the other thread: I didn't proposed such feature. I just wanted the attached submodule use case to be supported and of course --branch means attached is even easier to get this. -- To unsubscribe from this list: send the line 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] sha1_name: don't resolve refs when core.warnambiguousrefs is false
On Tue, Jan 07, 2014 at 12:31:57PM -0800, Junio C Hamano wrote: 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. Here's a series to do that. The first three are just cleanups I noticed while looking at the problem. While I was writing the commit messages, though, I had a thought. Maybe we could simply do the check faster for the common case that most refs do not look like object names? Right now we blindly call dwim_ref for each get_sha1 call, which is the expensive part. If we instead just loaded all of the refnames from the dwim_ref location (basically heads, tags and the top-level of refs/), we could build an index of all of the entries matching the 40-hex pattern. In 99% of cases, this would be zero entries, and the check would collapse to a simple integer comparison (and even if we did have one, it would be a simple binary search in memory). Our index is more racy than actually checking the filesystem, but I don't think it matters here. Anyway, here is the series I came up with, in the meantime. I can take a quick peek at just making it faster, too. [1/4]: cat-file: refactor error handling of batch_objects [2/4]: cat-file: fix a minor memory leak in batch_objects [3/4]: cat-file: restore ambiguity warning flag in batch_objects [4/4]: revision: turn off object/refname ambiguity check for --stdin -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
[PATCH 2/4] cat-file: fix a minor memory leak in batch_objects
We should always have been freeing our strbuf, but doing so consistently was annoying until the refactoring in the previous patch. Signed-off-by: Jeff King p...@peff.net --- builtin/cat-file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 971cdde..ce79103 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -314,6 +314,7 @@ static int batch_objects(struct batch_options *opt) break; } + strbuf_release(buf); return retval; } -- 1.8.5.2.500.g8060133 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] cat-file: restore ambiguity warning flag in batch_objects
Since commit 25fba78, we turn off the object/refname ambiguity warning using a global flag. However, we never restore it. This doesn't really matter in the current code, since the program generally exits immediately after the function is done, but it's good code hygeine to clean up after ourselves. Signed-off-by: Jeff King p...@peff.net --- builtin/cat-file.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index ce79103..c64e287 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -264,6 +264,7 @@ static int batch_objects(struct batch_options *opt) struct strbuf buf = STRBUF_INIT; struct expand_data data; int retval = 0; + int save_warning = warn_on_object_refname_ambiguity; if (!opt-format) opt-format = %(objectname) %(objecttype) %(objectsize); @@ -314,6 +315,7 @@ static int batch_objects(struct batch_options *opt) break; } + warn_on_object_refname_ambiguity = save_warning; strbuf_release(buf); return retval; } -- 1.8.5.2.500.g8060133 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] cat-file: refactor error handling of batch_objects
This just pulls the return value for the function out of the
inner loop, so we can break out of the loop rather than do
an early return. This will make it easier to put any cleanup
for the function in one place.
Signed-off-by: Jeff King p...@peff.net
---
Just making the subsequent diffs less noisy...
builtin/cat-file.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f8288c8..971cdde 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -263,6 +263,7 @@ static int batch_objects(struct batch_options *opt)
{
struct strbuf buf = STRBUF_INIT;
struct expand_data data;
+ int retval = 0;
if (!opt-format)
opt-format = %(objectname) %(objecttype) %(objectsize);
@@ -294,8 +295,6 @@ static int batch_objects(struct batch_options *opt)
warn_on_object_refname_ambiguity = 0;
while (strbuf_getline(buf, stdin, '\n') != EOF) {
- int error;
-
if (data.split_on_whitespace) {
/*
* Split at first whitespace, tying off the beginning
@@ -310,12 +309,12 @@ static int batch_objects(struct batch_options *opt)
data.rest = p;
}
- error = batch_one_object(buf.buf, opt, data);
- if (error)
- return error;
+ retval = batch_one_object(buf.buf, opt, data);
+ if (retval)
+ break;
}
- return 0;
+ return retval;
}
static const char * const cat_file_usage[] = {
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] revision: turn off object/refname ambiguity check for --stdin
We currently check that any 40-hex object name we receive is
not also a refname, and output a warning if this is the
case. When rev-list --stdin is used to receive object
names, we may receive a large number of inputs, and the cost
of checking each name as a ref is relatively high.
Commit 25fba78d already dropped this warning for cat-file
--batch-check. The same reasoning applies for rev-list
--stdin. Let's disable the check in that instance.
Here are before and after numbers:
$ git rev-list --all commits
[before]
$ best-of-five -i commits ./git rev-list --stdin --no-walk --pretty=raw
real0m0.675s
user0m0.552s
sys 0m0.120s
[after]
$ best-of-five -i commits ./git rev-list --stdin --no-walk --pretty=raw
real0m0.415s
user0m0.400s
sys 0m0.012s
Signed-off-by: Jeff King p...@peff.net
---
Obviously we drop this one (and revert 25fba78d) if I can just make the
check faster.
revision.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/revision.c b/revision.c
index a68fde6..87d04dd 100644
--- a/revision.c
+++ b/revision.c
@@ -1576,7 +1576,9 @@ static void read_revisions_from_stdin(struct rev_info
*revs,
{
struct strbuf sb;
int seen_dashdash = 0;
+ int save_warning = warn_on_object_refname_ambiguity;
+ warn_on_object_refname_ambiguity = 0;
strbuf_init(sb, 1000);
while (strbuf_getwholeline(sb, stdin, '\n') != EOF) {
int len = sb.len;
@@ -1595,6 +1597,7 @@ static void read_revisions_from_stdin(struct rev_info
*revs,
REVARG_CANNOT_BE_FILENAME))
die(bad revision '%s', sb.buf);
}
+ warn_on_object_refname_ambiguity = save_warning;
if (seen_dashdash)
read_pathspec_from_stdin(revs, sb, prune);
strbuf_release(sb);
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line 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
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.
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line 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
On Tue, Jan 07, 2014 at 02:06:12PM -0800, Junio C Hamano wrote:
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?).
I do not think there is any centralized days. From day one, Linus
advocated a triangular workflow, and that is how git and kernel develop
has always been done. And that is why the default of matching was
there.
There were people who came later, and who still exist today, who use git
in an SVN-like centralized way. So if there were centralized days, we
are in them now. :)
I just do not see any real advantage even in a centralized world for
upstream versus current. Before remote.pushdefault, I can
potentially see some use (if you want to abuse @{upstream}), but now I
do not see any point.
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.
I dunno. I have not gone back and read all of the arguments around
push.default from last year. It is entirely possible everything I just
said was refuted back then, and I am needlessly rehashing old arguments.
I remember that Matthieu was one of the main advocates of upstream. I
am cc-ing him here to bring his attention (not just to this message, but
to the whole thread).
-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: 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
Preferred local submodule branches (was: Introduce git submodule attached update)
On Tue, Jan 07, 2014 at 10:51:34PM +0100, Francesco Pretto wrote: 2014/1/7 W. Trevor King wk...@tremily.us: I'd be happy to hear ideas about superproject-branch-specific local overrides to a hypothetical submodule.name.local-branch, in the event that a developer doesn't like a default set in .gitmodules. If I could think of a way to do that, we could avoid this heuristic approach, and make the local submodule.name.local-branch vs. remote-tracking submodule.name.branch distinction more obvious. Uh, I think you got it wrong in the other thread: I'm grafting this discussion back on to the thread where I proposed submodule.name.local-branch. I didn't proposed such feature. Right. I proposed this feature after reading your proposed workflow. I just wanted the attached submodule use case to be supported and of course --branch means attached is even easier to get this. As I understood it, the '--branch means attached' stuff was tied up with automatic --remote updates. There are three branches that submodule folks usually care about: 1. The linked $sha1 in the superproject (set explicitly for every superproject commit, and thus for every superproject branch). 2. The remote-tracking submodule.name.branch that lives in the upstream submodule.name.url repository. 3. The submodule's locally checked out branch, which we currently let the developer setup by hand, which is used integrated with one of the other two branches during non-checkout updates. Git is currently a bit weak on conveniently handling type-3 branches. “Just use what the developer has setup” works well for many basic workflows, but falls short for: * Cloning-updates, where we currently always setup a detached HEAD. * Workflows where the preferred type-3 branch depends on the superproject branch. The former is easy to fix [1] if you accept submodule.name.branch as a guess, but this conflates the type-2 and type-3 branches. For the latter, you'd want something like: On Mon, Jan 06, 2014 at 08:10:04PM -0800, W. Trevor King wrote: * Auto checkout of the preferred branch * Can do this at clone-update time with my patch. * For later submodule branch switches, maybe we want: git submodule checkout [-b branch] [paths…] Then if a user blows off their detached HEAD, at least they'll feel a bit sheepish afterwards. which would likely need some of Jens' new core checkout handling [2]. Cheers, Trevor [1]: Using something along the lines of my http://article.gmane.org/gmane.comp.version-control.git/239967 [2]: http://article.gmane.org/gmane.comp.version-control.git/240117 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Re: [PATCH 2/2] Introduce git submodule attached update
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Mon, Jan 06, 2014 at 08:10:04PM -0800, W. Trevor King wrote: Here's an attempted summary of our desires, and my ideal route forward: * Preferred local submodule branches for each superproject branch. * Not currently supported by Git. * Requires some sort of per-superproject-branch .git/config. * Fall back to the remote-tracking submodule.name.branch? * Auto checkout of the preferred branch * Can do this at clone-update time with my patch. * For later submodule branch switches, maybe we want: git submodule checkout [-b branch] [paths…] Then if a user blows off their detached HEAD, at least they'll feel a bit sheepish afterwards. Well, for development on a detached HEAD in a submodule we are currently not very careful anyway. A simple git submodule update will already blow away any detached HEAD work. But AFAIK it should trigger the you are leaving commits from a detached HEAD behind warning, so there is some safeguard and recovery. Cheers Heiko -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.14 (GNU/Linux) iEYEARECAAYFAlLMhPAACgkQjLR3Aoip+rqP6wCeIhtpWLJC3XVO3nu2ViQTbHPg T5wAoLLEZ256GOOjBxoTKo2/FmfvQGLp =+bqm -END PGP SIGNATURE- -- To unsubscribe from this list: send the line 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
On Tue, Jan 07, 2014 at 11:51:28PM +0100, Heiko Voigt wrote: On Mon, Jan 06, 2014 at 08:10:04PM -0800, W. Trevor King wrote: Here's an attempted summary of our desires, and my ideal route forward: * Preferred local submodule branches for each superproject branch. * Not currently supported by Git. * Requires some sort of per-superproject-branch .git/config. * Fall back to the remote-tracking submodule.name.branch? * Auto checkout of the preferred branch * Can do this at clone-update time with my patch. * For later submodule branch switches, maybe we want: git submodule checkout [-b branch] [paths…] Then if a user blows off their detached HEAD, at least they'll feel a bit sheepish afterwards. Well, for development on a detached HEAD in a submodule we are currently not very careful anyway. A simple git submodule update will already blow away any detached HEAD work. Only if you use the checkout strategy. With --merge or --rebase, you'll have the $sha1 (or upstream remote with --remote) integrated with your detached HEAD work. You end up with a new detached HEAD containing the result of the integration (just confirmed with tests using Git v1.8.3.2). That seems reasonable to me, so I'm happy with the integration logic. But AFAIK it should trigger the you are leaving commits from a detached HEAD behind warning, so there is some safeguard and recovery. I did not see those in testing with Git v1.8.3.2, likely because of the '-f -q' we pass to 'git checkout' for checkout-mode updates. Regardless of branch integration issues, I think a per-superproject-branch preferred submodule branch is important for 'git checkout' to work in the superproject. If you want: * submodule branch master for superproject branch master, and * submodule branch my-feature for superproject branch my-feature, $ git checkout my-feature in the superproject is currently going to leave you with the submodule on master, which is not convenient ;). I think we should come up with a better solution to the superproject checkout problem before adding in additional complications due to branch integration ;). Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: Preferred local submodule branches (was: Introduce git submodule attached update)
On Tue, Jan 07, 2014 at 02:36:25PM -0800, W. Trevor King wrote: There are three branches that submodule folks usually care about: 1. The linked $sha1 in the superproject (set explicitly for every superproject commit, and thus for every superproject branch). 2. The remote-tracking submodule.name.branch that lives in the upstream submodule.name.url repository. 3. The submodule's locally checked out branch, which we currently let the developer setup by hand, which is used integrated with one of the other two branches during non-checkout updates. Git is currently a bit weak on conveniently handling type-3 branches. “Just use what the developer has setup” works well for many basic workflows, but falls short for: * Cloning-updates, where we currently always setup a detached HEAD. * Workflows where the preferred type-3 branch depends on the superproject branch. The former is easy to fix [1] if you accept submodule.name.branch as a guess, but this conflates the type-2 and type-3 branches. For the latter, you'd want something like: On Mon, Jan 06, 2014 at 08:10:04PM -0800, W. Trevor King wrote: * Auto checkout of the preferred branch * Can do this at clone-update time with my patch. * For later submodule branch switches, maybe we want: git submodule checkout [-b branch] [paths…] Then if a user blows off their detached HEAD, at least they'll feel a bit sheepish afterwards. which would likely need some of Jens' new core checkout handling [2]. [1]: Using something along the lines of my http://article.gmane.org/gmane.comp.version-control.git/239967 [2]: http://article.gmane.org/gmane.comp.version-control.git/240117 For example, in Jonathan's recent version of Jens' series, the initial-setup and update functionality are moving into C. See: * populate_submodule() [1] for the initial-clone setup (calling 'read-tree'), and * update_submodule() [2] for subsequent updates (calling 'checkout -q' with an optional '-f') this is where any submodule.name.local-branch would come into play, if we decide to go down that route. It doesn't look like the C updates have the auto-clone functionality that the Bash updates have. I'm not sure if that's in the pipe or not. I'm not as familiar with the C implementation though, so maybe I'm missing the mark here. Cheers, Trevor [1]: http://article.gmane.org/gmane.comp.version-control.git/239698 [2]: http://article.gmane.org/gmane.comp.version-control.git/239699 -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH v2] speeding up 40-hex ambiguity check
On Tue, Jan 07, 2014 at 05:08:56PM -0500, Jeff King wrote: 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. Here's a series to do that. The first three are just cleanups I noticed while looking at the problem. While I was writing the commit messages, though, I had a thought. Maybe we could simply do the check faster for the common case that most refs do not look like object names? Right now we blindly call dwim_ref for each get_sha1 call, which is the expensive part. If we instead just loaded all of the refnames from the dwim_ref location (basically heads, tags and the top-level of refs/), we could build an index of all of the entries matching the 40-hex pattern. In 99% of cases, this would be zero entries, and the check would collapse to a simple integer comparison (and even if we did have one, it would be a simple binary search in memory). That turned out very nicely, and I think we can drop the extra flag entirely. Brodie's patch still makes sense, for people who do want to turn off ambiguity warnings entirely (and I built on his patch, which matters textually for 4 and 5, but it would be easy to rebase). I'm cc-ing Michael, since it is his ref-traversal code I am butchering in the 3rd patch. The first two are the unrelated cleanups from v1. They are not necessary, but I do not see any reason not to include them. [1/5]: cat-file: refactor error handling of batch_objects [2/5]: cat-file: fix a minor memory leak in batch_objects [3/5]: refs: teach for_each_ref a flag to avoid recursion [4/5]: get_sha1: speed up ambiguous 40-hex test [5/5]: get_sha1: drop object/refname ambiguity flag -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
[PATCH v2 2/5] cat-file: fix a minor memory leak in batch_objects
We should always have been freeing our strbuf, but doing so consistently was annoying until the refactoring in the previous patch. Signed-off-by: Jeff King p...@peff.net --- builtin/cat-file.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 971cdde..ce79103 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -314,6 +314,7 @@ static int batch_objects(struct batch_options *opt) break; } + strbuf_release(buf); return retval; } -- 1.8.5.2.500.g8060133 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/5] cat-file: refactor error handling of batch_objects
This just pulls the return value for the function out of the
inner loop, so we can break out of the loop rather than do
an early return. This will make it easier to put any cleanup
for the function in one place.
Signed-off-by: Jeff King p...@peff.net
---
builtin/cat-file.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f8288c8..971cdde 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -263,6 +263,7 @@ static int batch_objects(struct batch_options *opt)
{
struct strbuf buf = STRBUF_INIT;
struct expand_data data;
+ int retval = 0;
if (!opt-format)
opt-format = %(objectname) %(objecttype) %(objectsize);
@@ -294,8 +295,6 @@ static int batch_objects(struct batch_options *opt)
warn_on_object_refname_ambiguity = 0;
while (strbuf_getline(buf, stdin, '\n') != EOF) {
- int error;
-
if (data.split_on_whitespace) {
/*
* Split at first whitespace, tying off the beginning
@@ -310,12 +309,12 @@ static int batch_objects(struct batch_options *opt)
data.rest = p;
}
- error = batch_one_object(buf.buf, opt, data);
- if (error)
- return error;
+ retval = batch_one_object(buf.buf, opt, data);
+ if (retval)
+ break;
}
- return 0;
+ return retval;
}
static const char * const cat_file_usage[] = {
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/5] refs: teach for_each_ref a flag to avoid recursion
The normal for_each_ref traversal descends into
subdirectories, returning each ref it finds. However, in
some cases we may want to just iterate over the top-level of
a certain part of the tree.
The introduction of the flags option is a little
mysterious. We already have a flags option that gets stuck
in a callback struct and ends up interpreted in do_one_ref.
But the traversal itself does not currently have any flags,
and it needs to know about this new flag.
We _could_ introduce this as a completely separate flag
parameter. But instead, we simply put both flag types into a
single namespace, and make it available at both sites. This
is simple, and given that we do not have a proliferation of
flags (we have had exactly one until now), it is probably
sufficient.
Signed-off-by: Jeff King p...@peff.net
---
I think the flags thing is OK as explained above, but Michael may have a
different suggestion for refactoring.
refs.c | 61 ++---
1 file changed, 38 insertions(+), 23 deletions(-)
diff --git a/refs.c b/refs.c
index 3926136..ca854d6 100644
--- a/refs.c
+++ b/refs.c
@@ -589,6 +589,8 @@ static void sort_ref_dir(struct ref_dir *dir)
/* Include broken references in a do_for_each_ref*() iteration: */
#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
+/* Do not recurse into subdirs, just iterate at a single level. */
+#define DO_FOR_EACH_NO_RECURSE 0x02
/*
* Return true iff the reference described by entry can be resolved to
@@ -661,7 +663,8 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
* called for all references, including broken ones.
*/
static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
- each_ref_entry_fn fn, void *cb_data)
+ each_ref_entry_fn fn, void *cb_data,
+ int flags)
{
int i;
assert(dir-sorted == dir-nr);
@@ -669,9 +672,13 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir,
int offset,
struct ref_entry *entry = dir-entries[i];
int retval;
if (entry-flag REF_DIR) {
- struct ref_dir *subdir = get_ref_dir(entry);
- sort_ref_dir(subdir);
- retval = do_for_each_entry_in_dir(subdir, 0, fn,
cb_data);
+ if (flags DO_FOR_EACH_NO_RECURSE) {
+ struct ref_dir *subdir = get_ref_dir(entry);
+ sort_ref_dir(subdir);
+ retval = do_for_each_entry_in_dir(subdir, 0,
+ fn, cb_data,
+ flags);
+ }
} else {
retval = fn(entry, cb_data);
}
@@ -691,7 +698,8 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir,
int offset,
*/
static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
struct ref_dir *dir2,
-each_ref_entry_fn fn, void *cb_data)
+each_ref_entry_fn fn, void *cb_data,
+int flags)
{
int retval;
int i1 = 0, i2 = 0;
@@ -702,10 +710,12 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
struct ref_entry *e1, *e2;
int cmp;
if (i1 == dir1-nr) {
- return do_for_each_entry_in_dir(dir2, i2, fn, cb_data);
+ return do_for_each_entry_in_dir(dir2, i2, fn, cb_data,
+ flags);
}
if (i2 == dir2-nr) {
- return do_for_each_entry_in_dir(dir1, i1, fn, cb_data);
+ return do_for_each_entry_in_dir(dir1, i1, fn, cb_data,
+ flags);
}
e1 = dir1-entries[i1];
e2 = dir2-entries[i2];
@@ -713,12 +723,15 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
if (cmp == 0) {
if ((e1-flag REF_DIR) (e2-flag REF_DIR)) {
/* Both are directories; descend them in
parallel. */
- struct ref_dir *subdir1 = get_ref_dir(e1);
- struct ref_dir *subdir2 = get_ref_dir(e2);
- sort_ref_dir(subdir1);
- sort_ref_dir(subdir2);
- retval = do_for_each_entry_in_dirs(
- subdir1, subdir2, fn, cb_data);
+ if (!(flags DO_FOR_EACH_NO_RECURSE)) {
+ struct ref_dir *subdir1 =
[PATCH v2 5/5] get_sha1: drop object/refname ambiguity flag
Now that our object/refname ambiguity test is much faster
(thanks to the previous commit), there is no reason for code
like cat-file --batch-check to turn it off. Here are
before and after timings with this patch (on git.git):
$ git rev-list --objects --all | cut -d' ' -f1 objects
[with flag]
$ best-of-five -i objects ./git cat-file --batch-check
real0m0.392s
user0m0.368s
sys 0m0.024s
[without flag, without speedup; i.e., pre-25fba78]
$ best-of-five -i objects ./git cat-file --batch-check
real0m1.652s
user0m0.904s
sys 0m0.748s
[without flag, with speedup]
$ best-of-five -i objects ./git cat-file --batch-check
real0m0.388s
user0m0.356s
sys 0m0.028s
So the new implementation does just as well as we did with
the flag turning the whole thing off (better actually, but
that is within the noise).
Signed-off-by: Jeff King p...@peff.net
---
builtin/cat-file.c | 9 -
cache.h| 1 -
environment.c | 1 -
sha1_name.c| 2 +-
4 files changed, 1 insertion(+), 12 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index ce79103..afba21f 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -285,15 +285,6 @@ static int batch_objects(struct batch_options *opt)
if (opt-print_contents)
data.info.typep = data.type;
- /*
-* We are going to call get_sha1 on a potentially very large number of
-* objects. In most large cases, these will be actual object sha1s. The
-* cost to double-check that each one is not also a ref (just so we can
-* warn) ends up dwarfing the actual cost of the object lookups
-* themselves. We can work around it by just turning off the warning.
-*/
- warn_on_object_refname_ambiguity = 0;
-
while (strbuf_getline(buf, stdin, '\n') != EOF) {
if (data.split_on_whitespace) {
/*
diff --git a/cache.h b/cache.h
index ce377e1..73afc38 100644
--- a/cache.h
+++ b/cache.h
@@ -566,7 +566,6 @@ extern int assume_unchanged;
extern int prefer_symlink_refs;
extern int log_all_ref_updates;
extern int warn_ambiguous_refs;
-extern int warn_on_object_refname_ambiguity;
extern int shared_repository;
extern const char *apply_default_whitespace;
extern const char *apply_default_ignorewhitespace;
diff --git a/environment.c b/environment.c
index 3c76905..c59f6d4 100644
--- a/environment.c
+++ b/environment.c
@@ -22,7 +22,6 @@ int prefer_symlink_refs;
int is_bare_repository_cfg = -1; /* unspecified */
int log_all_ref_updates = -1; /* unspecified */
int warn_ambiguous_refs = 1;
-int warn_on_object_refname_ambiguity = 1;
int repository_format_version;
const char *git_commit_encoding;
const char *git_log_output_encoding;
diff --git a/sha1_name.c b/sha1_name.c
index f83ecb7..b9aaf74 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -451,7 +451,7 @@ 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_ambiguous_refs warn_on_object_refname_ambiguity) {
+ if (warn_ambiguous_refs) {
if (sha1_is_ambiguous_with_ref(sha1)) {
warning(warn_msg, len, str);
if (advice_object_name_warning)
--
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 4/5] get_sha1: speed up ambiguous 40-hex test
Since 798c35f (get_sha1: warn about full or short object
names that look like refs, 2013-05-29), a 40-hex sha1 causes
us to call dwim_ref on the result, on the off chance that we
have a matching ref. This can cause a noticeable slow-down
when there are a large number of objects. E.g., on
linux.git:
[baseline timing]
$ best-of-five git rev-list --all --pretty=raw
real0m3.996s
user0m3.900s
sys 0m0.100s
[same thing, but calling get_sha1 on each commit from stdin]
$ git rev-list --all commits
$ best-of-five -i commits git rev-list --stdin --pretty=raw
real0m7.862s
user0m6.108s
sys 0m1.760s
The problem is that each call to dwim_ref individually stats
the possible refs in refs/heads, refs/tags, etc. In the
common case, there are no refs that look like sha1s at all.
We can therefore do the same check much faster by loading
all ambiguous-looking candidates once, and then checking our
index for each object.
This is technically more racy (somebody might create such a
ref after we build our index), but that's OK, as it's just a
warning (and we provide no guarantees about whether a
simultaneous process ran before or after the ref was created
anyway).
Here is the time after this patch, which implements the
strategy described above:
$ best-of-five -i commits git rev-list --stdin --pretty=raw
real0m4.966s
user0m4.776s
sys 0m0.192s
We still pay some price to read the commits from stdin, but
notice the system time is much lower, as we are avoiding
hundreds of thousands of stat() calls.
Signed-off-by: Jeff King p...@peff.net
---
I wanted to make the ref traversal as cheap as possible, hence the
NO_RECURSE flag I added. I thought INCLUDE_BROKEN used to not open up
the refs at all, but it looks like it does these days. I wonder if that
is worth changing or not.
refs.c | 47 +++
refs.h | 2 ++
sha1_name.c | 4 +---
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index ca854d6..cddd871 100644
--- a/refs.c
+++ b/refs.c
@@ -4,6 +4,7 @@
#include tag.h
#include dir.h
#include string-list.h
+#include sha1-array.h
/*
* Make sure ref is something reasonable to have under .git/refs/;
@@ -2042,6 +2043,52 @@ int dwim_log(const char *str, int len, unsigned char
*sha1, char **log)
return logs_found;
}
+static int check_ambiguous_sha1_ref(const char *refname,
+ const unsigned char *sha1,
+ int flags,
+ void *data)
+{
+ unsigned char tmp_sha1[20];
+ if (strlen(refname) == 40 !get_sha1_hex(refname, tmp_sha1))
+ sha1_array_append(data, tmp_sha1);
+ return 0;
+}
+
+static void build_ambiguous_sha1_ref_index(struct sha1_array *idx)
+{
+ const char **rule;
+
+ for (rule = ref_rev_parse_rules; *rule; rule++) {
+ const char *prefix = *rule;
+ const char *end = strstr(prefix, %.*s);
+ char *buf;
+
+ if (!end)
+ continue;
+
+ buf = xmemdupz(prefix, end - prefix);
+ do_for_each_ref(ref_cache, buf, check_ambiguous_sha1_ref,
+ end - prefix,
+ DO_FOR_EACH_INCLUDE_BROKEN |
+ DO_FOR_EACH_NO_RECURSE,
+ idx);
+ free(buf);
+ }
+}
+
+int sha1_is_ambiguous_with_ref(const unsigned char *sha1)
+{
+ struct sha1_array idx = SHA1_ARRAY_INIT;
+ static int loaded;
+
+ if (!loaded) {
+ build_ambiguous_sha1_ref_index(idx);
+ loaded = 1;
+ }
+
+ return sha1_array_lookup(idx, sha1) = 0;
+}
+
static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
int flags, int *type_p)
diff --git a/refs.h b/refs.h
index 87a1a79..c7d5f89 100644
--- a/refs.h
+++ b/refs.h
@@ -229,4 +229,6 @@ int update_refs(const char *action, const struct ref_update
**updates,
extern int parse_hide_refs_config(const char *var, const char *value, const
char *);
extern int ref_is_hidden(const char *);
+int sha1_is_ambiguous_with_ref(const unsigned char *sha1);
+
#endif /* REFS_H */
diff --git a/sha1_name.c b/sha1_name.c
index a5578f7..f83ecb7 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -452,13 +452,11 @@ static int get_sha1_basic(const char *str, int len,
unsigned char *sha1)
if (len == 40 !get_sha1_hex(str, sha1)) {
if (warn_ambiguous_refs warn_on_object_refname_ambiguity) {
- refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
- if (refs_found 0) {
+ if (sha1_is_ambiguous_with_ref(sha1)) {
warning(warn_msg, len, str);
Re: [PATCH v3] stash: handle specifying stashes with spaces
Junio C Hamano gitster at pobox.com writes:
Thomas Rast tr at thomasrast.ch writes:
Junio C Hamano gitster at pobox.com writes:
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 at { 0 }
It seems to refer to the same as stash at {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.
I completely agree that it's brittle. I mentioned it when I submitted v1
but at the time it didn't occur to me that new tests might be added
before it. And of course I agree with your proposed changes to the test.
I must say I like Thomas' solution because of its simplicity and the
whole test could be made much shorter: just create stash and try to pop
it.
But it's seems the spaces trigger some other way of interpreting the
selector. In my git.git, git rev-parse HEAD{0} gives me the same result
as HEAD@{ 0 } but HEAD@{1} and HEAD@{ 1 } are different. Is this
intentional? If not, can this impact the reliability of the test if we
use HEAD@{ 0 } ?
Thanks for queuing!
Regards,
Øsse
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/7 Heiko Voigt hvo...@hvoigt.net: One thing is missing though (and I think thats where Francesco came from): What if the developer already has a detached HEAD in the submodule? How does he attach to a branch? For this we need something similar to Francescos attach/detach or Trevors submodule checkout with Junio's checkout HEAD~0 from here[1]. I am still undecided how we should call it. Because of my Idea for feature branch support - --- For the branch attaching feature I would also like something that can actually modify .git/config and for me more importantly .gitmodules. So e.g. if I want to work on a longer lived feature branch in a submodule which I need in a feature branch in the superproject I would do something like this: $ git submodule checkout --gitmodules --merge -b hv/my-cool-feature I said in another thread I said to Junio am not pursuing --attach|--detach anymore, but seeing that now everybody seem to be excited about attached HEAD here we are... Heiko, it's all day I think this syntax: it supports your above git submodule checkout and more. Take attention at the feature branch part! NOTE: the following seems to me compatible with Trevor's submodule.module.branch means attached patch. git submodule head The full syntax is the sum of the following ones: git submodule head [-b branch] [--attach] [--] [path...] git submodule head [-b branch] [--attach] --index [--] [path...] git submodule head --reset [--] [path...] git submodule head --reset --index [--] [path...] (NOTE: --index should be the same as Heiko's above --gitmodules, it means - touch .gitmodules) All the switches combinations follow, explained: # Attach the submodule HEAD to branch. # Also set .git/config 'submodule.module.branch' to branch $ git submodule head -b branch --attach module # Attach the submodule HEAD to 'submodule.module.branch'. # If it does not exists defaults to remote/master $ git submodule head --attach module # Unset .git/config 'submodule.module.branch' # Also attach or detach the HEAD according to what is in .gitmodules: # with Trevor's patch 'submodule.module.branch' set means attached, # unset means detached $ git submodule head --reset module NOTE: feature branch part! # Set .gitmodules 'submodule.module.branch' to branch $ git submodule head -b branch --attach --index module # Unset .gitmodules 'submodule.module.branch' $ git submodule head --reset --index module - Also note that a --detach switch is not needed with Trevor's patch. To resync to a dettached HEAD workflow, when 'submodule.module.branch' is unset in .gitmodule, --reset (without --index) should be enough. What do you think? Better? Thank you, Francesco -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote:
# Attach the submodule HEAD to branch.
# Also set .git/config 'submodule.module.branch' to branch
$ git submodule head -b branch --attach module
I prefer submodule.name.local-branch for the submodule's local
branch name. I also prefer 'checkout' to 'head', because 'checkout'
already exists in non-submodule Git for switching between local
branches.
# Attach the submodule HEAD to 'submodule.module.branch'.
# If it does not exists defaults to remote/master
$ git submodule head --attach module
Defaulting to the configured local branch is fine, but I think it
should default to 'master' if no local branch is configured. This
should not have anything to do with remote-tracking branches (that's
what 'submodule update' already handles). I don't understand why
remote-tracking-branch integration keeps getting mixed up with
local-branch checkout.
# Unset .git/config 'submodule.module.branch'
# Also attach or detach the HEAD according to what is in .gitmodules:
# with Trevor's patch 'submodule.module.branch' set means attached,
# unset means detached
$ git submodule head --reset module
To me this reads “always detach HEAD” (because it unsets
submodule.name.branch, and submodule.name.branch unset means
detached). Note that I've moved away from “submodule.name.branch
set means attached” towards “we should set per-superproject-branch
submodule.name.local-branch explicitly” [1].
NOTE: feature branch part!
# Set .gitmodules 'submodule.module.branch' to branch
$ git submodule head -b branch --attach --index module
# Unset .gitmodules 'submodule.module.branch'
$ git submodule head --reset --index module
-
These are just manipulating .gitmodules. I think we also need
per-superproject-branch configs under the superproject's .git/ for
developer overrides.
What do you think? Better?
I don't think so. To elaborate the idea I sketched out here [2], say
you want:
Superproject branch Submodule branch Upstream branch
=== ===
master mastermaster
super-featuremastermaster
my-feature my-featuremaster
other-featureother-feature other-feature
That's only going to work with per-superproject-branch configs for
both the local and remote branches. Using the same name for both
local and remote branches does not work.
Let me motivate each of the combinations in the above table:
* master, master, master: The stable trunk.
* super-feature, master, master: A superproject feature that works
with the stock submodule.
* my-feature, my-feature, master: A superproject feature that needs an
improved submodule, but wants to integrate upstream master changes
during development.
* other-feature, other-feature, other-feature: A superproject feature
that needs an improved submodule, and wants to integrate
other-feature changes that are also being developed upstream.
The deal-breaker for recycling submodule.name.branch to also mean
the local branch name is the {my-feature, my-feature, master} case.
Do we want to force submodule developers to always match the upstream
name of the feature branch they'd like to integrate with? What if
there is no upstream my-feature branch (and the superproject developer
pushes submodule branches upstream via email)? Making the local
branch name independently configurable avoids these issues with a
minimal increase in complexity.
Cheers,
Trevor
[1]: http://article.gmane.org/gmane.comp.version-control.git/240177
[2]: http://article.gmane.org/gmane.comp.version-control.git/240180
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
signature.asc
Description: OpenPGP digital signature
Re: Re: [RFC v2] submodule: Respect requested branch on all clones
2014/1/8 W. Trevor King wk...@tremily.us: Note that I've moved away from “submodule.name.branch set means attached” towards “we should set per-superproject-branch submodule.name.local-branch explicitly” [1]. Honestly, I'm having an hard time to follow this thread. Also, you didn't update the patch. If you were endorsed by someone (Junio, Heiko, ...) for the submodule.name.local-branch feature please show me where. I somehow understand the point of the submodule.name.local-branch property, but I can't see the the workflow. Please, show me some hypothetical scripting example with as much complete as possible workflow (creation, developer update, mantainers creates feature branch, developer update, developer attach to another branch). Also, consider I proposed to support the attached HEAD path to reduce complexity and support a simpler use case for git submodules. I would be disappointed if the complexity is reduced in a way and augmented in another. On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote: # Attach the submodule HEAD to branch. # Also set .git/config 'submodule.module.branch' to branch $ git submodule head -b branch --attach module [...] I also prefer 'checkout' to 'head', because 'checkout' already exists in non-submodule Git for switching between local branches. I can agree with similarity to other git commands, but 'checkout' does not give me the idea of something that writes to .git/config or .gitmodules. # Attach the submodule HEAD to 'submodule.module.branch'. # If it does not exists defaults to remote/master $ git submodule head --attach module Defaulting to the configured local branch is fine, but I think it should default to 'master' if no local branch is configured. This should not have anything to do with remote-tracking branches (that's what 'submodule update' already handles). I don't understand why remote-tracking-branch integration keeps getting mixed up with local-branch checkout. Yep, it should default to master, my fault. # Unset .git/config 'submodule.module.branch' # Also attach or detach the HEAD according to what is in .gitmodules: # with Trevor's patch 'submodule.module.branch' set means attached, # unset means detached $ git submodule head --reset module To me this reads “always detach HEAD” (because it unsets submodule.name.branch, and submodule.name.branch unset means detached). I disagree: this would remove only the value in .git/config. If the value is till present in .gitmodules, as I wrote above, the behavior of what is in the index should be respected as for the other properties. Also it gives a nice meaning to a switch like --reset : return to how it was before. NOTE: feature branch part! # Set .gitmodules 'submodule.module.branch' to branch $ git submodule head -b branch --attach --index module # Unset .gitmodules 'submodule.module.branch' $ git submodule head --reset --index module - These are just manipulating .gitmodules. I think we also need per-superproject-branch configs under the superproject's .git/ for developer overrides. I disagree: in my idea the --index switch is a maintainer only command to modify the behavior of the developers and touch only indexed files (.gitmodules, or create a new submodule branch). It expressly don't touch .git/config. -- To unsubscribe from this list: send the line 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: Preferred local submodule branches
On Wed, Jan 08, 2014 at 03:12:44AM +0100, Francesco Pretto wrote:
2014/1/8 W. Trevor King wk...@tremily.us:
Note that I've moved away from “submodule.name.branch
set means attached” towards “we should set per-superproject-branch
submodule.name.local-branch explicitly” [1].
Honestly, I'm having an hard time to follow this thread.
I tried to refocus things (with a new subject) in this sub-thread.
Hopefully that helps make the discussion more linear ;).
Also, you didn't update the patch.
I'm waiting [1] to see how the C-level checkout by Jens and Jonathan
progresses [2,3] before writing more code.
If you were endorsed by someone (Junio, Heiko, ...) for the
submodule.name.local-branch feature please show me where.
As far as I know, no-one else has endorsed this idea (yet :). Heiko
has expressed concern [4], but not convincingly enough (yet :) to win
me over ;).
I somehow understand the point of the
submodule.name.local-branch property, but I can't see the the
workflow. Please, show me some hypothetical scripting example with
as much complete as possible workflow (creation, developer update,
mantainers creates feature branch, developer update, developer
attach to another branch).
I've put this at the bottom of the message to avoid bothering the
tl;dr crowd, although they have probably long since tuned us out ;).
Also, consider I proposed to support the attached HEAD path to
reduce complexity and support a simpler use case for git
submodules. I would be disappointed if the complexity is reduced in
a way and augmented in another.
Agreed. I think we're all looking for the least-complex solution that
covers all (or most) reasonable workflows.
On Wed, Jan 08, 2014 at 01:17:49AM +0100, Francesco Pretto wrote:
# Attach the submodule HEAD to branch.
# Also set .git/config 'submodule.module.branch' to branch
$ git submodule head -b branch --attach module
[...]
I also prefer 'checkout' to 'head', because 'checkout'
already exists in non-submodule Git for switching between local
branches.
I can agree with similarity to other git commands, but 'checkout'
does not give me the idea of something that writes to .git/config
or .gitmodules.
Neither does 'head'. We have precedence in 'git submodule add' for
embracing and extending a core git command with additional .gitmodules
manipulation. I think it's easier to pick up the submodule jargon
when we add submodule-specific side-effects to submodule-specific
commands named after their core analogs than it would be if we pick
unique names for the submodule-specific commands.
# Unset .git/config 'submodule.module.branch'
# Also attach or detach the HEAD according to what is in .gitmodules:
# with Trevor's patch 'submodule.module.branch' set means attached,
# unset means detached
$ git submodule head --reset module
To me this reads “always detach HEAD” (because it unsets
submodule.name.branch, and submodule.name.branch unset means
detached).
I disagree: this would remove only the value in .git/config. If the
value is till present in .gitmodules, as I wrote above, the behavior
of what is in the index should be respected as for the other
properties. Also it gives a nice meaning to a switch like --reset :
return to how it was before.
Ah, that makes more sense. I had confused .git/config with
“.gitmodules and .git/config”.
NOTE: feature branch part!
# Set .gitmodules 'submodule.module.branch' to branch
$ git submodule head -b branch --attach --index module
# Unset .gitmodules 'submodule.module.branch'
$ git submodule head --reset --index module
-
These are just manipulating .gitmodules. I think we also need
per-superproject-branch configs under the superproject's .git/ for
developer overrides.
I disagree: in my idea the --index switch is a maintainer only command
to modify the behavior of the developers and touch only indexed files
(.gitmodules, or create a new submodule branch). It expressly don't
touch .git/config.
Something that just touches the config files is syntactic sugar, so I
avoided a more detailed review and moved on to address what I saw as a
more fundamental issue (preferred submodule local branches on a
per-superproject-branch level).
Here's a detailed workflow for the {my-feature, my-feature, master}
example I roughed out before [5].
# create the subproject
mkdir subproject
(
cd subproject
git init
echo 'Hello, world' README
git add README
git commit -m 'Subproject v1'
)
# create the superproject
mkdir superproject
(
cd superproject
git init
git submodule add ../subproject submod
git config -f .gitmodules submodule.submod.update merge
git commit -am 'Superproject v1'
( # 'submodule update' doesn't look in .gitmodules (yet [6]) for a
# default update mode. Copy submodule.submod.update over to
#
[PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion
On Tue, Jan 07, 2014 at 06:58:50PM -0500, Jeff King wrote:
+ if (flags DO_FOR_EACH_NO_RECURSE) {
+ struct ref_dir *subdir = get_ref_dir(entry);
+ sort_ref_dir(subdir);
+ retval = do_for_each_entry_in_dir(subdir, 0,
Obviously this is totally wrong and inverts the point of the flag. And
causes something like half of the test suite to fail.
Michael was nice enough to point it out to me off-list, but well, I have
to face the brown paper bag at some point. :) In my defense, it was a
last minute refactor before going to dinner. That is what I get for
rushing out the series.
Here's a fixed version of patch 3/5.
-- 8 --
Subject: refs: teach for_each_ref a flag to avoid recursion
The normal for_each_ref traversal descends into
subdirectories, returning each ref it finds. However, in
some cases we may want to just iterate over the top-level of
a certain part of the tree.
The introduction of the flags option is a little
mysterious. We already have a flags option that gets stuck
in a callback struct and ends up interpreted in do_one_ref.
But the traversal itself does not currently have any flags,
and it needs to know about this new flag.
We _could_ introduce this as a completely separate flag
parameter. But instead, we simply put both flag types into a
single namespace, and make it available at both sites. This
is simple, and given that we do not have a proliferation of
flags (we have had exactly one until now), it is probably
sufficient.
Signed-off-by: Jeff King p...@peff.net
---
refs.c | 61 ++---
1 file changed, 38 insertions(+), 23 deletions(-)
diff --git a/refs.c b/refs.c
index 3926136..b70b018 100644
--- a/refs.c
+++ b/refs.c
@@ -589,6 +589,8 @@ static void sort_ref_dir(struct ref_dir *dir)
/* Include broken references in a do_for_each_ref*() iteration: */
#define DO_FOR_EACH_INCLUDE_BROKEN 0x01
+/* Do not recurse into subdirs, just iterate at a single level. */
+#define DO_FOR_EACH_NO_RECURSE 0x02
/*
* Return true iff the reference described by entry can be resolved to
@@ -661,7 +663,8 @@ static int do_one_ref(struct ref_entry *entry, void
*cb_data)
* called for all references, including broken ones.
*/
static int do_for_each_entry_in_dir(struct ref_dir *dir, int offset,
- each_ref_entry_fn fn, void *cb_data)
+ each_ref_entry_fn fn, void *cb_data,
+ int flags)
{
int i;
assert(dir-sorted == dir-nr);
@@ -669,9 +672,13 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir,
int offset,
struct ref_entry *entry = dir-entries[i];
int retval;
if (entry-flag REF_DIR) {
- struct ref_dir *subdir = get_ref_dir(entry);
- sort_ref_dir(subdir);
- retval = do_for_each_entry_in_dir(subdir, 0, fn,
cb_data);
+ if (!(flags DO_FOR_EACH_NO_RECURSE)) {
+ struct ref_dir *subdir = get_ref_dir(entry);
+ sort_ref_dir(subdir);
+ retval = do_for_each_entry_in_dir(subdir, 0,
+ fn, cb_data,
+ flags);
+ }
} else {
retval = fn(entry, cb_data);
}
@@ -691,7 +698,8 @@ static int do_for_each_entry_in_dir(struct ref_dir *dir,
int offset,
*/
static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
struct ref_dir *dir2,
-each_ref_entry_fn fn, void *cb_data)
+each_ref_entry_fn fn, void *cb_data,
+int flags)
{
int retval;
int i1 = 0, i2 = 0;
@@ -702,10 +710,12 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
struct ref_entry *e1, *e2;
int cmp;
if (i1 == dir1-nr) {
- return do_for_each_entry_in_dir(dir2, i2, fn, cb_data);
+ return do_for_each_entry_in_dir(dir2, i2, fn, cb_data,
+ flags);
}
if (i2 == dir2-nr) {
- return do_for_each_entry_in_dir(dir1, i1, fn, cb_data);
+ return do_for_each_entry_in_dir(dir1, i1, fn, cb_data,
+ flags);
}
e1 = dir1-entries[i1];
e2 = dir2-entries[i2];
@@ -713,12 +723,15 @@ static int do_for_each_entry_in_dirs(struct ref_dir *dir1,
if (cmp == 0) {
if
Re: Preferred local submodule branches
On Tue, Jan 07, 2014 at 07:47:08PM -0800, W. Trevor King wrote: #git checkout --recurse-submodules master ( # 'git checkout --recurse-submodules' doesn't exist yet [2,3]. # Even with that patch, 'git checkout' won't respect # submodule.name.local-branch without further work. git checkout master cd submod git checkout master # don't pull in our my-feature work ) git submodule update --remote git commit -am 'Catch submod up with Subproject v2' # update the my-feature branch git checkout my-feature ( # 'git checkout' doesn't mess with submodules cd submod git checkout my-feature ) Oops, the my-feature checkout block should have been: #git checkout --recurse-submodules my-feature ( # 'git checkout --recurse-submodules' doesn't exist yet... git checkout my-feature cd submod git checkout my-feature ) mirroring the earlier master checkout block. Sorry for the sloppy editing. Cheers, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
Re: It seems there is a very tight character count limit in .gitconfig
On Wed, Jan 08, 2014 at 02:59:37PM +0800, Li Zhang wrote: I tried to add url xxx insteadof xxx in .gitconfig. If the length of url exceed 125, git will not work. I am using Ubuntu. The default version is 1.7.9.5. Maybe the latest version solve this already. Yes, this was fixed in 0971e99 (Remove the hard coded length limit on variable names in config files, 2012-09-30). Git v1.8.0.1 and higher contain that commit. -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
It seems there is a very tight character count limit in .gitconfig
I tried to add url xxx insteadof xxx in .gitconfig. If the length of url exceed 125, git will not work. I am using Ubuntu. The default version is 1.7.9.5. Maybe the latest version solve this already. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
