Re: sys/param.h
On Tue, Dec 18, 2012 at 6:01 PM, Junio C Hamano gits...@pobox.com wrote: Johannes Sixt j.s...@viscovery.net writes: Junio C Hamano wrote: It could turn out that we may be able to get rid of sys/param.h altogether, but one step at a time. Inputs from people on minority platforms are very much appreciated---does your platform build fine when the inclusion of the file is removed from git-compat-util.h? MinGW works fine with sys/param.h removed from git-compat-util.h. It seems that OpenBSD 5.2 does not mind it getting removed, either. Debian 5 and Debian 6 seem OK; so do Ubuntu 10.04 and 12.04. I have a hunch that Fedora or anything based on glibc would be fine, too. And just to be sure; Fedora 17: OK. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Cannot push some grafted branches
On Tue, 18 Dec 2012 08:09:35 -0800 Junio C Hamano gits...@pobox.com wrote: Yann Dirson dir...@bertin.fr writes: On Mon, 17 Dec 2012 13:14:56 -0800 Junio C Hamano gits...@pobox.com wrote: Andreas Schwab sch...@linux-m68k.org writes: Christian Couder christian.cou...@gmail.com writes: Yeah, at one point I wanted to have a command that created to craft a new commit based on an existing one. This isn't hard to do, you only have to resort to plumbing: $ git cat-file commit fef11965da875c105c40f1a9550af1f5e34a6e62 | sed s/bfae342c973b0be3c9e99d3d86ed2e6b152b4a6b/790c83cda92f95f1b4b91e2ddc056a52a99a055d/ | git hash-object -t commit --stdin -w bb45cc6356eac6c7fa432965090045306dab7026 Good. I do not think an extra special-purpose command is welcome here. Well, I'm not sure this is intuitive enough to be useful to the average user :) I do not understand why you even want to go in the harder route in the first place, only to complicate things? Although the approach you propose is elegant, it still looks like one could not leave the worktree untouched in the case of creating a merge replace, which the just forge an arbitrary commit approach handles easily. It seems the latter would also be more powerful, in that you can create new commits with an arbitrary number of parents, even when merge-octopus would simply refuse to help; and it is has no special case for creating merges. Is this not intuitive enough? I would say it is a nice read that can help an advanced user to earn some XP - but well, replace refs are also meant for somewhat advanced users :) -- Yann Dirson - Bertin Technologies -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: problem with BOINC repository and CR/LF
On 12/18/2012 01:15 PM, Torsten Bögershausen wrote: HTH /Torsten Thx Torsten - I forwarded this answer (and all the other answers) to the boinc alpha mailing list - there's now a discussion about that. -- MfG/Sincerely Toralf Förster pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: problem with BOINC repository and CR/LF
On 12/18/2012 05:41 PM, Jeff King wrote: I could reproduce it, too, on Linux. The reason it does not always happen is that git will not re-examine the file content unless the timestamp on the file is older than what's in the index. So it is a race condition for git to see whether the file is stat-dirty. Ah - /me was wondering why sometimes (but rarely) I could not exactly reproduce the problem and was really wondering if the underlying file system (ext4) would give an extra layer of trouble or not. Thx for that explanation. -- MfG/Sincerely Toralf Förster pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Cannot push some grafted branches
On Wed, Dec 19, 2012 at 08:13:21AM +0100, Johannes Sixt wrote: Am 12/18/2012 17:24, schrieb Jeff King: I am not really interested in pushing this forward myself, but I worked up this toy that somebody might find interesting (you can git replace HEAD~20 to get dumped in an editor). It should probably handle trees, and it would probably make sense to do per-object-type sanity checks (e.g., call verify_tag on tags). I know it's just a throw-away patch, but I would discourage to go this route without also adding all the sanity checks. Otherwise, it will have just created a porcelain command that can generate a commit object with any content you want! I think I agree with you that it would not be worth doing without sanity checks. I am not sure if your any content you want statement means bad people can easily make bogus objects or it is too easy to make arbitrary mistakes, putting your repo in a bogus state. I would agree that the latter is compelling, but not the former. You can already easily generate a commit with any content you want via hash-object -t commit, and I have frequently done this while testing corner cases of fsck, how git behaves when given buggy data, etc. So to me it is not about preventing intentional abuse, but about not promoting a feature that makes it too easy to screw up. -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: [BUG] Cannot push some grafted branches
Junio C Hamano gits...@pobox.com writes:
I do not understand why you even want to go in the harder route in
the first place, only to complicate things?
All you want to do is to craft a commit object that records a
specific tree shape, has a set of parents you want, and has the log
information you want. Once you have the commit, you can replace an
unwanted commit with it.
[...]
$ git checkout X^0 ;# detach
$ git reset --soft A
$ git commit -C X
[...]
Is this not intuitive enough?
I still wouldn't recommend this approach in git-replace(1) for several
reasons:
* It does not generalize in any direction. For each field you may want
to change, you have to know a _specific_ way of getting just the
commit you want.
* More to the point of replacing the parent lists, while the above might
be expected of a slightly advanced git user, you get into deep magic
the second you want to fake a merge commit with an arbitrary
combination of parents. (No, you don't need to tell me how. I'm just
saying that fooling with either MERGE_HEAD or read-tree is not for
mere mortals.)
* The above potentially introduces clock skew into the repository, which
can trigger bugs (like rev-list accidentally missing out on some side
arm!) until we get around to implementing and using generation
numbers.
--
Thomas Rast
trast@{inf,student}.ethz.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] t9020: use configured Python to run test helper
gits...@pobox.com wrote on Tue, 18 Dec 2012 20:49 -0800: The test helper svnrdump_sim.py is used as svnrdump during the execution of this test, but the arrangement had a few undesirable things: - it relied on symbolic links; - unportable export VAR=VAL was used; - GIT_BUILD_DIR variable was not quoted correctly; - it assumed that the Python interpreter is in /usr/bin/ and called python (i.e. not python2.7 etc.) Rework this by writing a small shell script that spawns the right Python interpreter, using the right quoting. Signed-off-by: Junio C Hamano gits...@pobox.com --- * The analysis above counts more bugs than the number of lines that are deleted in this section of the code... t/t9020-remote-svn.sh | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/t/t9020-remote-svn.sh b/t/t9020-remote-svn.sh index 4f2dfe0..d7be66a 100755 --- a/t/t9020-remote-svn.sh +++ b/t/t9020-remote-svn.sh @@ -12,9 +12,13 @@ then test_done fi -# We override svnrdump by placing a symlink to the svnrdump-emulator in . -export PATH=$HOME:$PATH -ln -sf $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py $HOME/svnrdump +# Override svnrdump with our simulator +PATH=$HOME:$PATH +export PATH PYTHON_PATH GIT_BUILD_DIR + +write_script $HOME/svnrdump \EOF +exec $PYTHON_PATH $GIT_BUILD_DIR/contrib/svn-fe/svnrdump_sim.py $@ +EOF You don't really need to export PYTHON_PATH and GIT_BUILD_DIR if you get them expanded in the svnrdump script wrapper. Unquote the EOF but add \ for $@. Either way it's a nice improvement, especially with the bugs/lines metric being 1. -- Pete -- To unsubscribe from this list: send the line unsubscribe 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] t0200: locale may not exist
On Tue, Dec 18, 2012 at 10:47:03PM -0800, Junio C Hamano wrote:
On systems without locale installed, t0200-gettext-basic.sh leaked
error messages when checking if some test locales are available.
Hide them, as they are not very useful.
Obviously correct, though there is another way:
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..ae8883a 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -14,12 +14,14 @@ export GIT_TEXTDOMAINDIR GIT_PO_PATH
if test_have_prereq GETTEXT ! test_have_prereq GETTEXT_POISON
If we turn this line into:
test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
then people can see the error output of the setup step in verbose mode.
Annoyingly, though, it means tweaking the quoting throughout the block
to handle embedded single-quotes (if there is one feature I could take
from perl back into shell, it would be arbitrary quote delimiters).
Patch is below. I don't know if it is worth the complexity.
-Peff
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..d962c00 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -11,18 +11,17 @@ then
. $GIT_BUILD_DIR/git-sh-i18n
-if test_have_prereq GETTEXT ! test_have_prereq GETTEXT_POISON
-then
+test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
- is_IS_locale=$(locale -a | sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
+ is_IS_locale=$(locale -a | sed -n /^is_IS\.[uU][tT][fF]-*8\$/{
p
q
- }')
+ })
# is_IS.ISO8859-1 on Solaris and FreeBSD, is_IS.iso88591 on Debian
- is_IS_iso_locale=$(locale -a | sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
+ is_IS_iso_locale=$(locale -a | sed -n /^is_IS\.[iI][sS][oO]8859-*1\$/{
p
q
- }')
+ })
# Export them as an environment variable so the t0202/test.pl Perl
# test can use it too
@@ -37,7 +36,7 @@ then
# Exporting for t0202/test.pl
GETTEXT_LOCALE=1
export GETTEXT_LOCALE
- say # lib-gettext: Found '$is_IS_locale' as an is_IS UTF-8
locale
+ say # lib-gettext: Found \$is_IS_locale\ as an is_IS UTF-8
locale
else
say # lib-gettext: No is_IS UTF-8 locale available
fi
@@ -48,8 +47,8 @@ then
# Some of the tests need the reference Icelandic locale
test_set_prereq GETTEXT_ISO_LOCALE
- say # lib-gettext: Found '$is_IS_iso_locale' as an is_IS
ISO-8859-1 locale
+ say # lib-gettext: Found \$is_IS_iso_locale\ as an is_IS
ISO-8859-1 locale
else
say # lib-gettext: No is_IS ISO-8859-1 locale available
fi
-fi
+'
--
To unsubscribe from this list: send the line unsubscribe 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/WIP 0/3] Bye bye fnmatch()
On Wed, Dec 19, 2012 at 8:08 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote: For those who have not followed, nd/wildmatch brings another fnmatch-like implementation which can nearly replace fnmatch. System fnmatch() seems to behave differently in some cases. It's better to stay away and use one implementation for all. Oh I forgot. Case-insensitive matching will be available to everybody. On the other hand it'll be a lot more work to (implement and) use other FNM_* flags like FNM_PERIOD. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] wildmatch: make dowild() take arbitrary flags
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
wildmatch.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/wildmatch.c b/wildmatch.c
index 3972e26..9586ed9 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -55,7 +55,7 @@ typedef unsigned char uchar;
#define ISXDIGIT(c) (ISASCII(c) isxdigit(c))
/* Match pattern p against text */
-static int dowild(const uchar *p, const uchar *text, int force_lower_case)
+static int dowild(const uchar *p, const uchar *text, int flags)
{
uchar p_ch;
@@ -64,9 +64,9 @@ static int dowild(const uchar *p, const uchar *text, int
force_lower_case)
uchar t_ch, prev_ch;
if ((t_ch = *text) == '\0' p_ch != '*')
return ABORT_ALL;
- if (force_lower_case ISUPPER(t_ch))
+ if (flags FNM_CASEFOLD ISUPPER(t_ch))
t_ch = tolower(t_ch);
- if (force_lower_case ISUPPER(p_ch))
+ if (flags FNM_CASEFOLD ISUPPER(p_ch))
p_ch = tolower(p_ch);
switch (p_ch) {
case '\\':
@@ -100,7 +100,7 @@ static int dowild(const uchar *p, const uchar *text, int
force_lower_case)
* both foo/bar and foo/a/bar.
*/
if (p[0] == '/'
- dowild(p + 1, text,
force_lower_case) == MATCH)
+ dowild(p + 1, text, flags) == MATCH)
return MATCH;
special = TRUE;
} else
@@ -119,7 +119,7 @@ static int dowild(const uchar *p, const uchar *text, int
force_lower_case)
while (1) {
if (t_ch == '\0')
break;
- if ((matched = dowild(p, text,
force_lower_case)) != NOMATCH) {
+ if ((matched = dowild(p, text, flags)) !=
NOMATCH) {
if (!special || matched !=
ABORT_TO_STARSTAR)
return matched;
} else if (!special t_ch == '/')
@@ -229,6 +229,5 @@ static int dowild(const uchar *p, const uchar *text, int
force_lower_case)
/* Match the pattern against the text string. */
int wildmatch(const char *pattern, const char *text, int flags)
{
- return dowild((const uchar*)pattern, (const uchar*)text,
- flags FNM_CASEFOLD ? 1 :0);
+ return dowild((const uchar*)pattern, (const uchar*)text, flags);
}
--
1.8.0.rc2.23.g1fb49df
--
To unsubscribe from this list: send the line 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/3] wildmatch: support no FNM_PATHNAME mode
By default wildmatch(,, 0) is equivalent with fnmatch(,, FNM_PATHNAME).
This patch makes wildmatch behave more like fnmatch: FNM_PATHNAME
behavior is always applied when FNM_PATHNAME is passed to
wildmatch. Without FNM_PATHNAME, wildmatch accepts '/' in '?' and '[]'
and treats '*' like '**' in the original version.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
The choice of name pathspec is not good. I couldn't think of
anything appropriate and just did not care enough at this point.
dir.c| 2 +-
t/t3070-wildmatch.sh | 27 +++
test-wildmatch.c | 4 +++-
wildmatch.c | 12
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index cb7328b..7bbd6f8 100644
--- a/dir.c
+++ b/dir.c
@@ -595,7 +595,7 @@ int match_pathname(const char *pathname, int pathlen,
}
return wildmatch(pattern, name,
-ignore_case ? FNM_CASEFOLD : 0) == 0;
+FNM_PATHNAME | (ignore_case ? FNM_CASEFOLD : 0)) == 0;
}
/* Scan the list and let the last match determine the fate.
diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index 3155eab..ca4ac46 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -29,6 +29,18 @@ match() {
fi
}
+pathspec() {
+if [ $1 = 1 ]; then
+ test_expect_success pathspec:match '$2' '$3'
+ test-wildmatch pathspec '$2' '$3'
+
+else
+ test_expect_success pathspec: no match '$2' '$3'
+ ! test-wildmatch pathspec '$2' '$3'
+
+fi
+}
+
# Basic wildmat features
match 1 1 foo foo
match 0 0 foo bar
@@ -192,4 +204,19 @@ match 0 0
'XXX/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 'XXX/*/
match 1 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
match 0 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txtz' '**/*a*b*g*n*t'
+pathspec 1 foo foo
+pathspec 0 foo fo
+pathspec 1 foo/bar foo/bar
+pathspec 1 foo/bar 'foo/*'
+pathspec 1 foo/bba/arr 'foo/*'
+pathspec 1 foo/bba/arr 'foo/**'
+pathspec 1 foo/bba/arr 'foo*'
+pathspec 1 foo/bba/arr 'foo**'
+pathspec 1 foo/bba/arr 'foo/*arr'
+pathspec 1 foo/bba/arr 'foo/**arr'
+pathspec 0 foo/bba/arr 'foo/*z'
+pathspec 0 foo/bba/arr 'foo/**z'
+pathspec 1 foo/bar 'foo?bar'
+pathspec 1 foo/bar 'foo[/]bar'
+
test_done
diff --git a/test-wildmatch.c b/test-wildmatch.c
index e384c8e..7fefa4f 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -12,9 +12,11 @@ int main(int argc, char **argv)
argv[i] += 3;
}
if (!strcmp(argv[1], wildmatch))
+ return !!wildmatch(argv[3], argv[2], FNM_PATHNAME);
+ else if (!strcmp(argv[1], pathspec))
return !!wildmatch(argv[3], argv[2], 0);
else if (!strcmp(argv[1], iwildmatch))
- return !!wildmatch(argv[3], argv[2], FNM_CASEFOLD);
+ return !!wildmatch(argv[3], argv[2], FNM_PATHNAME |
FNM_CASEFOLD);
else if (!strcmp(argv[1], fnmatch))
return !!fnmatch(argv[3], argv[2], FNM_PATHNAME);
else
diff --git a/wildmatch.c b/wildmatch.c
index 9586ed9..6aa034f 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -80,14 +80,17 @@ static int dowild(const uchar *p, const uchar *text, int
flags)
continue;
case '?':
/* Match anything but '/'. */
- if (t_ch == '/')
+ if (flags FNM_PATHNAME t_ch == '/')
return NOMATCH;
continue;
case '*':
if (*++p == '*') {
const uchar *prev_p = p - 2;
while (*++p == '*') {}
- if ((prev_p == text || *prev_p == '/') ||
+ if (!(flags FNM_PATHNAME))
+ /* without FNM_PATHNAME, '*' == '**' */
+ special = TRUE;
+ else if ((prev_p == text || *prev_p == '/') ||
(*p == '\0' || *p == '/' ||
(p[0] == '\\' p[1] == '/'))) {
/*
@@ -106,7 +109,7 @@ static int dowild(const uchar *p, const uchar *text, int
flags)
} else
return ABORT_MALFORMED;
} else
- special = FALSE;
+ special = flags FNM_PATHNAME ? FALSE: TRUE;
if (*p == '\0') {
/* Trailing ** matches everything. Trailing
* matches
* only if there are no more slash characters.
*/
@@ -217,7 +220,8 @@ static int dowild(const uchar *p, const uchar *text, int
flags)
} else
Re: [PATCH] t0200: locale may not exist
Jeff King p...@peff.net writes:
On Tue, Dec 18, 2012 at 10:47:03PM -0800, Junio C Hamano wrote:
On systems without locale installed, t0200-gettext-basic.sh leaked
error messages when checking if some test locales are available.
Hide them, as they are not very useful.
Obviously correct, though there is another way:
diff --git a/t/lib-gettext.sh b/t/lib-gettext.sh
index 0f76f6c..ae8883a 100644
--- a/t/lib-gettext.sh
+++ b/t/lib-gettext.sh
@@ -14,12 +14,14 @@ export GIT_TEXTDOMAINDIR GIT_PO_PATH
if test_have_prereq GETTEXT ! test_have_prereq GETTEXT_POISON
If we turn this line into:
test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
then people can see the error output of the setup step in verbose mode.
Ok, so it was not obviously correct after all ;-)
+test_expect_success GETTEXT,!GETTEXT_POISON 'setup locale' '
# is_IS.UTF-8 on Solaris and FreeBSD, is_IS.utf8 on Debian
- is_IS_locale=$(locale -a | sed -n '/^is_IS\.[uU][tT][fF]-*8$/{
+ is_IS_locale=$(locale -a | sed -n /^is_IS\.[uU][tT][fF]-*8\$/{
Do we need to do this \$?
p
q
- }')
+ })
# is_IS.ISO8859-1 on Solaris and FreeBSD, is_IS.iso88591 on Debian
- is_IS_iso_locale=$(locale -a | sed -n '/^is_IS\.[iI][sS][oO]8859-*1$/{
+ is_IS_iso_locale=$(locale -a | sed -n /^is_IS\.[iI][sS][oO]8859-*1\$/{
p
q
- }')
+ })
# Export them as an environment variable so the t0202/test.pl Perl
# test can use it too
@@ -37,7 +36,7 @@ then
# Exporting for t0202/test.pl
GETTEXT_LOCALE=1
export GETTEXT_LOCALE
- say # lib-gettext: Found '$is_IS_locale' as an is_IS UTF-8
locale
+ say # lib-gettext: Found \$is_IS_locale\ as an is_IS UTF-8
locale
'\''?
--
To unsubscribe from this list: send the line 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 v8 0/3] submodule update: add --remote for submodule's upstream changes
From: W. Trevor King wk...@tremily.us Comments on v7 seem to have petered out, so here's v8. Changes since v7: * Series based on gitster/master instead of v1.8.0. * In Documentation/config.txt, restored trailing line of submodule.name.update documentation, which I had accidentally removed in v7. * In Documentation/git-submodule.txt, make --no-fetch example in the --remote description more general, following Phil's suggestion. * In git-submodule.sh: * Remove accidental ges line. * Use the submodule's default remote to determine which tracking branch to fetch. In v7 I'd been using the superproject's default remote. * In cmd_add(), use sm_name instead of sm_path to store the --branch option (catching up with 73b0898). W. Trevor King (3): submodule: add get_submodule_config helper funtion submodule update: add --remote for submodule's upstream changes submodule add: If --branch is given, record it in .gitmodules Documentation/config.txt| 6 + Documentation/git-submodule.txt | 25 +++- Documentation/gitmodules.txt| 5 git-submodule.sh| 51 - t/t7400-submodule-basic.sh | 1 + t/t7406-submodule-update.sh | 31 + 6 files changed, 117 insertions(+), 2 deletions(-) -- 1.8.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] wildmatch: make dowild() take arbitrary flags
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
wildmatch.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/wildmatch.c b/wildmatch.c
index 3972e26..9586ed9 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -55,7 +55,7 @@ typedef unsigned char uchar;
#define ISXDIGIT(c) (ISASCII(c) isxdigit(c))
/* Match pattern p against text */
-static int dowild(const uchar *p, const uchar *text, int force_lower_case)
+static int dowild(const uchar *p, const uchar *text, int flags)
It may be better to declare a bitset like this unsigned.
{
uchar p_ch;
@@ -64,9 +64,9 @@ static int dowild(const uchar *p, const uchar *text, int
force_lower_case)
uchar t_ch, prev_ch;
if ((t_ch = *text) == '\0' p_ch != '*')
return ABORT_ALL;
- if (force_lower_case ISUPPER(t_ch))
+ if (flags FNM_CASEFOLD ISUPPER(t_ch))
Please add parentheses around bitwise-AND that is used as a boolean,
i.e.
if ((flags FNM_CASEFOLD) ISUPPER(t_ch))
Less chance of confusion.
--
To unsubscribe from this list: send the line 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] compat/fnmatch: update old-style definition to ANSI
We usually try to avoid touching borrowed code, but we encourage
people to code without old-style definition these days and compile
with -Werror, and on platforms that need to use NO_FNMATCH, these
three functions make the compilation fail.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
compat/fnmatch/fnmatch.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
index 0ff1d27..b8b7dc2 100644
--- a/compat/fnmatch/fnmatch.c
+++ b/compat/fnmatch/fnmatch.c
@@ -135,9 +135,9 @@ extern int errno;
# if !defined HAVE___STRCHRNUL !defined _LIBC
static char *
-__strchrnul (s, c)
- const char *s;
- int c;
+__strchrnul (const char *s, int c)
+
+
{
char *result = strchr (s, c);
if (result == NULL)
@@ -159,11 +159,11 @@ static int internal_fnmatch __P ((const char *pattern,
const char *string,
internal_function;
static int
internal_function
-internal_fnmatch (pattern, string, no_leading_period, flags)
- const char *pattern;
- const char *string;
- int no_leading_period;
- int flags;
+internal_fnmatch (const char *pattern, const char *string, int
no_leading_period, int flags)
+
+
+
+
{
register const char *p = pattern, *n = string;
register unsigned char c;
@@ -481,10 +481,10 @@ internal_fnmatch (pattern, string, no_leading_period,
flags)
int
-fnmatch (pattern, string, flags)
- const char *pattern;
- const char *string;
- int flags;
+fnmatch (const char *pattern, const char *string, int flags)
+
+
+
{
return internal_fnmatch (pattern, string, flags FNM_PERIOD, flags);
}
--
1.8.1.rc2.196.g654d69e
--
To unsubscribe from this list: send the line unsubscribe 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/3] wildmatch: support no FNM_PATHNAME mode
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: By default wildmatch(,, 0) is equivalent with fnmatch(,, FNM_PATHNAME). Is this stating a fact before or after the patch? I think it is more like: So far, wildmatch() has always honoured directory boundary and there was no way to turn it off. Make it behave more like fnmatch() by requiring all callers that want the FNM_PATHNAME behaviour to pass that in the flags parameter. Callers that do not specify FNM_PATHNAME will get wildcards like ? and * in their patterns matched against '/' and ... The choice of name pathspec is not good. I couldn't think of anything appropriate and just did not care enough at this point. Well, you should, before this series leaves the WIP state. It seems that all operating modes supported by test-wildmatch are named as somethingmatch, so pathmatch may be a better candidate. diff --git a/test-wildmatch.c b/test-wildmatch.c index e384c8e..7fefa4f 100644 --- a/test-wildmatch.c +++ b/test-wildmatch.c @@ -12,9 +12,11 @@ int main(int argc, char **argv) argv[i] += 3; } if (!strcmp(argv[1], wildmatch)) + return !!wildmatch(argv[3], argv[2], FNM_PATHNAME); + else if (!strcmp(argv[1], pathspec)) return !!wildmatch(argv[3], argv[2], 0); ipathmatch to pass only FNM_CASEFOLD may be a natural extension, but I doubt we use that combination in the real life (yet). I am probably two step ahead of what is being done (read: this will be a Git 2.0 topic, if not later), but I am wondering how we'd integrate this new machinery well with the pathspec limited traversal. Traditionally, when traversing a tree limited with a pathspec, say, Documentation/*.txt, we used the simple part of the prefix and noticed that any subdirectory whose name is not Documentation can never match the pathspec and avoided descending into it. As the user can use ** to expand to any levels of subdirectory, I think the pathspec limited traversal eventually can safely (and should) use FNM_PATHNAME by default. That will allow people to say Documentation/**/*.txt to match both git.txt and howto/maintain-git.txt, without resorting to the more traditional !FNM_PATHNAME semantics Documentation/*.txt to match the latter (i.e. * matches howto/maintain-git). When that happens, we should want to retain the same do not bother to descend into subdirectories that will never match optimization for a pattern like Doc*tion/**/*.txt. Because of FNM_PATHNAME, we can tell if a subdirectory is worth descending into by looking at the not-so-simple prefix Doc*tion/; Documentation will match, Doc will not (because '*' won't match '/'). Which tells me that integrating this _well_ into the rest of the system is not just a matter of replacing fnmatch() with wildmatch(). I also expect that wildmatch() would be much slower than fnmatch() especially when doing its ** magic, but I personally do not think it will be a showstopper. If the user asks for a more powerful but expensive operation, we are saving time for the user by doing a more powerful thing (reducing the need to postprocess the results) and can afford to spend extra cycles. As long as simpler patterns fnmatch() groks (namely, '?', '*', and '[class]' wildcards only) are not slowed down by replacing it with wildmatch(), that is, of course. -- To unsubscribe from this list: send the line unsubscribe 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 v8 3/3] submodule add: If --branch is given, record it in .gitmodules
wk...@tremily.us writes: From: W. Trevor King wk...@tremily.us This allows you to easily record a submodule.name.branch option in .gitmodules when you add a new submodule. With this patch, $ git submodule add -b branch repository [path] $ git config -f .gitmodules submodule.path.branch branch reduces to $ git submodule add -b branch repository [path] This means that future calls to $ git submodule update --remote ... will get updates from the same branch that you used to initialize the submodule, which is usually what you want. I agree that it would usually be what you want when you are using the --remote option. Will replace the previous round with this. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Remove duplicate entry in ./Documentation/Makefile
Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 3615504..7df75d0 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -31,7 +31,6 @@ SP_ARTICLES += howto/separating-topic-branches SP_ARTICLES += howto/revert-a-faulty-merge SP_ARTICLES += howto/recover-corrupted-blob-object SP_ARTICLES += howto/rebuild-from-update-hook -SP_ARTICLES += howto/rebuild-from-update-hook SP_ARTICLES += howto/rebase-from-internal-branch SP_ARTICLES += howto/maintain-git API_DOCS = $(patsubst %.txt,%,$(filter-out technical/api-index-skel.txt technical/api-index.txt, $(wildcard technical/api-*.txt))) -- 1.8.0.msysgit.0 --- Thomas -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove duplicate entry in ./Documentation/Makefile
Thomas Ackermann th.ac...@arcor.de writes: Signed-off-by: Thomas Ackermann th.ac...@arcor.de --- Documentation/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 3615504..7df75d0 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -31,7 +31,6 @@ SP_ARTICLES += howto/separating-topic-branches SP_ARTICLES += howto/revert-a-faulty-merge SP_ARTICLES += howto/recover-corrupted-blob-object SP_ARTICLES += howto/rebuild-from-update-hook -SP_ARTICLES += howto/rebuild-from-update-hook Heh, good eyes. How did you spot it? If not by eyeballing but with some mechanical process, did you spot any others? SP_ARTICLES += howto/rebase-from-internal-branch SP_ARTICLES += howto/maintain-git API_DOCS = $(patsubst %.txt,%,$(filter-out technical/api-index-skel.txt technical/api-index.txt, $(wildcard technical/api-*.txt))) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Convert all fnmatch() calls to wildmatch()
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
diff --git a/tree-walk.c b/tree-walk.c
index 492c7cd..c729e89 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -3,6 +3,7 @@
#include unpack-trees.h
#include dir.h
#include tree.h
+#include wildmatch.h
static const char *get_mode(const char *str, unsigned int *modep)
{
@@ -627,7 +628,7 @@ enum interesting tree_entry_interesting(const struct
name_entry *entry,
return entry_interesting;
if (item-use_wildcard) {
- if (!fnmatch(match + baselen, entry-path, 0))
+ if (!wildmatch(match + baselen, entry-path, 0))
return entry_interesting;
This and the change to dir.c obviously have interactions with
8c6abbc (pathspec: apply *.c optimization from exclude,
2012-11-24).
I've already alluded to it in my response to 2/3, I guess.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove duplicate entry in ./Documentation/Makefile
Hi, Junio C Hamano wrote: If not by eyeballing but with some mechanical process, did you spot any others? I found one other unnecessarily duplicated line in the top-level Makefile: LIB_H += xdiff/xdiff.h by running find -name Makefile | xargs grep += | sort | uniq -d and inspecting the results by hand, but this only checked lines containing +=. -- Matt Kraai https://ftbfs.org/kraai -- To unsubscribe from this list: send the line unsubscribe 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] git-completion.bash: add support for path completion
[jch: again, adding area experts to Cc]
Manlio Perillo manlio.peri...@gmail.com writes:
Changes from version 2:
* Perl is no more used.
* Fixed some coding style issues.
* Refactorized code, to improve future path completion support for
the git reset command.
Thanks. Will replace what was queued on 'pu'.
+# Process path list returned by ls-files and diff-index --name-only
+# commands, in order to list only file names relative to a specified
+# directory, and append a slash to directory names.
+# It accepts 1 optional argument: a directory path. The path must have
+# a trailing slash.
The callsites that call this function, and the way the argument is
used, do not make it look like it is an optional argument to me.
After reading later parts of the patch, I think the callers are
wrong (see below) and you did intend to make $1 optional.
+__git_index_file_list_filter ()
+{
+ local path
+
+ while read -r path; do
+ path=${path#$1}
This will work correctly only if $1 does not have any shell
metacharacter that removes prefix of $path that matches $1 as a
pathname expansion pattern. As this file is for bash completion,
using string-oriented Bash-isms like ${#1} (to get the length of the
prefix) and ${path:$offset} (to get substring) are perfectly fine
way to correct it.
Also, as $1 is optional, won't this barf if it is run under set -u?
+# __git_index_files accepts 1 or 2 arguments:
+# 1: A string for file index status mode (c, m, d, o), as
+#supported by git ls-files command.
+# 2: A directory path (optional).
+#If provided, only files within the specified directory are listed.
+#Sub directories are never recursed. Path must have a trailing
+#slash.
+__git_index_files ()
+{
+ local dir=$(__gitdir)
+
+ if [ -d $dir ]; then
+ git --git-dir=$dir ls-files --exclude-standard -${1} ${2}
|
+ __git_index_file_list_filter ${2} | uniq
Even though everywhere else you seem to quote the variables with dq,
but this ${2} is not written as ${2}, which looks odd. Deliberate?
If you wanted to call __git_index_file_list_filter without parameter
when the caller did not give you the optional $2, then the above is
not the way to write it. It would be ${2+$2}. The upstream of
the pipe (ls-files) also is getting an empty string as the pathspec
when $2 is omitted, and the call will break if this is run under
set -u.
+# __git_diff_index_files accepts 1 or 2 arguments:
+# 1) The id of a tree object.
+# 2) A directory path (optional).
+#If provided, only files within the specified directory are listed.
+#Sub directories are never recursed. Path must have a trailing
+#slash.
+__git_diff_index_files ()
+{
+ local dir=$(__gitdir)
+
+ if [ -d $dir ]; then
+ git --git-dir=$dir diff-index --name-only ${1} |
+ __git_index_file_list_filter ${2} | uniq
This one, when the optional $2 is absent, will call __git_index_file_list_filter
with an empty string in its $1. Intended, or is it also ${2+$2}?
+# __git_complete_index_file requires 1 argument: the file index
+# status mode
+__git_complete_index_file ()
+{
+ local pfx cur_=$cur
+
+ case $cur_ in
+ ?*/*)
+ pfx=${cur_%/*}
+ cur_=${cur_##*/}
+ pfx=${pfx}/
+
+ __gitcomp_nl $(__git_index_files ${1} ${pfx}) $pfx
$cur_
+ ;;
+ *)
+ __gitcomp_nl $(__git_index_files ${1}) $cur_
+ ;;
+ esac
Please dedent the case arms by one level, i.e.
case $value in
$pattern1)
action1
;;
...
;;
esac
+# __git_complete_diff_index_file requires 1 argument: the id of a tree
+# object
+__git_complete_diff_index_file ()
+{
+ local pfx cur_=$cur
+
+ case $cur_ in
+ ?*/*)
+ pfx=${cur_%/*}
+ cur_=${cur_##*/}
+ pfx=${pfx}/
+
+ __gitcomp_nl $(__git_diff_index_files ${1} ${pfx})
$pfx $cur_
+ ;;
+ *)
+ __gitcomp_nl $(__git_diff_index_files ${1})
$cur_
+ ;;
Unquoted $1 looks fishy, even if the only caller of this function
always gives HEAD to it (in which case you can do without making
it a parameter in the first place).
Unquoted ${pfx} given to __git_diff_index_files also looks fishy.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG] Cannot push some grafted branches
Thomas Rast tr...@student.ethz.ch writes: I still wouldn't recommend this approach in git-replace(1) for several reasons: * It does not generalize in any direction. For each field you may want to change, you have to know a _specific_ way of getting just the commit you want. * More to the point of replacing the parent lists, while the above might be expected of a slightly advanced git user, you get into deep magic the second you want to fake a merge commit with an arbitrary combination of parents. (No, you don't need to tell me how. I'm just saying that fooling with either MERGE_HEAD or read-tree is not for mere mortals.) I do not buy either of the above. When you are replacing one with something else, you ought to know what that something else is and how to create it. Editing a text file with an editor to replace 40-hex object names with another is not a more intuitive way for end users, either (in other words, you are seeing this from the point of view of somebody who *knows* the internal representation of Git objects too much). * The above potentially introduces clock skew into the repository, which can trigger bugs (like rev-list accidentally missing out on some side arm!) until we get around to implementing and using generation numbers. That is an irrelevant point when comparing the go down to bare metal replacing the object representation vs use the usual Git tools the end users are already familiar with approaches. You will encounter the issue you are raising if you make a newer commit a parent of an existing child with an older commit timestamp, no matter how you do the grafting. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] add core.pathspecGlob config option
Git takes pathspec arguments in many places to limit the
scope of an operation. These pathspecs are treated not as
literal paths, but as glob patterns that can be fed to
fnmatch. When a user is giving a specific pattern, this is a
nice feature.
However, when programatically providing pathspecs, it can be
a nuisance. For example, to find the latest revision which
modified $foo, one can use git rev-list -- $foo. But if
$foo contains glob characters (e.g., f*), it will
erroneously match more entries than desired. The caller
needs to quote the characters in $foo, and even then, the
results may not be exactly the same as with a literal
pathspec. For instance, the depth checks in
match_pathspec_depth do not kick in if we match via fnmatch.
This patch introduces a config option to turn all pathspecs
into literal strings. This makes it easy to turn off the
globbing behavior for a whole environment (e.g., if you are
serving repos via a web interface that is only going to
use literal programmatic pathspecs), or for a particular run
(by using git -c to set the option for this run).
It cannot turn off globbing for particular pathspecs. That
could eventually be done with a :(noglob) magic pathspec
prefix. However, that level of granularity is not often
needed, and doing :(noglob) right would mean converting
the whole codebase to use struct pathspec, as the usual
const char **pathspec cannot represent extra per-item
flags.
Signed-off-by: Jeff King p...@peff.net
---
Part of me thinks this is just gross, because :(noglob) is the right
solution. But after spending a few hours trying it this morning, there
is a ton of refactoring required to make it work correctly everywhere
(although we could die() if we see it in places that are not using
init_pathspec, so at least we could say not supported yet and not just
silently ignore it).
Still, this is so easy to do, and the lack of granularity does not hurt
my use cases at all (which, if you have not guessed, are improving
corner cases in scripted calls on GitHub). And I do not think it is just
me, or just GitHub. Any server-side porcelain would be better off with
such an option (for example, I think gitweb has the same issues; it is
just that they are pretty rare, because most people do not put glob
metacharacters into their filenames).
So I think this is a nice, simple approach for sites that want it, and
noglob magic can come later (and will not be any harder to implement as
a result of this patch).
cache.h| 1 +
config.c | 5 +
dir.c | 12 +-
environment.c | 1 +
t/t6130-pathspec-noglob.sh | 56 ++
5 files changed, 70 insertions(+), 5 deletions(-)
create mode 100755 t/t6130-pathspec-noglob.sh
diff --git a/cache.h b/cache.h
index 18fdd18..0725a33 100644
--- a/cache.h
+++ b/cache.h
@@ -555,6 +555,7 @@ extern int precomposed_unicode;
extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+extern int core_pathspec_glob;
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
diff --git a/config.c b/config.c
index fb3f868..8a96ba7 100644
--- a/config.c
+++ b/config.c
@@ -760,6 +760,11 @@ static int git_default_core_config(const char *var, const
char *value)
return 0;
}
+ if (!strcmp(var, core.pathspecglob)) {
+ core_pathspec_glob = git_config_bool(var, value);
+ return 0;
+ }
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/dir.c b/dir.c
index 5a83aa7..6e81d4f 100644
--- a/dir.c
+++ b/dir.c
@@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name,
int namelen)
for (;;) {
unsigned char c1 = tolower(*match);
unsigned char c2 = tolower(*name);
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (core_pathspec_glob
is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -138,7 +138,7 @@ static int match_one(const char *match, const char *name,
int namelen)
for (;;) {
unsigned char c1 = *match;
unsigned char c2 = *name;
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (core_pathspec_glob
is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
@@ -148,14 +148,16 @@ static int match_one(const char *match, const char *name,
int namelen)
}
}
-
/*
* If we don't match the matchstring exactly,
* we need to match by fnmatch
*/
matchlen = strlen(match);
- if
Re: What's cooking in git.git (Dec 2012, #05; Tue, 18)
Junio C Hamano gits...@pobox.com writes: Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The tip of the 'master' branch is a bit past 1.8.1-rc2; hopefully we can go final around the end of the week. Many topics are getting into good shape in 'next', hopefully ready to be merged soon after the 1.8.1 final. As we seem to be getting more minor documentation updates that are safe for the upcoming release, and also because I've updated the AsciiDoc toolchain used to prepare the preformatted manpage and HTML tarballs, I changed my mind to delay the final til the end of the year, and tag an rc3 toward the end of this week instead. Both repositories http://code.google.com/p/git-htmldocs/ and http://code.google.com/p/git-manpages/, and the browser-reachable pages at http://git-htmldocs.googlecode.com/git/git.html should start getting the output formatted with AsciiDoc 8.6.8. The following documentation updates topics are likely to be in the rc3: as/doc-for-devs cr/doc-checkout-branch jc/doc-diff-blobs jc/fetch-tags-doc jk/avoid-mailto-invalid-in-doc nd/index-format-doc sl/git-svn-docs sl/readme-gplv2 ta/api-index-doc Also I am planning to have the .mailmap cleanup topic in the rc3: jk/mailmap-cleanup 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] add core.pathspecGlob config option
Jeff King p...@peff.net writes: ... doing :(noglob) right would mean converting the whole codebase to use struct pathspec, as the usual const char **pathspec cannot represent extra per-item flags. As that is the longer-term direction we would want to go, I'd rather not to take the approach in this patch for introducing user-facing support of literal pathspecs. Having said that, even when we have the ':(noglob)' magic pathspec support, it would make sense to introduce a command line option to make it easier for scripted Porcelains that call plumbing commands to pass literal pathspecs (i.e. they know exactly what paths they want to operate, not what patterns the paths they want to operate would match). I do not think configuration variable makes much sense (unless you are thinking git -c core.literalpathspec=true as that command line option). 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] add core.pathspecGlob config option
On Wed, Dec 19, 2012 at 12:54:04PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: ... doing :(noglob) right would mean converting the whole codebase to use struct pathspec, as the usual const char **pathspec cannot represent extra per-item flags. As that is the longer-term direction we would want to go, I'd rather not to take the approach in this patch for introducing user-facing support of literal pathspecs. Having said that, even when we have the ':(noglob)' magic pathspec support, it would make sense to introduce a command line option to make it easier for scripted Porcelains that call plumbing commands to pass literal pathspecs (i.e. they know exactly what paths they want to operate, not what patterns the paths they want to operate would match). Right, that is my use case. Even once :(noglob) exists, it will still be much simpler for me to use a single option, since I would never have mixed patterns and literal paths (and I suspect most use cases that would care about the distinction would be in the same boat). And that is what led me to submit this at all, as it is not just well, it is a hack until :(noglob) works, but this is not as fine a granularity as :(noglob), but it better matches what callers want. I do not think configuration variable makes much sense (unless you are thinking git -c core.literalpathspec=true as that command line option). The configuration variable is much more convenient for me, because otherwise I have to instrument every call to git with a --no-glob option. Because I have a very restricted environment, and happen to know that globs will _never_ be useful on my repos (or, say, on commands run by a particular user who has this in their ~/.gitconfig), it makes sense to just turn off the feature with one switch. And my thinking was that for people who are not in such a restricted environment, git -c core.pathspecglob=false could serve as that command-line option, as you mentioned. It's perhaps a better match to make it an environment variable. Then it is tied to a particular flow of execution, rather than having it be a property of a system, user, or repo (which is what config does). So for my restricted environment, it would be sufficient for me to set the environment variable for the user who runs the scripts. And people who want a command-line option can set it via the shell, or we can provide git --no-pathspec-glob to set it. -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] add core.pathspecGlob config option
Jeff King p...@peff.net writes:
diff --git a/dir.c b/dir.c
index 5a83aa7..6e81d4f 100644
--- a/dir.c
+++ b/dir.c
@@ -126,7 +126,7 @@ static int match_one(const char *match, const char *name,
int namelen)
for (;;) {
unsigned char c1 = tolower(*match);
unsigned char c2 = tolower(*name);
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (core_pathspec_glob
is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
I think you can also do the same to the common_prefix(); we check
for common leading directory prefix but punt upon seeing a directory
component that has a glob character, and under the literal mode,
it is not a special character.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add core.pathspecGlob config option
On Wed, Dec 19, 2012 at 01:30:57PM -0800, Junio C Hamano wrote:
Jeff King p...@peff.net writes:
diff --git a/dir.c b/dir.c
index 5a83aa7..6e81d4f 100644
--- a/dir.c
+++ b/dir.c
@@ -126,7 +126,7 @@ static int match_one(const char *match, const char
*name, int namelen)
for (;;) {
unsigned char c1 = tolower(*match);
unsigned char c2 = tolower(*name);
- if (c1 == '\0' || is_glob_special(c1))
+ if (c1 == '\0' || (core_pathspec_glob
is_glob_special(c1)))
break;
if (c1 != c2)
return 0;
I think you can also do the same to the common_prefix(); we check
for common leading directory prefix but punt upon seeing a directory
component that has a glob character, and under the literal mode,
it is not a special character.
Yeah, I think you're right. Will add to my re-roll.
-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] Add directory pattern matching to attributes
Jean-Noël AVILA avila...@gmail.com writes: This patch was not reviewed when I submitted it for the second time. Did you miss this? http://thread.gmane.org/gmane.comp.version-control.git/211214/focus=211470 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] add GIT_PATHSPEC_GLOB environment variable
On Wed, Dec 19, 2012 at 04:09:19PM -0500, Jeff King wrote: It's perhaps a better match to make it an environment variable. Then it is tied to a particular flow of execution, rather than having it be a property of a system, user, or repo (which is what config does). So for my restricted environment, it would be sufficient for me to set the environment variable for the user who runs the scripts. And people who want a command-line option can set it via the shell, or we can provide git --no-pathspec-glob to set it. Here it is as an environment variable. I think this probably makes more sense in the general case (it's a little more work for my use case, but I think the intended use is much more obvious). I included the common_prefix fix you mentioned (I do not think it produced incorrect results as it was, but it did not take full advantage of an optimization). -- 8 -- Subject: add GIT_PATHSPEC_GLOB environment variable Git takes pathspec arguments in many places to limit the scope of an operation. These pathspecs are treated not as literal paths, but as glob patterns that can be fed to fnmatch. When a user is giving a specific pattern, this is a nice feature. However, when programatically providing pathspecs, it can be a nuisance. For example, to find the latest revision which modified $foo, one can use git rev-list -- $foo. But if $foo contains glob characters (e.g., f*), it will erroneously match more entries than desired. The caller needs to quote the characters in $foo, and even then, the results may not be exactly the same as with a literal pathspec. For instance, the depth checks in match_pathspec_depth do not kick in if we match via fnmatch. This patch introduces an environment variable to turn all pathspecs into literal strings. This makes it easy to turn off the globbing behavior for a whole environment (e.g., if you are serving repos via a web interface that is only going to use literal programmatic pathspecs), or for a particular run. It cannot turn off globbing for particular pathspecs. That could eventually be done with a :(noglob) magic pathspec prefix. However, that level of granularity is not often needed, and doing :(noglob) right would mean converting the whole codebase to use struct pathspec, as the usual const char **pathspec cannot represent extra per-item flags. Signed-off-by: Jeff King p...@peff.net --- Documentation/git.txt | 15 +++ cache.h| 3 +++ dir.c | 24 +- git.c | 8 ++ t/t6130-pathspec-noglob.sh | 62 ++ 5 files changed, 106 insertions(+), 6 deletions(-) create mode 100755 t/t6130-pathspec-noglob.sh diff --git a/Documentation/git.txt b/Documentation/git.txt index 1d797f2..7008b4d 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -422,6 +422,10 @@ help ...`. Do not use replacement refs to replace git objects. See linkgit:git-replace[1] for more information. +--no-pathspec-glob:: + Do not treat pathspecs as glob patterns. See the section on + the `GIT_PATHSPEC_GLOB` environment variable below. + GIT COMMANDS @@ -790,6 +794,17 @@ for further details. as a file path and will try to write the trace messages into it. +GIT_PATHSPEC_GLOB:: + Setting this variable to `0` will turn off the globbing features + of git's pathspecs. For example, running `GIT_PATHSPEC_GLOB=0 git + log -- '*.c'` will search for commits literally touching the + path `*.c`, not any paths that the glob `*.c` matches. You might + want this if you are feeding literal paths to git (e.g., paths + previously given to you by `git ls-tree`, `--raw` diff output, + etc). Setting it to `1` enables pathspec globbing (which is also + the default). + + Discussion[[Discussion]] diff --git a/cache.h b/cache.h index 18fdd18..98af77c 100644 --- a/cache.h +++ b/cache.h @@ -362,6 +362,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NOTES_DISPLAY_REF_ENVIRONMENT GIT_NOTES_DISPLAY_REF #define GIT_NOTES_REWRITE_REF_ENVIRONMENT GIT_NOTES_REWRITE_REF #define GIT_NOTES_REWRITE_MODE_ENVIRONMENT GIT_NOTES_REWRITE_MODE +#define GIT_PATHSPEC_GLOB_ENVIRONMENT GIT_PATHSPEC_GLOB /* * Repository-local GIT_* environment variables @@ -490,6 +491,8 @@ extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pa extern void free_pathspec(struct pathspec *); extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec); +extern int allow_pathspec_glob(void); + #define HASH_WRITE_OBJECT 1 #define HASH_FORMAT_CHECK 2 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum object_type type, const char *path, unsigned flags); diff --git a/dir.c b/dir.c index 5a83aa7..a4d4321 100644 --- a/dir.c +++ b/dir.c @@ -38,6 +38,7 @@ static
Re: [PATCH v3] git-completion.bash: add support for path completion
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Il 19/12/2012 20:57, Junio C Hamano ha scritto:
[jch: again, adding area experts to Cc]
Manlio Perillo manlio.peri...@gmail.com writes:
Changes from version 2:
* Perl is no more used.
* Fixed some coding style issues.
* Refactorized code, to improve future path completion support for
the git reset command.
Thanks. Will replace what was queued on 'pu'.
+# Process path list returned by ls-files and diff-index --name-only
+# commands, in order to list only file names relative to a specified
+# directory, and append a slash to directory names.
+# It accepts 1 optional argument: a directory path. The path must have
+# a trailing slash.
The callsites that call this function, and the way the argument is
used, do not make it look like it is an optional argument to me.
After reading later parts of the patch, I think the callers are
wrong (see below) and you did intend to make $1 optional.
Thanks for the corrections.
As you can see, I'm not very expert in bash programming.
I will review the code to use proper escaping and correct optional
parameters handling, based on your review.
In this case, I incorrectly assumed that bash expands ${1} to an empty
string, in case no arguments are passed to the function.
+__git_index_file_list_filter ()
+{
+local path
+
+while read -r path; do
+path=${path#$1}
This will work correctly only if $1 does not have any shell
metacharacter that removes prefix of $path that matches $1 as a
pathname expansion pattern. As this file is for bash completion,
using string-oriented Bash-isms like ${#1} (to get the length of the
prefix) and ${path:$offset} (to get substring) are perfectly fine
way to correct it.
Ok.
Also, as $1 is optional, won't this barf if it is run under set -u?
Ok.
Here I should use ${1-}.
+# __git_index_files accepts 1 or 2 arguments:
+# 1: A string for file index status mode (c, m, d, o), as
+#supported by git ls-files command.
+# 2: A directory path (optional).
+#If provided, only files within the specified directory are listed.
+#Sub directories are never recursed. Path must have a trailing
+#slash.
+__git_index_files ()
+{
+local dir=$(__gitdir)
+
+if [ -d $dir ]; then
+git --git-dir=$dir ls-files --exclude-standard -${1} ${2}
|
+__git_index_file_list_filter ${2} | uniq
Even though everywhere else you seem to quote the variables with dq,
but this ${2} is not written as ${2}, which looks odd. Deliberate?
No, I simply missed it.
If you wanted to call __git_index_file_list_filter without parameter
when the caller did not give you the optional $2, then the above is
not the way to write it. It would be ${2+$2}.
Yes, this seems the better solution.
[...]
+# __git_diff_index_files accepts 1 or 2 arguments:
+# 1) The id of a tree object.
+# 2) A directory path (optional).
+#If provided, only files within the specified directory are listed.
+#Sub directories are never recursed. Path must have a trailing
+#slash.
+__git_diff_index_files ()
+{
+local dir=$(__gitdir)
+
+if [ -d $dir ]; then
+git --git-dir=$dir diff-index --name-only ${1} |
+__git_index_file_list_filter ${2} | uniq
This one, when the optional $2 is absent, will call
__git_index_file_list_filter
with an empty string in its $1. Intended, or is it also ${2+$2}?
No, it was not intended. But here it should probably be ${2-}.
One possible rule is:
* ${n+$n} should be used by the _git_complete_xxx_file function;
* ${n-} should be used by the _git_xxx_file functions
The alternative is for each function to use ${n-}, or {n+$n}.
But I'm not sure. What is the best practice in bash for optional
parameters propagation?
+# __git_complete_index_file requires 1 argument: the file index
+# status mode
+__git_complete_index_file ()
+{
+local pfx cur_=$cur
+
+case $cur_ in
+?*/*)
+pfx=${cur_%/*}
+cur_=${cur_##*/}
+pfx=${pfx}/
+
+__gitcomp_nl $(__git_index_files ${1} ${pfx}) $pfx
$cur_
+;;
+*)
+__gitcomp_nl $(__git_index_files ${1}) $cur_
+;;
+esac
Please dedent the case arms by one level, i.e.
I missed it.
Vim do not intent correctly this, and I forgot to dedent.
[...]
+# __git_complete_diff_index_file requires 1 argument: the id of a tree
+# object
+__git_complete_diff_index_file ()
+{
+local pfx cur_=$cur
+
+case $cur_ in
+?*/*)
+pfx=${cur_%/*}
+cur_=${cur_##*/}
+pfx=${pfx}/
+
+__gitcomp_nl $(__git_diff_index_files ${1} ${pfx})
$pfx $cur_
+;;
+*)
+
Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable
Jeff King p...@peff.net writes: I included the common_prefix fix you mentioned (I do not think it produced incorrect results as it was, but it did not take full advantage of an optimization). I do not think it would have affected the outcome; you would only have worked with more cycles. Subject: add GIT_PATHSPEC_GLOB environment variable Seems cleanly done from a quick look. Given that the normal mode of operation is to use globbing, I suspect that the names would have been more natural if the toggle were GIT_PATHSPEC_LITERAL and the boolean function were limit_pathspec_to_literal(), instead of allow_pathspec_glob(), sounding as if using glob is done only upon request. But that is a minor issue. This patch introduces an environment variable to turn all pathspecs into literal strings. This makes it easy to turn off the globbing behavior for a whole environment (e.g., if you are serving repos via a web interface that is only going to use literal programmatic pathspecs), or for a particular run. I am not sure if web interface is a particularly good example, though. Is it unusual to imagine a Web UI that takes pathspecs from the user to limit its output (e.g. diff or ls-tree) to those paths that match them? In such a case, the user would expect their pathspecs to work the same way as the Git installed on their desktop, I would think. Will queue; 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-completion.bash: add support for path completion
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 17/12/2012 20:42, Junio C Hamano ha scritto: [...] I am not sure how you would handle the last parameter to git mv, though. That is by definition a path that does not exist, i.e. cannot be completed. Right, the code should be changed. No completion should be done for the second parameter. I deliberately wrote the last not the second, as you can do $ mkdir X $ git mv COPYING README X/. You do need to expand the second parameter to README when the user types git mv COPYING REAMDE X then goes back with \C-b to M, types \C-d three times to remove MDE, and then finally says TAB, to result in git mv COPYING README X Assuming X is a new untracked directory, do you think it is an usability problem if an user try to do: git mv COPYING README TAB and X does not appear in the completion list? As far as I know, the solution is to only support custom expansion the first parameter, unless the user will do something like: git mv COPYING README -- TAB Regards Manlio -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlDSOWAACgkQscQJ24LbaUSOnACfds93RtX1CDOeGbwCGM5/N8HI yVwAn0AZEO6rE083gKgFimGIbiRTyN5Z =z7K5 -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] add GIT_PATHSPEC_GLOB environment variable
On Wed, Dec 19, 2012 at 02:00:03PM -0800, Junio C Hamano wrote: Subject: add GIT_PATHSPEC_GLOB environment variable Seems cleanly done from a quick look. Given that the normal mode of operation is to use globbing, I suspect that the names would have been more natural if the toggle were GIT_PATHSPEC_LITERAL and the boolean function were limit_pathspec_to_literal(), instead of allow_pathspec_glob(), sounding as if using glob is done only upon request. But that is a minor issue. Yeah, I was trying to avoid the double-negation of noglob=false for the default behavior. I guess calling it literal is another way of accomplishing that, and it keeps the default at false. I don't have a strong preference. This patch introduces an environment variable to turn all pathspecs into literal strings. This makes it easy to turn off the globbing behavior for a whole environment (e.g., if you are serving repos via a web interface that is only going to use literal programmatic pathspecs), or for a particular run. I am not sure if web interface is a particularly good example, though. Is it unusual to imagine a Web UI that takes pathspecs from the user to limit its output (e.g. diff or ls-tree) to those paths that match them? In such a case, the user would expect their pathspecs to work the same way as the Git installed on their desktop, I would think. Yes. If you want to provide for user-provided globbing pathspecs, then you'd have to annotate each invocation with whether you want globbing or not. What I was trying to illustrate was more how gitweb will let you click on the history link for foo in a tree listing, resulting in a page that is generated by calling git rev-list foo. You would probably not want pathspec globbing there. Will queue; thanks. Do we want to change the variable name and invert the logic? Now is probably the best time to do it. -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] add GIT_PATHSPEC_GLOB environment variable
Jeff King p...@peff.net writes: Will queue; thanks. Do we want to change the variable name and invert the logic? That would be my preference. I am deep into today's integration cycle, and this PATHSPEC_GLOB version is sitting at the tip of 'pu', so today's pushout will contain that version, 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] add GIT_PATHSPEC_GLOB environment variable
On Wed, Dec 19, 2012 at 02:16:52PM -0800, Junio C Hamano wrote: Jeff King p...@peff.net writes: Will queue; thanks. Do we want to change the variable name and invert the logic? That would be my preference. I am deep into today's integration cycle, and this PATHSPEC_GLOB version is sitting at the tip of 'pu', so today's pushout will contain that version, though. That's fine. I'll send out a revised version, and you can pick it up later. -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] Add directory pattern matching to attributes
Le mercredi 19 décembre 2012 22:44:59, vous avez écrit : Jean-Noël AVILA avila...@gmail.com writes: This patch was not reviewed when I submitted it for the second time. Did you miss this? Grml, I did. Sorry for the noise. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add GIT_PATHSPEC_GLOB environment variable
On Wed, Dec 19, 2012 at 05:20:35PM -0500, Jeff King wrote:
Do we want to change the variable name and invert the logic?
That would be my preference.
[...]
That's fine. I'll send out a revised version, and you can pick it up
later.
Here it is.
-- 8 --
Subject: [PATCH] add global --literal-pathspecs option
Git takes pathspec arguments in many places to limit the
scope of an operation. These pathspecs are treated not as
literal paths, but as glob patterns that can be fed to
fnmatch. When a user is giving a specific pattern, this is a
nice feature.
However, when programatically providing pathspecs, it can be
a nuisance. For example, to find the latest revision which
modified $foo, one can use git rev-list -- $foo. But if
$foo contains glob characters (e.g., f*), it will
erroneously match more entries than desired. The caller
needs to quote the characters in $foo, and even then, the
results may not be exactly the same as with a literal
pathspec. For instance, the depth checks in
match_pathspec_depth do not kick in if we match via fnmatch.
This patch introduces a global command-line option (i.e.,
one for git itself, not for specific commands) to turn
this behavior off. It also has a matching environment
variable, which can make it easier if you are a script or
porcelain interface that is going to issue many such
commands.
This option cannot turn off globbing for particular
pathspecs. That could eventually be done with a :(noglob)
magic pathspec prefix. However, that level of granularity is
more cumbersome to use for many cases, and doing :(noglob)
right would mean converting the whole codebase to use
struct pathspec, as the usual const char **pathspec
cannot represent extra per-item flags.
Signed-off-by: Jeff King p...@peff.net
---
Documentation/git.txt | 15 +++
cache.h| 3 +++
dir.c | 25 ++-
git.c | 8 ++
t/t6130-pathspec-noglob.sh | 62 ++
5 files changed, 107 insertions(+), 6 deletions(-)
create mode 100755 t/t6130-pathspec-noglob.sh
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 1d797f2..8d869db 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -422,6 +422,11 @@ help ...`.
Do not use replacement refs to replace git objects. See
linkgit:git-replace[1] for more information.
+--literal-pathspecs::
+ Treat pathspecs literally, rather than as glob patterns. This is
+ equivalent to setting the `GIT_LITERAL_PATHSPECS` environment
+ variable to `1`.
+
GIT COMMANDS
@@ -790,6 +795,16 @@ for further details.
as a file path and will try to write the trace messages
into it.
+GIT_LITERAL_PATHSPECS::
+ Setting this variable to `1` will cause git to treat all
+ pathspecs literally, rather than as glob patterns. For example,
+ running `GIT_LITERAL_PATHSPECS=1 git log -- '*.c'` will search
+ for commits that touch the path `*.c`, not any paths that the
+ glob `*.c` matches. You might want this if you are feeding
+ literal paths to git (e.g., paths previously given to you by
+ `git ls-tree`, `--raw` diff output, etc).
+
+
Discussion[[Discussion]]
diff --git a/cache.h b/cache.h
index 18fdd18..95608d9 100644
--- a/cache.h
+++ b/cache.h
@@ -362,6 +362,7 @@ static inline enum object_type object_type(unsigned int
mode)
#define GIT_NOTES_DISPLAY_REF_ENVIRONMENT GIT_NOTES_DISPLAY_REF
#define GIT_NOTES_REWRITE_REF_ENVIRONMENT GIT_NOTES_REWRITE_REF
#define GIT_NOTES_REWRITE_MODE_ENVIRONMENT GIT_NOTES_REWRITE_MODE
+#define GIT_LITERAL_PATHSPECS_ENVIRONMENT GIT_LITERAL_PATHSPECS
/*
* Repository-local GIT_* environment variables
@@ -490,6 +491,8 @@ extern int ce_path_match(const struct cache_entry *ce,
const struct pathspec *pa
extern void free_pathspec(struct pathspec *);
extern int ce_path_match(const struct cache_entry *ce, const struct pathspec
*pathspec);
+extern int limit_pathspec_to_literal(void);
+
#define HASH_WRITE_OBJECT 1
#define HASH_FORMAT_CHECK 2
extern int index_fd(unsigned char *sha1, int fd, struct stat *st, enum
object_type type, const char *path, unsigned flags);
diff --git a/dir.c b/dir.c
index 5a83aa7..03ff36b 100644
--- a/dir.c
+++ b/dir.c
@@ -38,6 +38,7 @@ static size_t common_prefix_len(const char **pathspec)
{
const char *n, *first;
size_t max = 0;
+ int literal = limit_pathspec_to_literal();
if (!pathspec)
return max;
@@ -47,7 +48,7 @@ static size_t common_prefix_len(const char **pathspec)
size_t i, len = 0;
for (i = 0; first == n || i max; i++) {
char c = n[i];
- if (!c || c != first[i] || is_glob_special(c))
+ if (!c || c != first[i] || (!literal
is_glob_special(c)))
break;
Re: [PATCH] git-completion.bash: add support for path completion
Manlio Perillo manlio.peri...@gmail.com writes: git mv COPYING README X Assuming X is a new untracked directory, do you think it is an usability problem if an user try to do: git mv COPYING README TAB and X does not appear in the completion list? It is hard to say. Will it show Documentation/ in the list? Will it show git.c in the list? Your git mv git.TAB completing to git mv git.c would be an improvement compared to the stock less git.TAB that offers git.c and git.o as choices. For things like mv and rm, this may not make too much of a difference, git add TAB would be a vast improvement from the stock one. The users will notice that the completion knows what it is doing, and will come to depend on it. But at that point, if git mv COPYING README TAB offers only directories that contain tracked files, the users may get irritated, because it is likely that the destination directory was created immediately before the user started typing git mv. You will hear Surely I created X, it is there, why aren't you showing it to me? The updated completion knows what it is doing everywhere else, and it sets the user-expectation at that level. Uneven cleverness will stand out like a sore thumb and hurts the user perception, which is arguably unfair, but nothing in life is fair X-. I think over-showing the choices is much better than hiding some choices, if we cannot do a perfect completion in some cases. You should know that I won't be moving these files in Y, as I already marked it to be ignored in the .gitignore file! is less grave a gripe than You know I created X just a minute ago, and it is there, why aren't you showing it to me? because you can simply say but Y is there as a directory. admitting that you are less clever than the user expects you to be, but at least you are giving the choice to the user of not picking it. In the ideal world (read: I am *not* saying you should implement it this way, or we won't look at your patch), I think you would want to show tracked files (because it may be the third path that the user wants to move with the command, not the destination directory) and any directory on the filesystem (it could be the third path that is being moved if it has tracked files, or it could be the final destination directory argument). -- To unsubscribe from this list: send the line unsubscribe 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] Add directory pattern matching to attributes
Jean-Noël AVILA avila...@gmail.com writes: Le mercredi 19 décembre 2012 22:44:59, vous avez écrit : Jean-Noël AVILA avila...@gmail.com writes: This patch was not reviewed when I submitted it for the second time. Did you miss this? Grml, I did. Sorry for the noise. That's OK. Your previous one, with the update suggested in the thread, has already been queued in 'next' and will be cooking there throughout the pre-release feature freeze. -- To unsubscribe from this list: send the line unsubscribe 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] git-completion.bash: add support for path completion
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 19/12/2012 20:57, Junio C Hamano ha scritto: [...] I just found a serious bug with git commit path completion. When doing the first commit on an empty repository, completion will cause an error: $git commit -m init TABfatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git command [revision...] -- [file...]' The problem is that for a newly created repository there is no HEAD. If HEAD does not exists, code must use ls-files instead of diff-index to get the completion list. Another change is to always call git commands with stderr redirected to /dev/null. By the way, this is also a bug of current bash completion code: $ git show does-not-exists:TABfatal: Not a valid object name does-not-exists I will write a patch (based on master) to fix this. [...] Regards Manlio -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlDSUR0ACgkQscQJ24LbaUSkRwCfVKk9JhSD4sKDFm4heAkVbN0o KAAAn3paTXyUiFlfY/UBpnkwiARegLsE =7Q5s -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] git-completion.bash: add support for path completion
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Il 19/12/2012 23:49, Junio C Hamano ha scritto: Manlio Perillo manlio.peri...@gmail.com writes: git mv COPYING README X Assuming X is a new untracked directory, do you think it is an usability problem if an user try to do: git mv COPYING README TAB and X does not appear in the completion list? It is hard to say. Will it show Documentation/ in the list? Will it show git.c in the list? Currently all cached files will be showed. Your git mv git.TAB completing to git mv git.c would be an improvement compared to the stock less git.TAB that offers git.c and git.o as choices. For things like mv and rm, this may not make too much of a difference, git add TAB would be a vast improvement from the stock one. The users will notice that the completion knows what it is doing, and will come to depend on it. Yes, this is precisely the reason why I'm implementing it ;-). I also use Mercurial (I discovered Git just a few weeks ago, after reading Pro Git), and Mercurial do have path completion (completion list does not stop at directory boundary, however). When I started to use Git, one of the first thing I noticed was the lack of path completion for git add. [...] In the ideal world (read: I am *not* saying you should implement it this way, or we won't look at your patch), I think you would want to show tracked files (because it may be the third path that the user wants to move with the command, not the destination directory) and any directory on the filesystem (it could be the third path that is being moved if it has tracked files, or it could be the final destination directory argument). What about this? * show all and only cached files for the first argument * show all cached + untracked directories and files for all other arguments This should solve most of the problem, and will still not show ignored files. And, most important, it is very easy to implement. The only issue is that git ls-files -o does not show empty directories, and git ls-files --directory -o will add a trailing slash. Thanks Manlio -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlDSUr4ACgkQscQJ24LbaURf0gCeJVZviwfKHgHNZ8vYBjnjwP8+ WF4AnAn3/KPJciJg9r387qIzCajx4j0s =/10k -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: [RFC] test: Old shells and physical paths
David Michael fedora@gmail.com writes: In working on a port, I have to tolerate an ancient shell. The cd and pwd commands don't understand the -P flag for physical paths, as some tests use. The biggest offender is cd -P causing a failure in t/test-lib.sh (since 1bd9c64), which is sourced by every test script. Is here is a nickel, get a better shell an option? Running tests is one thing, but I'd be worried more about scripted Porcelains broken by a non-POSIX shell if I were you. Would it be acceptable to instead force the platform's shell option (if it exists) to always use physical paths for the tests and drop the -P flags? As a patch to the source files in my tree? Not likely, even though I cannot say for sure without looking at how the change would look like. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add core.pathspecGlob config option
On Thu, Dec 20, 2012 at 3:34 AM, Jeff King p...@peff.net wrote: Part of me thinks this is just gross, because :(noglob) is the right solution. But after spending a few hours trying it this morning, there is a ton of refactoring required to make it work correctly everywhere (although we could die() if we see it in places that are not using init_pathspec, so at least we could say not supported yet and not just silently ignore it). Yep, I'm still half way to converting everything to the new pathspec code. I'm not there yet. And things like this probably better goes with a config key or command line option, appending :(noglob) to every pathspec item is not pleasant. :(icase) may go the same way. So I think this is a nice, simple approach for sites that want it, and noglob magic can come later (and will not be any harder to implement as a result of this patch). Any chance to make use of nd/pathspec-wildcard? It changes the same code path in match_one. If you base on top of nd/pathspec-wildcard, all you have to do is assign nowildcard_len to len (i.e. no wildcard part). -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] wildmatch: support no FNM_PATHNAME mode
On Thu, Dec 20, 2012 at 12:24 AM, Junio C Hamano gits...@pobox.com wrote: When that happens, we should want to retain the same do not bother to descend into subdirectories that will never match optimization for a pattern like Doc*tion/**/*.txt. Because of FNM_PATHNAME, we can tell if a subdirectory is worth descending into by looking at the not-so-simple prefix Doc*tion/; Documentation will match, Doc will not (because '*' won't match '/'). Which tells me that integrating this _well_ into the rest of the system is not just a matter of replacing fnmatch() with wildmatch(). Yeah, we open a door for more opportunities and a lot more headache. I also expect that wildmatch() would be much slower than fnmatch() especially when doing its ** magic, but I personally do not think it will be a showstopper. A potential showstopper is the lack of multibyte support. I don't know if people want that though. If the user asks for a more powerful but expensive operation, we are saving time for the user by doing a more powerful thing (reducing the need to postprocess the results) and can afford to spend extra cycles. In some case we may be able to spend fewer cycles by preprocessing patterns first. As long as simpler patterns fnmatch() groks (namely, '?', '*', and '[class]' wildcards only) are not slowed down by replacing it with wildmatch(), that is, of course. I'm concerned about performance vs fnmatch too. I'll probably write a small program to exercise both and measure it with glibc. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] test: Old shells and physical paths
Hi, On Thu, Dec 20, 2012 at 12:17 AM, Junio C Hamano gits...@pobox.com wrote: Is here is a nickel, get a better shell an option? It is, somewhat. There is a pre-built port of GNU bash 2.03 for the platform, but I was trying to see how far things could go with the OS's supported shell before having to bring in unsupported dependencies. Unfortunately, I do not believe the OS fully conforms to POSIX.1-2001 yet, so that means no -P or -L without going rogue. I'll carry test fixes for this platform locally. Thanks. David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sys/param.h
On 12/19/2012 02:59 AM, Erik Faye-Lund wrote: On Tue, Dec 18, 2012 at 6:01 PM, Junio C Hamano gits...@pobox.com wrote: Johannes Sixt j.s...@viscovery.net writes: Junio C Hamano wrote: It could turn out that we may be able to get rid of sys/param.h altogether, but one step at a time. Inputs from people on minority platforms are very much appreciated---does your platform build fine when the inclusion of the file is removed from git-compat-util.h? cygwin is fine with that removed. Mark -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sys/param.h
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] add core.pathspecGlob config option
On Thu, Dec 20, 2012 at 08:28:57AM +0700, Nguyen Thai Ngoc Duy wrote:
So I think this is a nice, simple approach for sites that want it, and
noglob magic can come later (and will not be any harder to implement as
a result of this patch).
Any chance to make use of nd/pathspec-wildcard? It changes the same
code path in match_one. If you base on top of nd/pathspec-wildcard,
all you have to do is assign nowildcard_len to len (i.e. no wildcard
part).
I'd rather keep it separate for now. One, just because they really are
independent topics, and two, because I am actually back-porting it for
GitHub (we are fairly conservative about upgrading our backend git
versions, as most of the interesting stuff happens on the client side; I
cherry-pick critical patches with no regard to the release cycle).
And the resolution is pretty trivial, too. It looks like this:
diff --cc dir.c
index 5c0e5f6,03ff36b..81cb439
--- a/dir.c
+++ b/dir.c
@@@ -1456,14 -1433,10 +1460,18 @@@ int init_pathspec(struct pathspec *path
item-match = path;
item-len = strlen(path);
- item-nowildcard_len = simple_length(path);
- item-use_wildcard = !limit_pathspec_to_literal()
- !no_wildcard(path);
- if (item-use_wildcard)
- pathspec-has_wildcard = 1;
+ item-flags = 0;
- if (item-nowildcard_len item-len) {
- pathspec-has_wildcard = 1;
- if (path[item-nowildcard_len] == '*'
- no_wildcard(path + item-nowildcard_len + 1))
- item-flags |= PATHSPEC_ONESTAR;
++ if (limit_pathspec_to_literal())
++ item-nowildcard_len = item-len;
++ else {
++ item-nowildcard_len = simple_length(path);
++ if (item-nowildcard_len item-len) {
++ pathspec-has_wildcard = 1;
++ if (path[item-nowildcard_len] == '*'
++ no_wildcard(path + item-nowildcard_len +
1))
++ item-flags |= PATHSPEC_ONESTAR;
++ }
+ }
}
qsort(pathspec-items, pathspec-nr,
Not re-indenting the conditional would make the diff more readable, but
I think the resulting code is simpler to read if all of the wildcard
stuff is inside the else block.
-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] add core.pathspecGlob config option
Jeff King p...@peff.net writes:
On Thu, Dec 20, 2012 at 08:28:57AM +0700, Nguyen Thai Ngoc Duy wrote:
So I think this is a nice, simple approach for sites that want it, and
noglob magic can come later (and will not be any harder to implement as
a result of this patch).
Any chance to make use of nd/pathspec-wildcard? It changes the same
code path in match_one. If you base on top of nd/pathspec-wildcard,
all you have to do is assign nowildcard_len to len (i.e. no wildcard
part).
I'd rather keep it separate for now. One, just because they really are
independent topics, and two, because I am actually back-porting it for
GitHub (we are fairly conservative about upgrading our backend git
versions, as most of the interesting stuff happens on the client side; I
cherry-pick critical patches with no regard to the release cycle).
And the resolution is pretty trivial, too. It looks like this:
diff --cc dir.c
index 5c0e5f6,03ff36b..81cb439
--- a/dir.c
+++ b/dir.c
@@@ -1456,14 -1433,10 +1460,18 @@@ int init_pathspec(struct pathspec *path
item-match = path;
item-len = strlen(path);
- item-nowildcard_len = simple_length(path);
-item-use_wildcard = !limit_pathspec_to_literal()
- !no_wildcard(path);
-if (item-use_wildcard)
-pathspec-has_wildcard = 1;
+item-flags = 0;
- if (item-nowildcard_len item-len) {
- pathspec-has_wildcard = 1;
- if (path[item-nowildcard_len] == '*'
- no_wildcard(path + item-nowildcard_len + 1))
- item-flags |= PATHSPEC_ONESTAR;
++if (limit_pathspec_to_literal())
++item-nowildcard_len = item-len;
++else {
++item-nowildcard_len = simple_length(path);
++if (item-nowildcard_len item-len) {
++pathspec-has_wildcard = 1;
++if (path[item-nowildcard_len] == '*'
++no_wildcard(path + item-nowildcard_len +
1))
++item-flags |= PATHSPEC_ONESTAR;
++}
+}
}
qsort(pathspec-items, pathspec-nr,
Hmph. I thought that returning the length without any stop at glob
special trick from simple_length() would be a simpler resolution.
That is what is queued at the tip of 'pu', anyway.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add core.pathspecGlob config option
On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:
++ if (limit_pathspec_to_literal())
++ item-nowildcard_len = item-len;
++ else {
++ item-nowildcard_len = simple_length(path);
++ if (item-nowildcard_len item-len) {
++ pathspec-has_wildcard = 1;
++ if (path[item-nowildcard_len] == '*'
++ no_wildcard(path + item-nowildcard_len +
1))
++ item-flags |= PATHSPEC_ONESTAR;
++ }
+ }
Hmph. I thought that returning the length without any stop at glob
special trick from simple_length() would be a simpler resolution.
That is what is queued at the tip of 'pu', anyway.
I don't think we can make a change in simple_length. It gets used not
only for pathspecs, but also for parsing exclude patterns, which I do
not think should be affected by this option.
-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] add core.pathspecGlob config option
On Wed, Dec 19, 2012 at 10:55:43PM -0500, Jeff King wrote:
On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:
++if (limit_pathspec_to_literal())
++item-nowildcard_len = item-len;
++else {
++item-nowildcard_len = simple_length(path);
++if (item-nowildcard_len item-len) {
++pathspec-has_wildcard = 1;
++if (path[item-nowildcard_len] == '*'
++no_wildcard(path +
item-nowildcard_len + 1))
++item-flags |= PATHSPEC_ONESTAR;
++}
+}
Hmph. I thought that returning the length without any stop at glob
special trick from simple_length() would be a simpler resolution.
That is what is queued at the tip of 'pu', anyway.
I don't think we can make a change in simple_length. It gets used not
only for pathspecs, but also for parsing exclude patterns, which I do
not think should be affected by this option.
Our test suite wouldn't catch such a misfeature, of course, because the
feature is not turned on by default. But I found it instructive to run
all of the tests with GIT_LITERAL_PATHSPECS on. There are failures, of
course, but by inspecting each failure you can see that it is an
intended effect of the patch (i.e., each tries to use a wildcard
pathspec, which no longer works).
When you suggested changing common_prefix, I ran such a test both with
and without the change[1] and confirmed that it did not change the set
of failure sites. I did not try it, but I suspect running such a test
with the tip of pu would reveal new failures in the .gitignore tests.
-Peff
[1] This is in addition to reading and reasoning about the code, of
course. I would not consider this a very robust form of testing,
though a test failure which cannot be easily explained would be a
good starting point for investigation.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add core.pathspecGlob config option
On Wed, Dec 19, 2012 at 11:06:02PM -0500, Jeff King wrote: I don't think we can make a change in simple_length. It gets used not only for pathspecs, but also for parsing exclude patterns, which I do not think should be affected by this option. Our test suite wouldn't catch such a misfeature, of course, because the feature is not turned on by default. But I found it instructive to run all of the tests with GIT_LITERAL_PATHSPECS on. There are failures, of course, but by inspecting each failure you can see that it is an intended effect of the patch (i.e., each tries to use a wildcard pathspec, which no longer works). When you suggested changing common_prefix, I ran such a test both with and without the change[1] and confirmed that it did not change the set of failure sites. I did not try it, but I suspect running such a test with the tip of pu would reveal new failures in the .gitignore tests. I just tried it, and indeed, running the test suite with this patch: diff --git a/t/test-lib.sh b/t/test-lib.sh index 256f1c6..1c43593 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -102,6 +102,9 @@ export EDITOR export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME export EDITOR +GIT_LITERAL_PATHSPECS=1 +export GIT_LITERAL_PATHSPECS + # Add libc MALLOC and MALLOC_PERTURB test # only if we are not executing the test with valgrind if expr $GIT_TEST_OPTS : .* --valgrind /dev/null || produces many more failures on pu than it does on jk/pathspec-literal. -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] mergetools/p4merge: Handle /dev/null
On Sat, Oct 27, 2012 at 1:47 AM, Jeremy Morton ad...@game-point.net wrote: Sorry to be replying to this so late; I hadn't noticed the post until now! I've tried putting that code in my p4merge script and yes it does indeed work fine. However, it puts a temporary file in the working directory which I'm not sure is a good idea? If we look at this patch which actually solved pretty much the same problem, but when merging and, during a merge conflict, a file was created in both branches: https://github.com/git/git/commit/ec245ba ... it is creating a temp file in a proper temp dir, rather than in the working dir. I think that would be the proper solution here. However, I really want to get this fixed so I'd be happy for this band-aid fix of the p4merge script to be checked in until we could get a patch more like the aforementioned one, at a later date, to create empty files in a proper temp dir and pass them as $LOCAL and $REMOTE. :-) I had the same thoughts when I wrote it, but I figured that following the existing pattern used by mergetool for $REMOTE and $LOCAL when they do exist was simpler as the first step. I have a patch that fixes this by using mktemp that I will send shortly. It only does it for the /dev/null file since the existing behavior for files that do exist is fine. -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
Use mktemp to create the /dev/null placeholder for p4merge.
This keeps it out of the current directory.
Reported-by: Jeremy Morton ad...@game-point.net
Signed-off-by: David Aguilar dav...@gmail.com
---
I consider this a final finishing touch on a new 1.8.1 feature,
so hopefully we can get this in before 1.8.1.
mergetools/p4merge | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 295361a..090fa9b 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -4,13 +4,13 @@ diff_cmd () {
rm_remote=
if test /dev/null = $LOCAL
then
- LOCAL=./p4merge-dev-null.LOCAL.$$
+ LOCAL=$(create_empty_file)
$LOCAL
rm_local=true
fi
if test /dev/null = $REMOTE
then
- REMOTE=./p4merge-dev-null.REMOTE.$$
+ REMOTE=$(create_empty_file)
$REMOTE
rm_remote=true
fi
@@ -33,3 +33,7 @@ merge_cmd () {
$merge_tool_path $BASE $LOCAL $REMOTE $MERGED
check_unchanged
}
+
+create_empty_file () {
+ mktemp -t git-difftool-p4merge-empty-file.XX
+}
--
1.8.1.rc2.6.g18499ba
--
To unsubscribe from this list: send the line unsubscribe 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] test: Old shells and physical paths
On Wed, Dec 19, 2012 at 6:28 PM, David Michael fedora@gmail.com wrote: Hi, On Thu, Dec 20, 2012 at 12:17 AM, Junio C Hamano gits...@pobox.com wrote: Is here is a nickel, get a better shell an option? It is, somewhat. There is a pre-built port of GNU bash 2.03 for the platform, but I was trying to see how far things could go with the OS's supported shell before having to bring in unsupported dependencies. Unfortunately, I do not believe the OS fully conforms to POSIX.1-2001 yet, so that means no -P or -L without going rogue. I'll carry test fixes for this platform locally. Do you know if the differences are relegated to cd, or do other common commands such as awk, grep, sed, mktemp, expr, etc. have similar issues? I wonder if it'd be helpful to have a low-numbered test that checks the basics needed by the scripted Porcelains and test suite. It would give us an easy way to answer these questions, and could be a good way to document (in code) the capabilities we expect. -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] add core.pathspecGlob config option
Jeff King p...@peff.net writes:
On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:
++ if (limit_pathspec_to_literal())
++ item-nowildcard_len = item-len;
++ else {
++ item-nowildcard_len = simple_length(path);
++ if (item-nowildcard_len item-len) {
++ pathspec-has_wildcard = 1;
++ if (path[item-nowildcard_len] == '*'
++ no_wildcard(path + item-nowildcard_len +
1))
++ item-flags |= PATHSPEC_ONESTAR;
++ }
+ }
Hmph. I thought that returning the length without any stop at glob
special trick from simple_length() would be a simpler resolution.
That is what is queued at the tip of 'pu', anyway.
I don't think we can make a change in simple_length. It gets used not
only for pathspecs, but also for parsing exclude patterns, which I do
not think should be affected by this option.
Ouch, yeah, you're right.
--
To unsubscribe from this list: send the line unsubscribe 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] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
David Aguilar dav...@gmail.com writes:
Use mktemp to create the /dev/null placeholder for p4merge.
This keeps it out of the current directory.
Reported-by: Jeremy Morton ad...@game-point.net
Signed-off-by: David Aguilar dav...@gmail.com
---
I consider this a final finishing touch on a new 1.8.1 feature,
so hopefully we can get this in before 1.8.1.
Does everybody have mktemp(1), which is not even in POSIX.1?
I'm a bit hesitant to apply this to the upcoming release without
cooking it in 'next' for sufficiently long time to give it a chance
to be tried by wider audience.
mergetools/p4merge | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 295361a..090fa9b 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -4,13 +4,13 @@ diff_cmd () {
rm_remote=
if test /dev/null = $LOCAL
then
- LOCAL=./p4merge-dev-null.LOCAL.$$
+ LOCAL=$(create_empty_file)
$LOCAL
rm_local=true
fi
if test /dev/null = $REMOTE
then
- REMOTE=./p4merge-dev-null.REMOTE.$$
+ REMOTE=$(create_empty_file)
$REMOTE
rm_remote=true
fi
@@ -33,3 +33,7 @@ merge_cmd () {
$merge_tool_path $BASE $LOCAL $REMOTE $MERGED
check_unchanged
}
+
+create_empty_file () {
+ mktemp -t git-difftool-p4merge-empty-file.XX
+}
--
To unsubscribe from this list: send the line unsubscribe 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] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
Junio C Hamano gits...@pobox.com writes:
David Aguilar dav...@gmail.com writes:
Use mktemp to create the /dev/null placeholder for p4merge.
This keeps it out of the current directory.
Reported-by: Jeremy Morton ad...@game-point.net
Signed-off-by: David Aguilar dav...@gmail.com
---
I consider this a final finishing touch on a new 1.8.1 feature,
so hopefully we can get this in before 1.8.1.
Does everybody have mktemp(1), which is not even in POSIX.1?
I'm a bit hesitant to apply this to the upcoming release without
cooking it in 'next' for sufficiently long time to give it a chance
to be tried by wider audience.
One approach may be to do something like this as a preparation step
(current callers of unpack-file may want to do their temporary work
in TMPDIR as well), and then use
git unpack-file -t e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
to create your empty temporary file.
Documentation/git-unpack-file.txt | 10 +++---
builtin/unpack-file.c | 28 +---
2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-unpack-file.txt
b/Documentation/git-unpack-file.txt
index e9f148a..56af328 100644
--- a/Documentation/git-unpack-file.txt
+++ b/Documentation/git-unpack-file.txt
@@ -10,16 +10,20 @@ git-unpack-file - Creates a temporary file with a blob's
contents
SYNOPSIS
[verse]
-'git unpack-file' blob
+'git unpack-file' [-t] blob
DESCRIPTION
---
Creates a file holding the contents of the blob specified by sha1. It
-returns the name of the temporary file in the following format:
- .merge_file_X
+returns the name of the temporary file (by default `.merge_file_X`).
+
OPTIONS
---
+-t::
+ The temporary file is created in `$TMPDIR` directory,
+ instead of the current directory.
+
blob::
Must be a blob id
diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c
index 1920029..de1f845 100644
--- a/builtin/unpack-file.c
+++ b/builtin/unpack-file.c
@@ -1,8 +1,9 @@
#include builtin.h
-static char *create_temp_file(unsigned char *sha1)
+static char *create_temp_file(unsigned char *sha1, int in_tempdir)
{
- static char path[50];
+ static char path[1024];
+ static const char template[] = .merge_file_XX;
void *buf;
enum object_type type;
unsigned long size;
@@ -12,8 +13,12 @@ static char *create_temp_file(unsigned char *sha1)
if (!buf || type != OBJ_BLOB)
die(unable to read blob object %s, sha1_to_hex(sha1));
- strcpy(path, .merge_file_XX);
- fd = xmkstemp(path);
+ if (in_tempdir) {
+ fd = git_mkstemp(path, sizeof(path) - 1, template);
+ } else {
+ strcpy(path, template);
+ fd = xmkstemp(path);
+ }
if (write_in_full(fd, buf, size) != size)
die_errno(unable to write temp-file);
close(fd);
@@ -23,14 +28,23 @@ static char *create_temp_file(unsigned char *sha1)
int cmd_unpack_file(int argc, const char **argv, const char *prefix)
{
unsigned char sha1[20];
+ int in_tempdir = 0;
+
+ if (argc 2 || 3 argc || !strcmp(argv[1], -h))
+ usage(git unpack-file [-t] sha1);
+ if (argc == 3) {
+ if (strcmp(argv[1], -t))
+ usage(git unpack-file [-t] sha1);
+ in_tempdir = 1;
+ argc--;
+ argv++;
+ }
- if (argc != 2 || !strcmp(argv[1], -h))
- usage(git unpack-file sha1);
if (get_sha1(argv[1], sha1))
die(Not a valid object name %s, argv[1]);
git_config(git_default_config, NULL);
- puts(create_temp_file(sha1));
+ puts(create_temp_file(sha1, in_tempdir));
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
