Re: [PATCH v2] receive-pack: optionally deny case clone refs
Am 6/4/2014 5:13, schrieb David Turner: It is possible to have two branches which are the same but for case. This works great on the case-sensitive filesystems, but not so well on case-insensitive filesystems. It is fairly typical to have case-insensitive clients (Macs, say) with a case-sensitive server (GNU/Linux). Should a user attempt to pull on a Mac when there are case clone branches with differing contents, they'll get an error message containing something like Ref refs/remotes/origin/lower is at [sha-of-lowercase-branch] but expected [sha-of-uppercase-branch] (unable to update local ref) With a case-insensitive git server, if a branch called capital-M Master (that differs from lowercase-m-master) is pushed, nobody else can push to (lowercase-m) master until the branch is removed. Create the option receive.denycaseclonebranches, which checks pushed branches to ensure that they are not case clones of an existing branch. This setting is turned on by default if core.ignorecase is set, but not otherwise. Signed-off-by: David Turner dtur...@twitter.com --- Documentation/config.txt | 6 ++ Documentation/git-push.txt | 5 +++-- Documentation/glossary-content.txt | 5 + builtin/receive-pack.c | 27 +++- t/t5400-send-pack.sh | 43 ++ 5 files changed, 83 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 1932e9b..4deddf8 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2053,6 +2053,12 @@ receive.unpackLimit:: especially on slow filesystems. If not set, the value of `transfer.unpackLimit` is used instead. +receive.denyCaseCloneBranches:: + If set to true, git-receive-pack will deny a ref update that creates + a ref which is the same but for case as an existing ref. This is + useful when clients are on a case-insensitive filesystem, which + will cause errors when given refs which differ only in case. Shouldn't this better be named 'receive.denyCaseCloneRefs'? How about 'denyCaseInsensitiveRefs', 'denyIgnoreCaseRefs'? Is this entry really so important that it must be the first in the list of receive.deny* list, which is not alphabetically sorted? + receive.denyDeletes:: If set to true, git-receive-pack will deny a ref update that deletes the ref. Use this to prevent such a ref deletion via a push. --- a/t/t5400-send-pack.sh +++ b/t/t5400-send-pack.sh @@ -129,6 +129,49 @@ test_expect_success 'denyNonFastforwards trumps --force' ' test $victim_orig = $victim_head ' +test_expect_success 'denyCaseCloneBranches works' ' + ( + cd victim + git config receive.denyCaseCloneBranches true Broken chain. + git config receive.denyDeletes false + ) + git send-pack ./victim HEAD:refs/heads/caseclone + orig_ver=$(git rev-parse HEAD) + test_must_fail git send-pack ./victim HEAD^:refs/heads/CaseClone + #confirm that this had no effect upstream + ( + cd victim + test_must_fail git rev-parse CaseClone + remote_ver=$(git rev-parse caseclone) + test $orig_ver = $remote_ver Please use double-quotes around the variable expansions: There could be a failure mode where remote_ver (and even orig_ver) are empty, which would lead to a syntax error or a wrong result. BTW, on a case-insensitive file system, is there not a chance that 'git rev-parse CaseClone' succeeds even though the ref is stored in victim/.git/refs/heads/caseclone? Perhaps you should inspect the output of 'git for-each-ref' for the expected result? (Mental note: At least a case-preserving file system is required to run the test.) + ) + git send-pack ./victim HEAD^:refs/heads/notacaseclone + test_must_fail git send-pack ./victim :CaseClone + #confirm that this had no effect upstream Please insert a blank after the hash mark. + ( + cd victim + test_must_fail git rev-parse CaseClone + remote_ver=$(git rev-parse caseclone) + test $orig_ver = $remote_ver + ) + git send-pack ./victim :caseclone + #confirm that this took effect upstream + ( + cd victim + test_must_fail git rev-parse caseclone + ) Broken chain. + #check that we can recreate a branch after deleting a + #case-clone of it + case_clone_ver=$(git rev-parse HEAD^) Broken chain. + git send-pack ./victim HEAD^:CaseClone + ( + cd victim + test_must_fail git rev-parse caseclone + remote_ver=$(git rev-parse CaseClone) + test $case_clone_ver = $remote_ver + ) +' + test_expect_success 'push --all excludes remote-tracking hierarchy' ' mkdir parent ( -- Hannes -- To unsubscribe from this list: send the
[PATCH] update-index: fix segfault with missing --cacheinfo argument
Running git update-index --cacheinfo without any further arguments results in a segfault rather than an error message. Commit ec160ae (update-index: teach --cacheinfo a new syntax mode,sha1,path, 2014-03-23) added code to examine the format of the argument, but forgot to handle the NULL case. Returning an error from the parser is enough, since we then treat it as an old-style --cacheinfo mode sha1 path, and complain that we have less than 3 arguments to read. Signed-off-by: Jeff King p...@peff.net --- builtin/update-index.c| 3 +++ t/t2107-update-index-basic.sh | 4 2 files changed, 7 insertions(+) diff --git a/builtin/update-index.c b/builtin/update-index.c index ba54e19..ebea285 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -637,6 +637,9 @@ static int parse_new_style_cacheinfo(const char *arg, unsigned long ul; char *endp; + if (!arg) + return -1; + errno = 0; ul = strtoul(arg, endp, 8); if (errno || endp == arg || *endp != ',' || (unsigned int) ul != ul) diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh index fe2fb17..1bafb90 100755 --- a/t/t2107-update-index-basic.sh +++ b/t/t2107-update-index-basic.sh @@ -29,6 +29,10 @@ test_expect_success 'update-index -h with corrupt index' ' test_i18ngrep [Uu]sage: git update-index broken/usage ' +test_expect_success '--cacheinfo complains of missing arguments' ' + test_must_fail git update-index --cacheinfo +' + test_expect_success '--cacheinfo does not accept blob null sha1' ' echo content file git add file -- 2.0.0.rc1.436.g03cb729 -- To unsubscribe from this list: send the line unsubscribe 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component
On 2014-06-04 05.38, David Turner wrote: [] [] diff --git a/Makefile b/Makefile index a53f3a8..dd2127a 100644 --- a/Makefile +++ b/Makefile @@ -1326,6 +1326,11 @@ else COMPAT_OBJS += compat/win32mmap.o endif endif +ifdef NO_SSE42 + BASIC_CFLAGS += -DNO_SSE42 +else + BASIC_CFLAGS += -msse4.2 +endif This does work for some people, but break for others, like the systems in my test-lab. On 2 different systems the gcc has support for -msse4.2, but the processor has not, and t5511 fails with Illegal instruction. How can that be? The maintainer of a Linux distro wants to ship gcc with all possible features, an the end-user can compile the code with all the features his very processor has. On the other hand, a pre-compiled package like e.g. Git is compiled into a binary package with all the latest features switched of, to be able to run the binary on as many different processor variants as possible. He already needs to make 3 binaries only for x86: - the minimum version is a 32 bit processor like 486/586/686. - a medium version for systems with 4GB RAM (or more) which have 32 bit processors with PAE (686-pae) - a version for x86_64 E.g. a maintainer wants to have SSE42 enabled, when he builds Git for his system, but disabled when he builds an RPM. The people compiling Git need to know what the binary is used for, how about using something like this in Makefile: ifdef HAVE_SSE42 BASIC_CFLAGS += -msse4.2 -DHAS_SSE42 +endif ifdef OBJECT_CREATION_USES_RENAMES COMPAT_CFLAGS += -DOBJECT_CREATION_MODE=1 endif @@ -2199,6 +2204,7 @@ GIT-BUILD-OPTIONS: FORCE @echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' $@ @echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' $@ @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' $@ + @echo NO_SSE42=\''$(subst ','\'',$(subst ','\'',$(NO_SSE42)))'\' $@ Same here: Use HAVE_SSE42 rather than NO_SSE42 diff --git a/aclocal.m4 b/aclocal.m4 index f11bc7e..d9f3f19 100644 --- a/aclocal.m4 +++ b/aclocal.m4 @@ -38,3 +38,9 @@ AC_DEFUN([TYPE_SOCKLEN_T], [#include sys/types.h #include sys/socket.h]) ]) As the whole detection logic does not work as expected (we need to compile and test-run the code, not only compile), can we drop this part completely ? (at least for the first round) --- a/git-compat-util.h +++ b/git-compat-util.h @@ -668,6 +668,28 @@ void git_qsort(void *base, size_t nmemb, size_t size, #endif #endif +#ifndef NO_SSE42 +#include nmmintrin.h +/* + * Clang ships with a version of nmmintrin.h that's incomplete; if + * necessary, we define the constants that we're going to use. + */ +#ifndef _SIDD_UBYTE_OPS +#define _SIDD_UBYTE_OPS 0x00 +#define _SIDD_CMP_EQUAL_ANY 0x00 +#define _SIDD_CMP_RANGES0x04 +#define _SIDD_CMP_EQUAL_ORDERED 0x0c +#define _SIDD_NEGATIVE_POLARITY 0x10 +#endif Why do this defines end up in git-compat-util.h when they are needed by one file? (see even below) --- a/refs.c +++ b/refs.c @@ -24,6 +24,25 @@ static unsigned char refname_disposition[256] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4 }; +static int check_refname_component_trailer(const char *cp, const char *refname, int flags) +{ + if (cp == refname) + return 0; /* Component has zero length. */ + if (refname[0] == '.') { + if (!(flags REFNAME_DOT_COMPONENT)) + return -1; /* Component starts with '.'. */ + /* + * Even if leading dots are allowed, don't allow . + * as a component (.. is prevented by a rule above). + */ + if (refname[1] == '\0') + return -1; /* Component equals .. */ + } + if (cp - refname = 5 !memcmp(cp - 5, .lock, 5)) + return -1; /* Refname ends with .lock. */ + return cp - refname; +} + /* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is @@ -37,7 +56,7 @@ static unsigned char refname_disposition[256] = { * - it ends with .lock * - it contains a \ (backslash) */ -static int check_refname_component(const char *refname, int flags) +static int check_refname_component_1(const char *refname, int flags) The name check_refname_component_1() doesn't tell too much, (check_refname_component_sse42() or check_refname_component_nonsse42() say more) can I suggest to move all SSE code out to a file under compat/, like compat/refs_sse42.c, or something similar ? (And here we need the missing sse4 defines from git-compat-util.h) { const char *cp; char last = '\0'; @@ -47,7 +66,7 @@ static int check_refname_component(const char *refname, int flags) unsigned char disp = refname_disposition[ch];
[PATCH] t9001: avoid not portable '\n' with sed
t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'. As the test already requires perl as a prerequisite, use perl instead of sed. Signed-off-by: Torsten Bögershausen tbo...@web.de --- t/t9001-send-email.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 64d9434..2bf48d1 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1342,7 +1342,7 @@ test_cover_addresses () { git format-patch --cover-letter -2 -o outdir cover=`echo outdir/-*.patch` mv $cover cover-to-edit.patch - sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + $PERL_PATH -pe s/^From:/$header: extra\@address.com\nFrom:/ cover-to-edit.patch | tr Q $LF $cover git send-email \ --force \ --from=Example nob...@example.com \ -- 2.0.0.553.ged01b91 -- To unsubscribe from this list: send the line 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] clone: add clone.recursesubmodules config option
Add a config option that will cause clone to recurse into submodules as if the --recurse-submodules option had been specified on the command line. This can be overridden with the --no-recurse-submodules option. Signed-off-by: Chris Packham judge.pack...@gmail.com --- On 04/06/14 09:05, Junio C Hamano wrote: Mara Kim mara@vanderbilt.edu writes: Apologies if this question has been asked already, but what is the reasoning behind making git clone not recursive (--recursive) by default? The primary reason why submodules are separate repositories is not to require people to have everything. Some people want recursive, some others don't, and the world is not always majority wins (not that I am saying that majority will want recursive). Inertia, aka backward compatibility and not surprising existing users, plays some role when deciding the default. Also, going --recursive when the user did not want is a lot more expensive mistake to fix than not being --recursive when the user wanted to. Having said all that, I do not mean to say that I am opposed to introduce some mechanism to let the users express their preference between recursive and non-recursive better, so that git clone without an explicit --recursive (or --no-recursive) can work to their taste. A configuration in $HOME/.gitconfig might be a place to start, even though that has the downside of assuming that the given user would want to use the same settings for all his projects, which may not be the case in practice. And here's a quick proof of concept. Not sure about the config variable name and it could probably do with a negative test as well. builtin/clone.c | 9 + t/t7407-submodule-foreach.sh | 17 + 2 files changed, 26 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index b12989d..92aea81 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -734,6 +734,14 @@ static void write_refspec_config(const char* src_ref_prefix, strbuf_release(value); } +static int git_clone_config(const char *key, const char *value, void *data) +{ + if (!strcmp(key, clone.recursesubmodules)) + option_recursive = git_config_bool(key, value); + + return 0; +} + int cmd_clone(int argc, const char **argv, const char *prefix) { int is_bundle = 0, is_local; @@ -759,6 +767,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) junk_pid = getpid(); packet_trace_identity(clone); + git_config(git_clone_config, NULL); argc = parse_options(argc, argv, prefix, builtin_clone_options, builtin_clone_usage, 0); diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 7ca10b8..fc2c189 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -307,6 +307,23 @@ test_expect_success 'use update --recursive nested1 to checkout all submodules ) ' +test_expect_success 'use git clone with clone.recursesubmodules to checkout all submodules' ' + git config --local clone.recursesubmodules true + git clone super clone7 + ( + cd clone7 + git rev-parse --resolve-git-dir .git + git rev-parse --resolve-git-dir sub1/.git + git rev-parse --resolve-git-dir sub2/.git + git rev-parse --resolve-git-dir sub3/.git + git rev-parse --resolve-git-dir nested1/.git + git rev-parse --resolve-git-dir nested1/nested2/.git + git rev-parse --resolve-git-dir nested1/nested2/nested3/.git + git rev-parse --resolve-git-dir nested1/nested2/nested3/submodule/.git + ) + git config --local --unset clone.recursesubmodules +' + test_expect_success 'command passed to foreach retains notion of stdin' ' ( cd super -- 2.0.0.153.g79d -- To unsubscribe from this list: send the line unsubscribe 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component
On Wed, Jun 4, 2014 at 3:04 PM, Torsten Bögershausen tbo...@web.de wrote: On 2014-06-04 05.38, David Turner wrote: [] [] diff --git a/Makefile b/Makefile index a53f3a8..dd2127a 100644 --- a/Makefile +++ b/Makefile @@ -1326,6 +1326,11 @@ else COMPAT_OBJS += compat/win32mmap.o endif endif +ifdef NO_SSE42 + BASIC_CFLAGS += -DNO_SSE42 +else + BASIC_CFLAGS += -msse4.2 +endif This does work for some people, but break for others, like the systems in my test-lab. On 2 different systems the gcc has support for -msse4.2, but the processor has not, and t5511 fails with Illegal instruction. How can that be? The maintainer of a Linux distro wants to ship gcc with all possible features, an the end-user can compile the code with all the features his very processor has. I think glibc code uses cpuid instruction to decide whether to use optimized version. May be we can do the same? If we go that route and have a way to detect sse support from compiler, then we can drop NO_SSE42, enable all and pick one at runtime. -- 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] t5000, t5003: do not use test_cmp to compare binary files
test_cmp() is primarily meant to compare text files (and display the difference for debug purposes). Raw cmp is better suited to compare binary files (tar, zip, etc.). On MinGW, test_cmp is a shell function mingw_test_cmp that tries to read both files into environment, stripping CR characters (introduced in commit 4d715ac0). This function usually speeds things up, as fork is extremly slow on Windows. But no wonder that this function is extremely slow and sometimes even crashes when comparing large tar or zip files. Signed-off-by: Stepan Kasal ka...@ucw.cz --- t/t5000-tar-tree.sh | 34 +- t/t5001-archive-attr.sh | 2 +- t/t5003-archive-zip.sh | 6 +++--- t/t5004-archive-corner-cases.sh | 2 +- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh index 1cf0a4e..31b1fd1 100755 --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ -164,7 +164,7 @@ check_tar with_olde-prefix olde- test_expect_success 'git archive on large files' ' test_config core.bigfilethreshold 1 git archive HEAD b3.tar -test_cmp b.tar b3.tar +cmp b.tar b3.tar ' test_expect_success \ @@ -173,15 +173,15 @@ test_expect_success \ test_expect_success \ 'git archive vs. the same in a bare repo' \ -'test_cmp b.tar b3.tar' +'cmp b.tar b3.tar' test_expect_success 'git archive with --output' \ 'git archive --output=b4.tar HEAD -test_cmp b.tar b4.tar' +cmp b.tar b4.tar' test_expect_success 'git archive --remote' \ 'git archive --remote=. HEAD b5.tar -test_cmp b.tar b5.tar' +cmp b.tar b5.tar' test_expect_success \ 'validate file modification time' \ @@ -198,7 +198,7 @@ test_expect_success \ test_expect_success 'git archive with --output, override inferred format' ' git archive --format=tar --output=d4.zip HEAD - test_cmp b.tar d4.zip + cmp b.tar d4.zip ' test_expect_success \ @@ -244,34 +244,34 @@ test_expect_success 'archive --list shows only enabled remote filters' ' test_expect_success 'invoke tar filter by format' ' git archive --format=tar.foo HEAD config.tar.foo tr ab ba config.tar.foo config.tar - test_cmp b.tar config.tar + cmp b.tar config.tar git archive --format=bar HEAD config.bar tr ab ba config.bar config.tar - test_cmp b.tar config.tar + cmp b.tar config.tar ' test_expect_success 'invoke tar filter by extension' ' git archive -o config-implicit.tar.foo HEAD - test_cmp config.tar.foo config-implicit.tar.foo + cmp config.tar.foo config-implicit.tar.foo git archive -o config-implicit.bar HEAD - test_cmp config.tar.foo config-implicit.bar + cmp config.tar.foo config-implicit.bar ' test_expect_success 'default output format remains tar' ' git archive -o config-implicit.baz HEAD - test_cmp b.tar config-implicit.baz + cmp b.tar config-implicit.baz ' test_expect_success 'extension matching requires dot' ' git archive -o config-implicittar.foo HEAD - test_cmp b.tar config-implicittar.foo + cmp b.tar config-implicittar.foo ' test_expect_success 'only enabled filters are available remotely' ' test_must_fail git archive --remote=. --format=tar.foo HEAD \ remote.tar.foo git archive --remote=. --format=bar remote.bar HEAD - test_cmp remote.bar config.bar + cmp remote.bar config.bar ' test_expect_success GZIP 'git archive --format=tgz' ' @@ -280,27 +280,27 @@ test_expect_success GZIP 'git archive --format=tgz' ' test_expect_success GZIP 'git archive --format=tar.gz' ' git archive --format=tar.gz HEAD j1.tar.gz - test_cmp j.tgz j1.tar.gz + cmp j.tgz j1.tar.gz ' test_expect_success GZIP 'infer tgz from .tgz filename' ' git archive --output=j2.tgz HEAD - test_cmp j.tgz j2.tgz + cmp j.tgz j2.tgz ' test_expect_success GZIP 'infer tgz from .tar.gz filename' ' git archive --output=j3.tar.gz HEAD - test_cmp j.tgz j3.tar.gz + cmp j.tgz j3.tar.gz ' test_expect_success GZIP 'extract tgz file' ' gzip -d -c j.tgz j.tar - test_cmp b.tar j.tar + cmp b.tar j.tar ' test_expect_success GZIP 'remote tar.gz is allowed by default' ' git archive --remote=. --format=tar.gz HEAD remote.tar.gz - test_cmp j.tgz remote.tar.gz + cmp j.tgz remote.tar.gz ' test_expect_success GZIP 'remote tar.gz can be disabled' ' diff --git a/t/t5001-archive-attr.sh b/t/t5001-archive-attr.sh index 51dedab..dfc35b3 100755 --- a/t/t5001-archive-attr.sh +++ b/t/t5001-archive-attr.sh @@ -68,7 +68,7 @@ test_expect_missing worktree2/ignored-by-worktree test_expect_success 'git archive vs. bare' ' (cd bare git archive HEAD) bare-archive.tar - test_cmp archive.tar bare-archive.tar + cmp archive.tar
[PATCH] Add a Windows-specific fallback to getenv(HOME);
From: Johannes Schindelin johannes.schinde...@gmx.de Date: Wed, 2 Jun 2010 00:41:33 +0200 If HOME is not set, use $HOMEDRIVE/$HOMEPATH Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hi, this patch is present in msysGit for 4 years. Stepan compat/mingw.c| 18 ++ compat/mingw.h| 3 +++ git-compat-util.h | 4 path.c| 4 ++-- shell.c | 2 +- 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index a0e13bc..8eb21dc 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1847,3 +1847,21 @@ int mingw_offset_1st_component(const char *path) return offset + is_dir_sep(path[offset]); } + +const char *get_windows_home_directory(void) +{ + static const char *home_directory = NULL; + struct strbuf buf = STRBUF_INIT; + + if (home_directory) + return home_directory; + + home_directory = getenv(HOME); + if (home_directory *home_directory) + return home_directory; + + strbuf_addf(buf, %s/%s, getenv(HOMEDRIVE), getenv(HOMEPATH)); + home_directory = strbuf_detach(buf, NULL); + + return home_directory; +} diff --git a/compat/mingw.h b/compat/mingw.h index 3eaf822..a88a7ab 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -386,3 +386,6 @@ static int mingw_main(c,v) * Used by Pthread API implementation for Windows */ extern int err_win_to_posix(DWORD winerr); + +extern const char *get_windows_home_directory(); +#define get_home_directory() get_windows_home_directory() diff --git a/git-compat-util.h b/git-compat-util.h index b6f03b3..409e644 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -740,4 +740,8 @@ struct tm *git_gmtime_r(const time_t *, struct tm *); #define gmtime_r git_gmtime_r #endif +#ifndef get_home_directory +#define get_home_directory() getenv(HOME) +#endif + #endif diff --git a/path.c b/path.c index bc804a3..09b362c 100644 --- a/path.c +++ b/path.c @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { @@ -274,7 +274,7 @@ char *expand_user_path(const char *path) const char *username = path + 1; size_t username_len = first_slash - username; if (username_len == 0) { - const char *home = getenv(HOME); + const char *home = get_home_directory(); if (!home) goto return_null; strbuf_add(user_path, home, strlen(home)); diff --git a/shell.c b/shell.c index 5c0d47a..edd8c3a 100644 --- a/shell.c +++ b/shell.c @@ -55,7 +55,7 @@ static char *make_cmd(const char *prog) static void cd_to_homedir(void) { - const char *home = getenv(HOME); + const char *home = get_home_directory(); if (!home) die(could not determine user's home directory; HOME is unset); if (chdir(home) == -1) -- 1.9.2.msysgit.0.655.g1a42564 -- To unsubscribe from this list: send the line unsubscribe 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] t5000, t5003: do not use test_cmp to compare binary files
Am 04.06.2014 13:42, schrieb Stepan Kasal: test_cmp() is primarily meant to compare text files (and display the difference for debug purposes). Raw cmp is better suited to compare binary files (tar, zip, etc.). On MinGW, test_cmp is a shell function mingw_test_cmp that tries to read both files into environment, stripping CR characters (introduced in commit 4d715ac0). This function usually speeds things up, as fork is extremly slow on Windows. But no wonder that this function is extremely slow and sometimes even crashes when comparing large tar or zip files. Signed-off-by: Stepan Kasal ka...@ucw.cz --- t/t5000-tar-tree.sh | 34 +- t/t5001-archive-attr.sh | 2 +- t/t5003-archive-zip.sh | 6 +++--- t/t5004-archive-corner-cases.sh | 2 +- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh index 1cf0a4e..31b1fd1 100755 --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ -164,7 +164,7 @@ check_tar with_olde-prefix olde- test_expect_success 'git archive on large files' ' test_config core.bigfilethreshold 1 git archive HEAD b3.tar -test_cmp b.tar b3.tar +cmp b.tar b3.tar ' Wouldn't a function like test_cmp_bin() be better suited for all? The windows folks can then use cmp inside test_cmp_bin() and all others just use test_cmp. -- To unsubscribe from this list: send the line unsubscribe 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] t5000, t5003: do not use test_cmp to compare binary files
Hello Thomas, On Wed, Jun 04, 2014 at 02:13:44PM +0200, Thomas Braun wrote: Wouldn't a function like test_cmp_bin() be better suited for all? I also considered it. The advantage is that is shows that this intentionally differs from test_cmp. The windows folks can then use cmp inside test_cmp_bin() and all others ... would use cmp as well because it is better suited for the task than diff -u. So test_cmp_bin would be just an alias for cmp, on all platforms. Doesn't that sound weird? Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] t5000, t5003: do not use test_cmp to compare binary files
Hi Stephan, Am 04.06.2014 14:42, schrieb Stepan Kasal: On Wed, Jun 04, 2014 at 02:13:44PM +0200, Thomas Braun wrote: Wouldn't a function like test_cmp_bin() be better suited for all? I also considered it. The advantage is that is shows that this intentionally differs from test_cmp. The windows folks can then use cmp inside test_cmp_bin() and all others ... would use cmp as well because it is better suited for the task than diff -u. So test_cmp_bin would be just an alias for cmp, on all platforms. Doesn't that sound weird? I actually like the idea that the test assertions follow a common naming scheme and can easily be overriden by $arbitrary-crazy-platform. Using test_cmp_bin instead of cmp would result in then four assertions for comparing arbitrary data test_cmp test_18ncmp test_cmp_text test_cmp_bin where I think the purpose of each function is clear from its name. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote: @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { Just checking. Instead of replace the call sites, can we check and setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. -- 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 v4] Add an explicit GIT_DIR to the list of excludes
On Wed, Jun 4, 2014 at 3:55 AM, Pasha Bolokhov pasha.bolok...@gmail.com wrote: The case when $GIT_DIR points to a _file_ seems uncovered. setup_git_directory() will transform the file to the directory internally and we never know the .git file's path (so we can't exclude it). So people could accidentally add the .git file in, then remove it from from work tree and suddenly the work tree becomes repo-less. It's not as bad as .git _directory_ because we don't lose valuable data. I don't know if you want to cover this too. That's right, there is no way of knowing what the original .git file was. I guess the only way to work around this problem is to modify read_gitfile() so it saves the name of the original file. Then we can add both that .git-file and GIT_DIR to the exclude list. Not a big problem with me, but need to see what you guys think My view is this non-standard $(basename $GIT_DIR) is a corner case. Unless people who care about it (e.g. you) do something that affects the common .git case, or really mess up the code, I don't think it's a problem if you decide to ignore some smaller cases. -- 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: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote: @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { Just checking. Instead of replace the call sites, can we check and setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really need one more place? It seems some of these could be dropped... -- To unsubscribe from this list: send the line unsubscribe 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component
On 2014-06-04 13.21, Duy Nguyen wrote: On Wed, Jun 4, 2014 at 3:04 PM, Torsten Bögershausen tbo...@web.de wrote: On 2014-06-04 05.38, David Turner wrote: [] [] diff --git a/Makefile b/Makefile index a53f3a8..dd2127a 100644 --- a/Makefile +++ b/Makefile @@ -1326,6 +1326,11 @@ else COMPAT_OBJS += compat/win32mmap.o endif endif +ifdef NO_SSE42 + BASIC_CFLAGS += -DNO_SSE42 +else + BASIC_CFLAGS += -msse4.2 +endif This does work for some people, but break for others, like the systems in my test-lab. On 2 different systems the gcc has support for -msse4.2, but the processor has not, and t5511 fails with Illegal instruction. How can that be? The maintainer of a Linux distro wants to ship gcc with all possible features, an the end-user can compile the code with all the features his very processor has. I think glibc code uses cpuid instruction to decide whether to use optimized version. May be we can do the same? If we go that route and have a way to detect sse support from compiler, then we can drop NO_SSE42, enable all and pick one at runtime. Running make under a non-X86 processor like arm fails, as his gcc does not have -msse4.2 On the other hand, looking here: http://sourceware.org/ml/libc-alpha/2009-10/msg00063.html and looking into refs.c, it seems as if we can try to run strcspn(refname, bad_characters) and strstr(refname, @{ and strstr(refname, .. on each refname, instead of checking each char in a loop. The library will pick the fastest version for strcspn() automatically. David, the repo you run the tests on, is it public? Or is there a public repo with this many refs ? Or can you make a dummy repo with 60k refs ? -- To unsubscribe from this list: send the line unsubscribe 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 7/9] fetch doc: remove short-cut section
On 14-06-03 06:16 PM, Junio C Hamano wrote: It is misleading to mention that ref that does not store is to fetch the ref into FETCH_HEAD, because a refspec that does store is also to fetch the LHS into FETCH_HEAD. It is doubly misleading to list it as part of short-cut. ref stands for a refspec that has it on the LHS with a colon and an empty RHS, and that definition should be given at the beginning of the entry where the format is defined. Tentatively remove this misleading description, which leaves the `tag tag` as the only true short-hand, so move it at the beginning of the entry. Well that neatly solves the missing empty line problem... :) M. -- To unsubscribe from this list: send the line unsubscribe 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 5/9] fetch doc: on pulling multiple refspecs
On 14-06-03 06:16 PM, Junio C Hamano wrote: Replace desription of old-style Pull: lines in remotes/ configuration with modern remote.*.fetch variables. As this note applies only to git pull, enable it only in git-pull manual page. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/pull-fetch-param.txt | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt index 40f8687..25af2ce 100644 --- a/Documentation/pull-fetch-param.txt +++ b/Documentation/pull-fetch-param.txt @@ -34,22 +34,26 @@ will be needed for such branches. There is no way to determine or declare that a branch will be made available in a repository with this behavior; the pulling user simply must know this is the expected usage pattern for a branch. +ifdef::git-pull[] + [NOTE] There is a difference between listing multiple refspec directly on 'git pull' command line and having multiple -`Pull:` refspec lines for a repository and running +`remote.repository.fetch` entries in your configuration +for a repository and running s/running/running a/ 'git pull' command without any explicit refspec parameters. refspec listed explicitly on the command line are always s/refspec/refspecs/ merged into the current branch after fetching. In other words, if you list more than one remote refs, you would be making s/refs, you would be making/ref, 'git pull' will create/ -an Octopus. While 'git pull' run without any explicit refspec -parameter takes default refspecs from `Pull:` lines, it +an Octopus merge. On the other hand, 'git pull' that is run +without any explicit refspec parameter takes default +refspecs from `remote.repository.fetch` configuration, it merges only the first refspec found into the current branch, -after fetching all the remote refs. This is because making an +after fetching all the remote refs specified. This is because making an That On the other hand sentence is hard to parse. How about On the other hand, if you do not list any remote refs, 'git pull' will fetch all the refspecs it finds in the `remote.repository.fetch` configuration and merge only the first refspec found into the current branch. Actually, first refspec found is also wrong, isn't it? I'm not sure I can craft a succinct summary of pull's merge behaviour for this note! Octopus from remote refs is rarely done, while keeping track of multiple remote heads in one-go by fetching more than one is often useful. +endif::git-pull[] + Some short-cut notations are also supported. + Hmmm, in my 2.0 man page output (asciidoc 8.6.9 on Ubuntu) there's no empty line between the end of the note and the Some short-cut notations line, which I think is inconsistent with the rest of the formatting. The HTML version looks fine though. Is there some asciidoc-ese that would insert a line there for the man format? M. -- To unsubscribe from this list: send the line unsubscribe 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 a Windows-specific fallback to getenv(HOME);
Hello, On Wed, Jun 04, 2014 at 08:47:58PM +0700, Duy Nguyen wrote: setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. well, I would be afraid to modify the environment for subprocesses. It could hit back in certain situations, like with git filter-branch. We could replace getenv() with a wrapper though. But I don't think it's worth it. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. Actually, the patch had to be updated when msysgit modifications were rebased. But yes, the number of call sites has not icreased: it was called in four places back than, and it is called at three places now. There is only one that is in common, though. Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Am 04.06.2014 16:05, schrieb Erik Faye-Lund: On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote: @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { Just checking. Instead of replace the call sites, can we check and setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. Setting the variable instead of wrapping getenv has the additional benefit that it also affects child processes (read: scripted commands). Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really need one more place? ...all of these also do the string concatenation correctly (i.e. not C:/\Users\MyName as this patch does), fall back to %USERPROFILE% if %HOMEPATH% is not set, and most (except git-wrapper) even check that the directory exists. So IMO this patch has been superseded by more robust solutions and should be dropped. -- To unsubscribe from this list: send the line unsubscribe 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 9/9] fetch: allow explicit --refmap to override configuration
On 14-06-03 06:16 PM, Junio C Hamano wrote: Since the introduction of opportunisitic updates of remote-tracking branches, started at around f2690487 (fetch: opportunistically update tracking refs, 2013-05-11) with a few updates in v1.8.4 era, the remote.*.fetch configuration always kicks in even when a refspec to specify what to fetch is given on the command line, and there is no way to disable or override it per-invocation. Teach the command to pay attention to the --refmap=lhs:rhs command-line options that can be used to override the use of configured remote.*.fetch as the refmap. (Your 0/9 message merely said The new patches at the end clarifies how remote.*.fetch configuration variables are used in two conceptually different ways. so I was not expecting fetch to get a new option.) Signed-off-by: Junio C Hamano gits...@pobox.com --- --- Documentation/fetch-options.txt | 8 Documentation/git-fetch.txt | 3 +++ builtin/fetch.c | 35 --- t/t5510-fetch.sh| 37 + 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 92c68c3..d5c6289 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -72,6 +72,14 @@ endif::git-pull[] setting. See linkgit:git-config[1]. ifndef::git-pull[] +--refmap=refspec:: + When fetching refs listed on the command line, use the + specified refspec (can be given more than once) to map the + refs to remote-tracking branches, instead of values of s/values/the values/ + `remote.*.fetch` configuration variable for the remote s/variable/variables/ + repository. See section on Configured Remote-tracking + Branches for details. + -t:: --tags:: Fetch all tags from the remote (i.e., fetch remote tags diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index d09736a..96c44f9 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -93,6 +93,9 @@ This configuration is used in two ways: only used to decide _where_ the refs that are fetched are stored by acting as a mapping. +The latter use of the configured values can be overridden by giving For consistency with my suggestions on 8/9: s/configured values/`remote.repository.fetch` values/ M. +the `--refmap=refspec` parameter(s) on the command line. + EXAMPLES diff --git a/builtin/fetch.c b/builtin/fetch.c index 55f457c..dd46b61 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -45,6 +45,8 @@ static struct transport *gsecondary; static const char *submodule_prefix = ; static const char *recurse_submodules_default; static int shown_url = 0; +static int refmap_alloc, refmap_nr; +static const char **refmap_array; static int option_parse_recurse_submodules(const struct option *opt, const char *arg, int unset) @@ -69,6 +71,19 @@ static int git_fetch_config(const char *k, const char *v, void *cb) return 0; } +static int parse_refmap_arg(const struct option *opt, const char *arg, int unset) +{ + ALLOC_GROW(refmap_array, refmap_nr + 1, refmap_alloc); + + /* + * git fetch --refmap='' origin foo + * can be used to tell the command not to store anywhere + */ + if (*arg) + refmap_array[refmap_nr++] = arg; + return 0; +} + static struct option builtin_fetch_options[] = { OPT__VERBOSITY(verbosity), OPT_BOOL(0, all, all, @@ -107,6 +122,8 @@ static struct option builtin_fetch_options[] = { N_(default mode for recursion), PARSE_OPT_HIDDEN }, OPT_BOOL(0, update-shallow, update_shallow, N_(accept refs that update .git/shallow)), + { OPTION_CALLBACK, 0, refmap, NULL, N_(refmap), + N_(specify fetch refmap), PARSE_OPT_NONEG, parse_refmap_arg }, OPT_END() }; @@ -278,6 +295,9 @@ static struct ref *get_ref_map(struct transport *transport, const struct ref *remote_refs = transport_get_remote_refs(transport); if (refspec_count) { + struct refspec *fetch_refspec; + int fetch_refspec_nr; + for (i = 0; i refspec_count; i++) { get_fetch_map(remote_refs, refspecs[i], tail, 0); if (refspecs[i].dst refspecs[i].dst[0]) @@ -307,12 +327,21 @@ static struct ref *get_ref_map(struct transport *transport, * by ref_remove_duplicates() in favor of one of these * opportunistic entries with FETCH_HEAD_IGNORE. */ - for (i = 0; i transport-remote-fetch_refspec_nr; i++) - get_fetch_map(ref_map, transport-remote-fetch[i], - oref_tail, 1); + if
Re: [PATCH v2 8/9] fetch doc: add a section on configured remote-tracking branches
On 14-06-03 06:16 PM, Junio C Hamano wrote: To resurrect a misleading mention removed in the previous step, add a section to explain how the remote-tracking configuration interacts with the refspecs given as the command-line arguments. Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-fetch.txt | 43 +++ 1 file changed, 43 insertions(+) diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt index 06106b9..d09736a 100644 --- a/Documentation/git-fetch.txt +++ b/Documentation/git-fetch.txt @@ -51,6 +51,49 @@ include::pull-fetch-param.txt[] include::urls-remotes.txt[] +CONFIGURED REMOTE-TRACKING BRANCHES +--- + +You would often interact with the same remote repository by s/would// +regularly and repeatedly fetching from it. In order to keep track +of the progress of such a remote repository, `git fetch` allows you +to configure `remote.repository.fetch` configuration variable. /variable/variables/ + +Typically such a variable may look like this: + + +[remote origin] + fetch = +refs/heads/*:refs/remotes/origin/* + + +This configuration is used in two ways: + +* When `git fetch` command is run without specifying what branches s/command// + and/or tags to fetch on the command line, e.g. `git fetch origin` + or `git fetch`, the values configured to this variable are used as s/values configured to this variable/`remote.repository.fetch` values/ + the refspecs to be used to fetch. The example above will fetch /to be used// + all branches that exist on the `origin` (i.e. any ref that matches s/on/in/ + the left-hand side of the value, `refs/heads/*`) and update the + corresponding remote-tracking branches in `refs/remotes/origin/*` s/in/in the/ + hierarchy. + +* When `git fetch` command is run with explicit branches and/or tags s/command// + to fetch on the command line, e.g. `git fetch origin master`, the + refspec given on the command line (e.g. `master` in the example, + which is a short-hand for `master:`, which in turn would mean + fetch the 'master' branch but I do not explicitly say what + remote-tracking branch to update with it from the command line) + determines what are to be fetched, and the example command will Change determines what are to be fetched to determines what gets fetched and move the phrase to before the parenthetical comment. + fetch _only_ the 'master' branch. The values of the variable are s/values of the variable/`remote.repository.fetch` values/ + used to map the branch (i.e. `master`) to determine which s/used to map the branch (i.e. `master`) to// + remote-tracking branch, if any, is updated. When used in this + way, the values of the configuration variable do not have any s/values of the configuration variable/`remote.repository.fetch` values/ M. + effect in deciding _what_ gets fetched (i.e. the values are not + used as refspecs when the command-line lists refspecs); they are + only used to decide _where_ the refs that are fetched are stored + by acting as a mapping. + + EXAMPLES -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi Duy, On Wed, 4 Jun 2014, Duy Nguyen wrote: On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote: @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { Just checking. Instead of replace the call sites, can we check and setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. There is a good reason why we did not go for that (noticably cheaper) solution. In fact, it used to be our solution until too many things got broken by setting the HOME variable: Git is not the only program making use of that variable (and IIRC Putty or a merge helper got seriously confused when we set it). So I am afraid, no, we cannot simply setenv(HOME). Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi Erik, On Wed, 4 Jun 2014, Erik Faye-Lund wrote: On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote: @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { Just checking. Instead of replace the call sites, can we check and setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really need one more place? It seems some of these could be dropped... No. Git is not always called through Bash or the git-wrapper, unfortunately. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Erik, On Wed, 4 Jun 2014, Erik Faye-Lund wrote: On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote: @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { Just checking. Instead of replace the call sites, can we check and setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really need one more place? It seems some of these could be dropped... No. Git is not always called through Bash or the git-wrapper, unfortunately. I'm aware of that. But you said in a previous e-mail that e.g putty got confused when we set HOME. How is this a problem for git.exe, but not when we set it in the shell? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi kusma, On Wed, 4 Jun 2014, Erik Faye-Lund wrote: On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: On Wed, 4 Jun 2014, Erik Faye-Lund wrote: On Wed, Jun 4, 2014 at 3:47 PM, Duy Nguyen pclo...@gmail.com wrote: On Wed, Jun 4, 2014 at 6:47 PM, Stepan Kasal ka...@ucw.cz wrote: @@ -133,7 +133,7 @@ char *git_path(const char *fmt, ...) void home_config_paths(char **global, char **xdg, char *file) { char *xdg_home = getenv(XDG_CONFIG_HOME); - char *home = getenv(HOME); + const char *home = get_home_directory(); char *to_free = NULL; if (!home) { Just checking. Instead of replace the call sites, can we check and setenv(HOME) if it's missing instead? MinGW port already replaces main(). Extra initialization should not be a problem. I feel getenv(HOME) a tiny bit more familiar than get_home_directory(), but that's really weak argument as the number of call sites has not increased in 4 years. Yeah. But we already set %HOME% to %HOMEDRIVE%%HOMEPATH% in /etc/profile, git-cmd.bat, gitk.cmd *and* git-wrapper... Do we really need one more place? It seems some of these could be dropped... No. Git is not always called through Bash or the git-wrapper, unfortunately. I'm aware of that. But you said in a previous e-mail that e.g putty got confused when we set HOME. How is this a problem for git.exe, but not when we set it in the shell? The problem arises whenever git.exe calls subprocesses. You can pollute the environment by setting HOME, I do not recall the details, but I remember that we had to be very careful *not* to do that, hence the patch. Sorry, has been a long time. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi dscho, On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: No. Git is not always called through Bash or the git-wrapper, unfortunately. but you have to admit, that in most cases it is called through bash or the git wrapper. The problem arises whenever git.exe calls subprocesses. You can pollute the environment by setting HOME, I do not recall the details, but I remember that we had to be very careful *not* to do that, hence the patch. Sorry, has been a long time. Yeah, memories. Is this experience still valid? How many users do profit from this, using c:/Program\ Files \(86\)/bin/git.exe instead of c:/Program\ Files \(86\)/cmd/git.exe, either by pure luck or intentionally? It seems that we should keep the patch, to minimize surprise if bin/git.exe is used directly. But we should probably make it consistent with other places: - $HOMEDRIVE$HOMEPATH (without the slash) - $USERPROFILE if the above dir does not exist. - setenv HOME instead of wrapper We can make this change for msysGit 2.0.0 only, so that we do not break 1.9.4 ;-) Does this make sense? Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi kusma, On Wed, 4 Jun 2014, Johannes Schindelin wrote: The problem arises whenever git.exe calls subprocesses. You can pollute the environment by setting HOME, I do not recall the details, but I remember that we had to be very careful *not* to do that, hence the patch. Sorry, has been a long time. Actually, a quick search in my Applegate vaults^W^Wmail archives suggests that we had tons of troubles with non-ASCII characters in the path. Given that none of us really has time to recreate the problems, or to take care of them if there arises a new problem due to setting the HOME variable again (remember: while we have UTF-8 support in Git, thanks to Karsten's tireless efforts, and while that seems to fix the biggest bugs for us, other MinGW software does not have that luxury and will continue to barf on non-ASCII characters in the HOME variable), I would be strongly in favor of fixing the problem by the root: avoiding to have Git rely on the HOME environment variable to be set, but instead add a clean API call that even says what it is supposed to do: gimme the user's home directory's path. And that is exactly what the patch does. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi Stepan, On Wed, 4 Jun 2014, Stepan Kasal wrote: On Wed, Jun 4, 2014 at 5:14 PM, Johannes Schindelin johannes.schinde...@gmx.de wrote: No. Git is not always called through Bash or the git-wrapper, unfortunately. but you have to admit, that in most cases it is called through bash or the git wrapper. It would seem so. But the plan was always to make the user experience on Windows less abysmal than now (I just do not have enough time these days to pursue that goal myself), which includes the goal to make git.exe the main entrance point, not Bash nor the git-wrapper. The problem arises whenever git.exe calls subprocesses. You can pollute the environment by setting HOME, I do not recall the details, but I remember that we had to be very careful *not* to do that, hence the patch. Sorry, has been a long time. Yeah, memories. *Very* vague. Sorry. Is this experience still valid? How many users do profit from this, using c:/Program\ Files \(86\)/bin/git.exe instead of c:/Program\ Files \(86\)/cmd/git.exe, either by pure luck or intentionally? Keep in mind that the most problems were introduced by the fact that USERPROFILE disagrees with HOMEDRIVE\HOMEPATH at times. It seems that we should keep the patch, to minimize surprise if bin/git.exe is used directly. I am also in favor of keeping the patch because it introduces a bit of documentation. It says pretty precisely what it wants and allows platform-specific handling without having to play games with the environment, as was suggested earlier. And of course you cannot deny that it had four years of testing. The HOME problems never came back after we included this patch. But we should probably make it consistent with other places: - $HOMEDRIVE$HOMEPATH (without the slash) - $USERPROFILE if the above dir does not exist. - setenv HOME instead of wrapper Possibly. But again, it is hard to argue with four years of testing. Any change you make now will lack that kind of vetting. We can make this change for msysGit 2.0.0 only, so that we do not break 1.9.4 ;-) So we can break 2.0.0! ;-) Actually, 2.0.0 for Windows needs to wait a little longer (it is a little bit unfortunate that we could not coordinate it with upstream) because the plan is to switch to mingwGitDevEnv for said release. No more msysGit. Like, bu-bye. Thanks for all the fish. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] t5000, t5003: do not use test_cmp to compare binary files
test_cmp() is primarily meant to compare text files (and display the difference for debug purposes). Raw cmp is better suited to compare binary files (tar, zip, etc.). On MinGW, test_cmp is a shell function mingw_test_cmp that tries to read both files into environment, stripping CR characters (introduced in commit 4d715ac0). This function usually speeds things up, as fork is extremly slow on Windows. But no wonder that this function is extremely slow and sometimes even crashes when comparing large tar or zip files. Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hi Thomas, On Wed, Jun 04, 2014 at 02:59:44PM +0200, Thomas Braun wrote: Using test_cmp_bin instead of cmp would result in then four assertions for comparing arbitrary data test_cmp test_i18ncmp test_cmp_text test_cmp_bin where I think the purpose of each function is clear from its name. [test_cmp_text does not exist (yet)] OK, I agree, hence this modified version of the patch. Stepan t/t5000-tar-tree.sh | 34 +- t/t5001-archive-attr.sh | 2 +- t/t5003-archive-zip.sh | 6 +++--- t/t5004-archive-corner-cases.sh | 2 +- t/test-lib-functions.sh | 6 ++ 5 files changed, 28 insertions(+), 22 deletions(-) diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh index 1cf0a4e..4efaf8c 100755 --- a/t/t5000-tar-tree.sh +++ b/t/t5000-tar-tree.sh @@ -164,7 +164,7 @@ check_tar with_olde-prefix olde- test_expect_success 'git archive on large files' ' test_config core.bigfilethreshold 1 git archive HEAD b3.tar -test_cmp b.tar b3.tar +test_cmp_bin b.tar b3.tar ' test_expect_success \ @@ -173,15 +173,15 @@ test_expect_success \ test_expect_success \ 'git archive vs. the same in a bare repo' \ -'test_cmp b.tar b3.tar' +'test_cmp_bin b.tar b3.tar' test_expect_success 'git archive with --output' \ 'git archive --output=b4.tar HEAD -test_cmp b.tar b4.tar' +test_cmp_bin b.tar b4.tar' test_expect_success 'git archive --remote' \ 'git archive --remote=. HEAD b5.tar -test_cmp b.tar b5.tar' +test_cmp_bin b.tar b5.tar' test_expect_success \ 'validate file modification time' \ @@ -198,7 +198,7 @@ test_expect_success \ test_expect_success 'git archive with --output, override inferred format' ' git archive --format=tar --output=d4.zip HEAD - test_cmp b.tar d4.zip + test_cmp_bin b.tar d4.zip ' test_expect_success \ @@ -244,34 +244,34 @@ test_expect_success 'archive --list shows only enabled remote filters' ' test_expect_success 'invoke tar filter by format' ' git archive --format=tar.foo HEAD config.tar.foo tr ab ba config.tar.foo config.tar - test_cmp b.tar config.tar + test_cmp_bin b.tar config.tar git archive --format=bar HEAD config.bar tr ab ba config.bar config.tar - test_cmp b.tar config.tar + test_cmp_bin b.tar config.tar ' test_expect_success 'invoke tar filter by extension' ' git archive -o config-implicit.tar.foo HEAD - test_cmp config.tar.foo config-implicit.tar.foo + test_cmp_bin config.tar.foo config-implicit.tar.foo git archive -o config-implicit.bar HEAD - test_cmp config.tar.foo config-implicit.bar + test_cmp_bin config.tar.foo config-implicit.bar ' test_expect_success 'default output format remains tar' ' git archive -o config-implicit.baz HEAD - test_cmp b.tar config-implicit.baz + test_cmp_bin b.tar config-implicit.baz ' test_expect_success 'extension matching requires dot' ' git archive -o config-implicittar.foo HEAD - test_cmp b.tar config-implicittar.foo + test_cmp_bin b.tar config-implicittar.foo ' test_expect_success 'only enabled filters are available remotely' ' test_must_fail git archive --remote=. --format=tar.foo HEAD \ remote.tar.foo git archive --remote=. --format=bar remote.bar HEAD - test_cmp remote.bar config.bar + test_cmp_bin remote.bar config.bar ' test_expect_success GZIP 'git archive --format=tgz' ' @@ -280,27 +280,27 @@ test_expect_success GZIP 'git archive --format=tgz' ' test_expect_success GZIP 'git archive --format=tar.gz' ' git archive --format=tar.gz HEAD j1.tar.gz - test_cmp j.tgz j1.tar.gz + test_cmp_bin j.tgz j1.tar.gz ' test_expect_success GZIP 'infer tgz from .tgz filename' ' git archive --output=j2.tgz HEAD - test_cmp j.tgz j2.tgz + test_cmp_bin j.tgz j2.tgz ' test_expect_success GZIP 'infer tgz from .tar.gz filename' ' git archive --output=j3.tar.gz HEAD - test_cmp j.tgz j3.tar.gz + test_cmp_bin j.tgz j3.tar.gz ' test_expect_success GZIP 'extract tgz file' ' gzip -d -c j.tgz j.tar - test_cmp b.tar j.tar + test_cmp_bin b.tar j.tar ' test_expect_success GZIP 'remote tar.gz is allowed by default' ' git
Re: [PATCH] gitweb: Harden UTF-8 handling in generated links
On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote: W dniu 2014-05-15 21:28, Jakub Narębski pisze: On Thu, May 15, 2014 at 8:48 PM, Michael Wagner accou...@mwagner.org wrote: On Thu, May 15, 2014 at 10:04:24AM +0100, Peter Krefting wrote: Michael Wagner: diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a9f57d6..f1414e1 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -7138,7 +7138,7 @@ sub git_tree { my @entries = (); { local $/ = \0; - open my $fd, -|, git_cmd(), ls-tree, '-z', + open my $fd, -|encoding(UTF-8), git_cmd(), ls-tree, '-z', ($show_sizes ? '-l' : ()), @extra_options, $hash or die_error(500, Open git-ls-tree failed); Or put binmode $fd, ':utf8'; like in the rest of the code. @entries = map { chomp; $_ } $fd; Even better solution would be to use use open IN = ':encoding(utf-8)'; at the beginning of gitweb.perl, once and for all. Or harden esc_param / esc_path_info the same way esc_html is hardened against missing ':utf8' flag. -- 8 -- Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links esc_html() ensures that its input is properly UTF-8 encoded and marked as UTF-8 with to_utf8(). Make esc_param() (used for query parameters in generated URLs), esc_path_info() (for escaping path_info components) and esc_url() use it too. This hardens gitweb against errors in UTF-8 handling; because to_utf8() is idempotent it won't change correct output. Reported-by: Michael Wagner accou...@mwagner.org Signed-off-by: Jakub Narębski jna...@gmail.com --- gitweb/gitweb.perl |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index a9f57d6..77e1312 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -1548,8 +1548,11 @@ sub to_utf8 { sub esc_param { my $str = shift; return undef unless defined $str; + + $str = to_utf8($str); $str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg; $str =~ s/ /\+/g; + return $str; } @@ -1558,6 +1561,7 @@ sub esc_path_info { my $str = shift; return undef unless defined $str; + $str = to_utf8($str); # path_info doesn't treat '+' as space (specially), but '?' must be escaped $str =~ s/([^A-Za-z0-9\-_.~();\/;:@= +]+)/CGI::escape($1)/eg; @@ -1568,8 +1572,11 @@ sub esc_path_info { sub esc_url { my $str = shift; return undef unless defined $str; + + $str = to_utf8($str); $str =~ s/([^A-Za-z0-9\-_.~();\/;?:@= ]+)/CGI::escape($1)/eg; $str =~ s/ /\+/g; + return $str; } -- 1.7.1 While trying to view a blob_plain of Gütekritierien.txt, a 404 error occured. git_get_hash_by_path tries to resolve the hash with the wrong filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails. The filename needs the correct encoding. Something like this is probably needed for all filenames and should be done at a prior stage: --- gitweb/gitweb.perl |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 77e1312..e4a50e7 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4725,7 +4725,7 @@ sub git_print_tree_entry { } print | . $cgi-a({-href = href(action=blob_plain, hash_base=$hash_base, - file_name=$basedir$t-{'name'})}, + file_name=$basedir . to_utf8($t-{'name'}))}, raw); print /td\n; -- 1.7.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi dscho, your arguments seem really strong. (Especially the four years of battle testing, with the memories of constant problems with HOME before.) I hope they are strong enough to convince Junio to accept this patch; that would help. Stepan PS (about mingwGitDevEnv): plan is to switch to mingwGitDevEnv for said release. No more msysGit. Like, bu-bye. Thanks for all the fish. Interesting. With msysgit, there is the net installer - first time I installed msys/mingw sucessfully, it was as easy as Cygwin, perhaps even easier. When I go to mingwGitDevEnv home page, I read about chickens, eggs, and upgrading Perl (which msysGit simply gives up, hinting that it is almost impossible). So I decided to wait for their Git 2.0.0 release before I try to install it (again). I apologize for being so cheeky, I hope it will help anyway... PPS: from marketing point of view, mingwGitDevEnv is far from usable name. Dscho, if you support the idea, would you mind franchising msysGit 2.0 for a decent amount? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] clone: add clone.recursesubmodules config option
Chris Packham judge.pack...@gmail.com writes: On 04/06/14 09:05, Junio C Hamano wrote: Also, going --recursive when the user did not want is a lot more expensive mistake to fix than not being --recursive when the user wanted to. Having said all that, I do not mean to say that I am opposed to introduce some mechanism to let the users express their preference between recursive and non-recursive better, so that git clone without an explicit --recursive (or --no-recursive) can work to their taste. A configuration in $HOME/.gitconfig might be a place to start, even though that has the downside of assuming that the given user would want to use the same settings for all his projects, which may not be the case in practice. And here's a quick proof of concept. Not sure about the config variable name and it could probably do with a negative test as well. I would be more worried about the semantics than the name, though; re-read the part you quoted with extra stress on has the downside. I think I heard the submodule folks (cc'ed) discuss an approach to allow various submodules to be marked with tags with a new type of entry in .gitmodules file in the superproject, and use these tags to signal by default, a new clone will recurse into this submodule. E.g. if projects standardized on defaultClone to mark such submodules, then $HOME/.gitconfig could say [clone] recursesubmodules = defaultClone Or the projects may mark platform specific submodules with tags, e.g. a .gitmodules in a typical superproject might say something like this: [submodule posix] path = ports/posix tags = linux obsd fbsd osx [submodule windows] path = ports/windows tags = win32 [submodule doc] path = documentation tags = defaultClone and then the user's $HOME/.gitconfig might say [clone] recursesubmodules = defaultClone win32 to tell a git clone of such a superproject to clone the top-level, read its .gitmodules, and choose documentation/ and ports/windows submodules but not ports/posix submodule to be further cloned into the working tree of the superproject. Of course, if this kind of project organization proves to be useful, we should try to standardize the set of tags early before people start coming up with random variations of the same thing, spelling the same concept in different ways only to be different, and if that happens, then we could even give a non-empty default value for the clone.recursesubmodules when $HOME/.gitconfig is missing one. Just a random thought. -- To unsubscribe from this list: send the line unsubscribe 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] receive-pack: optionally deny case clone refs
On Wed, 2014-06-04 at 08:06 +0200, Johannes Sixt wrote: +receive.denyCaseCloneBranches:: + If set to true, git-receive-pack will deny a ref update that creates + a ref which is the same but for case as an existing ref. This is + useful when clients are on a case-insensitive filesystem, which + will cause errors when given refs which differ only in case. Shouldn't this better be named 'receive.denyCaseCloneRefs'? Yes. I'll fix that. How about 'denyCaseInsensitiveRefs', 'denyIgnoreCaseRefs'? I don't love these, because it's not the case-insensitivity that's being denied but the duplication. Is this entry really so important that it must be the first in the list of receive.deny* list, which is not alphabetically sorted? I think the list should be sorted alphabetically, so that we don't have to worry about what is most important. snip issues that I'll fix when I reroll BTW, on a case-insensitive file system, is there not a chance that 'git rev-parse CaseClone' succeeds even though the ref is stored in victim/.git/refs/heads/caseclone? Perhaps you should inspect the output of 'git for-each-ref' for the expected result? (Mental note: At least a case-preserving file system is required to run the test.) I'll look into that and fix if necessary. Thanks. snip more issues that I'll fix when I reroll -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9001: avoid not portable '\n' with sed
Torsten Bögershausen tbo...@web.de writes: t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'. As the test already requires perl as a prerequisite, use perl instead of sed. Signed-off-by: Torsten Bögershausen tbo...@web.de --- Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html The escape sequence '\n' shall match a newline embedded in the pattern space. so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people. t/t9001-send-email.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 64d9434..2bf48d1 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1342,7 +1342,7 @@ test_cover_addresses () { git format-patch --cover-letter -2 -o outdir cover=`echo outdir/-*.patch` mv $cover cover-to-edit.patch - sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + $PERL_PATH -pe s/^From:/$header: extra\@address.com\nFrom:/ cover-to-edit.patch | tr Q $LF $cover We have a shell function perl in test-lib-function.sh these days so that you do not have to write $PERL_PATH yourself in tests ;-) git send-email \ --force \ --from=Example nob...@example.com \ 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: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Hi Stepan, On Wed, 4 Jun 2014, Stepan Kasal wrote: PS (about mingwGitDevEnv): plan is to switch to mingwGitDevEnv for said release. No more msysGit. Like, bu-bye. Thanks for all the fish. Interesting. With msysgit, there is the net installer - first time I installed msys/mingw sucessfully, it was as easy as Cygwin, perhaps even easier. When I go to mingwGitDevEnv home page, I read about chickens, eggs, and upgrading Perl (which msysGit simply gives up, hinting that it is almost impossible). So I decided to wait for their Git 2.0.0 release before I try to install it (again). I understand. And now that upstream Git 2.0.0 is out, it will be very hard to use that as a deadline to push against. So: don't hold your breath. PPS: from marketing point of view, mingwGitDevEnv is far from usable name. Dscho, if you support the idea, would you mind franchising msysGit 2.0 for a decent amount? Make me an offer :-P Seriously again, I am in favor of calling it the Git for Windows SDK. But really, it is bikeshedding at this point. There is real work to do, still, before we can switch. Lots of unaddressed questions. Too little time. Speaking of which... budget's depleted for today ;-) Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] update-index: fix segfault with missing --cacheinfo argument
Jeff King p...@peff.net writes: Running git update-index --cacheinfo without any further arguments results in a segfault rather than an error message. Commit ec160ae (update-index: teach --cacheinfo a new syntax mode,sha1,path, 2014-03-23) added code to examine the format of the argument, but forgot to handle the NULL case. Returning an error from the parser is enough, since we then treat it as an old-style --cacheinfo mode sha1 path, and complain that we have less than 3 arguments to read. Signed-off-by: Jeff King p...@peff.net --- 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] t9001: avoid not portable '\n' with sed
Junio C Hamano gits...@pobox.com writes: Torsten Bögershausen tbo...@web.de writes: t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'. As the test already requires perl as a prerequisite, use perl instead of sed. Signed-off-by: Torsten Bögershausen tbo...@web.de --- Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html The escape sequence '\n' shall match a newline embedded in the pattern space. so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people. t/t9001-send-email.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 64d9434..2bf48d1 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1342,7 +1342,7 @@ test_cover_addresses () { git format-patch --cover-letter -2 -o outdir cover=`echo outdir/-*.patch` mv $cover cover-to-edit.patch -sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover +$PERL_PATH -pe s/^From:/$header: extra\@address.com\nFrom:/ cover-to-edit.patch | tr Q $LF $cover We have a shell function perl in test-lib-function.sh these days so that you do not have to write $PERL_PATH yourself in tests ;-) Also, piping output from perl to tr feels somewhat suboptimal. I do not see where in the test material we use Q to LF, and we may want to remove that altogether, but without that removal, an updated patch may look like this. t/t9001-send-email.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 64d9434..9f06b8c 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1342,7 +1342,10 @@ test_cover_addresses () { git format-patch --cover-letter -2 -o outdir cover=`echo outdir/-*.patch` mv $cover cover-to-edit.patch - sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + perl -pe + s/^From:/$header: extra\@address.com\nFrom:/; + y/Q/\n/; +cover-to-edit.patch $cover git send-email \ --force \ --from=Example nob...@example.com \ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] t5000, t5003: do not use test_cmp to compare binary files
Stepan Kasal ka...@ucw.cz writes: test_cmp() is primarily meant to compare text files (and display the difference for debug purposes). Raw cmp is better suited to compare binary files (tar, zip, etc.). On MinGW, test_cmp is a shell function mingw_test_cmp that tries to read both files into environment, stripping CR characters (introduced in commit 4d715ac0). This function usually speeds things up, as fork is extremly slow on Windows. But no wonder that this function is extremely slow and sometimes even crashes when comparing large tar or zip files. Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hi Thomas, On Wed, Jun 04, 2014 at 02:59:44PM +0200, Thomas Braun wrote: Using test_cmp_bin instead of cmp would result in then four assertions for comparing arbitrary data test_cmp test_i18ncmp test_cmp_text test_cmp_bin where I think the purpose of each function is clear from its name. [test_cmp_text does not exist (yet)] OK, I agree, hence this modified version of the patch. Yeah, I think the above reasoning is sound. And I do not think we ever need to have test_cmp_text -- our payload and our messages compared by tests to make sure our expectations hold are text by default. 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: 2.0.0 regression? request pull does not seem to find head
On Mon, Jun 02, 2014 at 02:36:01PM -0700, Linus Torvalds wrote: On Mon, Jun 2, 2014 at 2:01 PM, Michael S. Tsirkin m...@redhat.com wrote: [mst@robin linux]$ git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'net-next' there? git request-pull is clearly correct. There is no net-next in that public repository. OK, I see that it's correct. It used to match commit and go from there, but it does not anymore, and I didn't know this. However, it does not tell me this. It tells me there's no match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc: that commit is there. Also match implies some matching still going on, might be best to drop this. It *used* to be that request-pull then magically tried to fix it up for you, which in turn resulted in the guess not being right, like pointing to the wrong branch that just happened to have the same SHA1, or pointing to a branch when it _should_ have pointed to a tag. Why not just put the SHA1 in there? In fact it would be a bit more robust in case of non-signed pull requests, won't it? Now, if you pushed your local net-next branch to another branch name (I can find a branch name called vhost-next at that repository, then you can *tell* git that, using the same syntax you must have used for the push. So something like git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git net-next:vhost-next should work so that git doesn't end up having to guess (and potentially guessing wrong). But it may actually be a simpler driver error, and you wanted to use vhost-next, and that net-next was actually incorrect? Linus Yes: net-next is a local name, on the remote it's called vhost-next. I realize now that with git request-pull [-p] start url [end] end actually is a name of commit in the *remote* tree, not the local one. Documentation could be improved a bit: Commit to end at (defaults to HEAD). This names the commit at the tip of the history you are asking to be pulled. When the repository named by url has the commit at a tip of a ref that is different from the ref you have locally, you can use the local:remote syntax, to have its local name, a colon :, and its remote name. It does not have to be commit (could be signed tag), and that text does not make it very clear what is different from what until you re-read it a couple of times. How about: Object (commit or tag) to end at (defaults to HEAD). This names the object at the tip of the history you are asking to be pulled. The name end must refer to the same object in both the local tree and the remote tree pointed at by url. If the object that you want pulled has a different name in the local and the remote trees, you can use the local:remote syntax, to have its local name, a colon :, and its remote name. The error message could be improved too, it asks me whether I pushed net-next which I did. Would be nicer if it asked Pushed net-next to net-next there? Also, how is it supposed to work without end? git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'HEAD' there? Where should I push HEAD, and how? In fact does git let you push to HEAD? Finally, the output even with a warning could be better: git request-pull net-next/master git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: No match for commit 2ae76693b8bcabf370b981cd00c36cd41d33fabc found at git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git warn: Are you sure you pushed 'HEAD' there? The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed: Revert net/mlx4_en: Use affinity hint (2014-06-02 00:18:48 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git If someone does not notice the warning (e.g. the warning is on stderr and script could only redirect stdout) then pull request is actually wrong. It would be better to find the commit on both sides and if it's there, just use the hash name. -- MST -- To unsubscribe from this list: send the line unsubscribe 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] t9001: avoid not portable '\n' with sed
On 2014-06-04 20.13, Junio C Hamano wrote: Junio C Hamano gits...@pobox.com writes: Torsten Bögershausen tbo...@web.de writes: t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'. As the test already requires perl as a prerequisite, use perl instead of sed. Signed-off-by: Torsten Bögershausen tbo...@web.de --- Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html The escape sequence '\n' shall match a newline embedded in the pattern space. so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people. t/t9001-send-email.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 64d9434..2bf48d1 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1342,7 +1342,7 @@ test_cover_addresses () { git format-patch --cover-letter -2 -o outdir cover=`echo outdir/-*.patch` mv $cover cover-to-edit.patch - sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + $PERL_PATH -pe s/^From:/$header: extra\@address.com\nFrom:/ cover-to-edit.patch | tr Q $LF $cover We have a shell function perl in test-lib-function.sh these days so that you do not have to write $PERL_PATH yourself in tests ;-) Also, piping output from perl to tr feels somewhat suboptimal. I do not see where in the test material we use Q to LF, and we may want to remove that altogether, but without that removal, an updated patch may look like this. t/t9001-send-email.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 64d9434..9f06b8c 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1342,7 +1342,10 @@ test_cover_addresses () { git format-patch --cover-letter -2 -o outdir cover=`echo outdir/-*.patch` mv $cover cover-to-edit.patch - sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + perl -pe + s/^From:/$header: extra\@address.com\nFrom:/; + y/Q/\n/; + cover-to-edit.patch $cover git send-email \ --force \ --from=Example nob...@example.com \ Good catch, the tr should had been removed. My first version used sed s/^From:/$header: extra@address.comQFrom:/ and the Q was replaced by tr with a literal LF. So I think the 'Q' - '\n' conversion should be removed completely :-) The sed in question is /usr/bin/sed under Mac OS X. Then we have the question: What exactly is the pattern space? In default operation, sed cyclically shall append a line of input, less its terminating newline character, into the pattern space Isn't that the stuff from the input? But that doesn't make too much sence to me, since input lines are terminated by \n. So pattern space seems to mean output when they talk about the \n Anyway, the \n (to insert a newline into the output) works under Linux, but not Mac OS. -- To unsubscribe from this list: send the line unsubscribe 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] t9001: avoid not portable '\n' with sed
On Wed, Jun 04, 2014 at 10:42:46AM -0700, Junio C Hamano wrote: Torsten Bögershausen tbo...@web.de writes: t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'. As the test already requires perl as a prerequisite, use perl instead of sed. Signed-off-by: Torsten Bögershausen tbo...@web.de --- Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html The escape sequence '\n' shall match a newline embedded in the pattern space. so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people. - sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + $PERL_PATH -pe s/^From:/$header: extra\@address.com\nFrom:/ cover-to-edit.patch | tr Q $LF $cover Note that quoted section of POSIX says embedded in the pattern space; under the description of the s command, it says: The replacement string shall be scanned from beginning to end. [...] The meaning of a backslash immediately followed by any character other than '', backslash, a digit, or the delimiter character used for this command, is unspecified. A line can be split by substituting a newline into it. The application shall escape the newline in the replacement by preceding it by a backslash. So the portable way to do it is: sed s/^From:/$header: ex...@address.com\ From:/ cover-to-edit.patch $cover but that requires the continuation to start in column 0, so the Perl variant is probably neater. -- To unsubscribe from this list: send the line unsubscribe 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] t9001: avoid not portable '\n' with sed
Junio C Hamano gits...@pobox.com writes: Torsten Bögershausen tbo...@web.de writes: t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed simply ignore the '\' before the 'n', treating '\n' as 'n'. As the test already requires perl as a prerequisite, use perl instead of sed. Signed-off-by: Torsten Bögershausen tbo...@web.de --- Hmph. I read this in pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html The escape sequence '\n' shall match a newline embedded in the pattern space. so it may be better to be a bit more explicit in the log message to say whose implementation has this issue to warn people. shall match talks about the match expression, not the replacement. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe 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] gitweb: Harden UTF-8 handling in generated links
Michael Wagner wrote: On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote: Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links esc_html() ensures that its input is properly UTF-8 encoded and marked as UTF-8 with to_utf8(). Make esc_param() (used for query parameters in generated URLs), esc_path_info() (for escaping path_info components) and esc_url() use it too. This hardens gitweb against errors in UTF-8 handling; because to_utf8() is idempotent it won't change correct output. [...] sub esc_param { my $str = shift; return undef unless defined $str; + +$str = to_utf8($str); $str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg; $str =~ s/ /\+/g; + return $str; } While trying to view a blob_plain of Gütekritierien.txt, a 404 error occured. git_get_hash_by_path tries to resolve the hash with the wrong filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails. The filename needs the correct encoding. Something like this is probably needed for all filenames and should be done at a prior stage: True. First, I wonder why the tests I did for this situation didn't show any errors even before the harden href() patch. What is different in your config that you see those errors? --- gitweb/gitweb.perl |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 77e1312..e4a50e7 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -4725,7 +4725,7 @@ sub git_print_tree_entry { } print | . $cgi-a({-href = href(action=blob_plain, hash_base=$hash_base, - file_name=$basedir$t-{'name'})}, + file_name=$basedir . to_utf8($t-{'name'}))}, Second, my harder href() patch does not work for this because concatenation of non-UFT8 with UTF8 string screws up Perl knowledge what is and isn't UTF8. So to_utf8() after concat doesn't help. raw); print /td\n; -- To unsubscribe from this list: send the line 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 00/11] Add transaction support for reflog
This patch series is based on the ref-transaction series and is available at https://github.com/rsahlberg/git/tree/ref-transactions-reflog This patch series adds transaction support for updating the reflog. Ronnie Sahlberg (11): refs.c make ref_transaction_create a wrapper to ref_transaction_update refs.c: make ref_transaction_delete a wrapper for ref_transaction_update refs.c: rename the transaction functions refs.c: add a new update_type field to ref_update refs.c: add a function to append a reflog entry to a fd lockfile.c: make hold_lock_file_for_append preserve meaningful errno refs.c: add a transaction function to append a reflog entry refs.c: add a flag to allow reflog updates to truncate the log refs.c: only write reflog update if msg is non-NULL refs.c: allow multiple reflog updates during a single transaction reflog.c: use a reflog transaction when writing during expire branch.c | 11 +- builtin/commit.c | 14 +-- builtin/fetch.c| 12 +- builtin/receive-pack.c | 14 +-- builtin/reflog.c | 84 ++--- builtin/replace.c | 10 +- builtin/tag.c | 10 +- builtin/update-ref.c | 22 ++-- copy.c | 20 ++-- fast-import.c | 23 ++-- lockfile.c | 7 +- refs.c | 317 - refs.h | 64 ++ sequencer.c| 12 +- walker.c | 17 ++- 15 files changed, 404 insertions(+), 233 deletions(-) -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/11] refs.c: add a flag to allow reflog updates to truncate the log
Add a flag that allows us to truncate the reflog before we write the update. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 17 +++-- refs.h | 4 +++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/refs.c b/refs.c index b99fcd9..f2619e1 100644 --- a/refs.c +++ b/refs.c @@ -3714,7 +3714,12 @@ int transaction_commit(struct ref_transaction *transaction, } } - /* Update all reflog files */ + /* +* Update all reflog files +* We have already done all ref updates and deletes. +* There is not much we can do here if there are any reflog +* update errors other than complain. +*/ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; @@ -3722,7 +3727,15 @@ int transaction_commit(struct ref_transaction *transaction, continue; if (update-reflog_fd == -1) continue; - + if (update-flags REFLOG_TRUNCATE) + if (lseek(update-reflog_fd, 0, SEEK_SET) 0 || + ftruncate(update-reflog_fd, 0)) { + error(Could not truncate reflog: %s. %s, + update-refname, strerror(errno)); + rollback_lock_file(update-reflog_lock); + update-reflog_fd = -1; + continue; + } if (log_ref_write_fd(update-reflog_fd, update-old_sha1, update-new_sha1, update-committer, update-msg)) { diff --git a/refs.h b/refs.h index ae8a800..5748cde 100644 --- a/refs.h +++ b/refs.h @@ -317,8 +317,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction, int flags, int have_old, const char *msg, struct strbuf *err); +#define REFLOG_TRUNCATE 0x01 /* - * Append a reflog entry for refname. + * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set + * this update will first truncate the reflog before writing the entry. */ int transaction_update_reflog(struct ref_transaction *transaction, const char *refname, -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/11] lockfile.c: make hold_lock_file_for_append preserve meaningful errno
Update hold_lock_file_for_append and copy_fd to return a meaningful errno on failure. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- copy.c | 20 +--- lockfile.c | 7 ++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/copy.c b/copy.c index a7f58fd..5cb8679 100644 --- a/copy.c +++ b/copy.c @@ -9,10 +9,12 @@ int copy_fd(int ifd, int ofd) if (!len) break; if (len 0) { - int read_error = errno; + int save_errno = errno; close(ifd); - return error(copy-fd: read returned %s, -strerror(read_error)); + error(copy-fd: read returned %s, + strerror(save_errno)); + errno = save_errno; + return -1; } while (len) { int written = xwrite(ofd, buf, len); @@ -22,12 +24,16 @@ int copy_fd(int ifd, int ofd) } else if (!written) { close(ifd); - return error(copy-fd: write returned 0); + error(copy-fd: write returned 0); + errno = EAGAIN; + return -1; } else { - int write_error = errno; + int save_errno = errno; close(ifd); - return error(copy-fd: write returned %s, -strerror(write_error)); + error(copy-fd: write returned %s, + strerror(save_errno)); + errno = save_errno; + return -1; } } } diff --git a/lockfile.c b/lockfile.c index f5da18c..97414b0 100644 --- a/lockfile.c +++ b/lockfile.c @@ -217,15 +217,20 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags) orig_fd = open(path, O_RDONLY); if (orig_fd 0) { if (errno != ENOENT) { + int save_errno = errno; if (flags LOCK_DIE_ON_ERROR) die(cannot open '%s' for copying, path); close(fd); - return error(cannot open '%s' for copying, path); + error(cannot open '%s' for copying, path); + errno = save_errno; + return -1; } } else if (copy_fd(orig_fd, fd)) { + int save_errno = errno; if (flags LOCK_DIE_ON_ERROR) exit(128); close(fd); + errno = save_errno; return -1; } return fd; -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/11] reflog.c: use a reflog transaction when writing during expire
Use a transaction for all updates during expire_reflog. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- builtin/reflog.c | 84 refs.c | 2 +- 2 files changed, 37 insertions(+), 49 deletions(-) diff --git a/builtin/reflog.c b/builtin/reflog.c index e8a8fb1..f11fee3 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -32,8 +32,11 @@ struct cmd_reflog_expire_cb { int recno; }; +static struct strbuf err = STRBUF_INIT; + struct expire_reflog_cb { - FILE *newlog; + struct ref_transaction *t; + const char *refname; enum { UE_NORMAL, UE_ALWAYS, @@ -316,20 +319,18 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1, if (cb-cmd-recno --(cb-cmd-recno) == 0) goto prune; - if (cb-newlog) { - char sign = (tz 0) ? '-' : '+'; - int zone = (tz 0) ? (-tz) : tz; - fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s, - sha1_to_hex(osha1), sha1_to_hex(nsha1), - email, timestamp, sign, zone, - message); + if (cb-t) { + if (transaction_update_reflog(cb-t, cb-refname, nsha1, osha1, + email, timestamp, tz, message, 0, + err)) + return -1; hashcpy(cb-last_kept_sha1, nsha1); } if (cb-cmd-verbose) printf(keep %s, message); return 0; prune: - if (!cb-newlog) + if (!cb-t) printf(would prune %s, message); else if (cb-cmd-verbose) printf(prune %s, message); @@ -353,29 +354,26 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, { struct cmd_reflog_expire_cb *cmd = cb_data; struct expire_reflog_cb cb; - struct ref_lock *lock; - char *log_file, *newlog_path = NULL; struct commit *tip_commit; struct commit_list *tips; int status = 0; memset(cb, 0, sizeof(cb)); + cb.refname = ref; - /* -* we take the lock for the ref itself to prevent it from -* getting updated. -*/ - lock = lock_any_ref_for_update(ref, sha1, 0, NULL); - if (!lock) - return error(cannot lock ref '%s', ref); - log_file = git_pathdup(logs/%s, ref); if (!reflog_exists(ref)) goto finish; - if (!cmd-dry_run) { - newlog_path = git_pathdup(logs/%s.lock, ref); - cb.newlog = fopen(newlog_path, w); + cb.t = transaction_begin(err); + if (!cb.t) { + status |= error(%s, err.buf); + goto cleanup; + } + if (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1, + NULL, 0, 0, NULL, REFLOG_TRUNCATE, + err)) { + status |= error(%s, err.buf); + goto cleanup; } - cb.cmd = cmd; if (!cmd-expire_unreachable || !strcmp(ref, HEAD)) { @@ -407,7 +405,10 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, mark_reachable(cb); } - for_each_reflog_ent(ref, expire_reflog_ent, cb); + if (for_each_reflog_ent(ref, expire_reflog_ent, cb)) { + status |= error(%s, err.buf); + goto cleanup; + } if (cb.unreachable_expire_kind != UE_ALWAYS) { if (cb.unreachable_expire_kind == UE_HEAD) { @@ -420,32 +421,19 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, } } finish: - if (cb.newlog) { - if (fclose(cb.newlog)) { - status |= error(%s: %s, strerror(errno), - newlog_path); - unlink(newlog_path); - } else if (cmd-updateref - (write_in_full(lock-lock_fd, - sha1_to_hex(cb.last_kept_sha1), 40) != 40 || -write_str_in_full(lock-lock_fd, \n) != 1 || -close_ref(lock) 0)) { - status |= error(Couldn't write %s, - lock-lk-filename); - unlink(newlog_path); - } else if (rename(newlog_path, log_file)) { - status |= error(cannot rename %s to %s, - newlog_path, log_file); - unlink(newlog_path); - } else if (cmd-updateref commit_ref(lock)) { - status |= error(Couldn't set %s, lock-ref_name); - } else { - adjust_shared_perm(log_file); + if
[PATCH 04/11] refs.c: add a new update_type field to ref_update
Add a field that describes what type of update this refers to. For now the only type is UPDATE_SHA1 but we will soon add more types. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 24 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/refs.c b/refs.c index f0800ac..0c3e83b 100644 --- a/refs.c +++ b/refs.c @@ -3343,6 +3343,10 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) return retval; } +enum transaction_update_type { + UPDATE_SHA1 = 0, +}; + /** * Information needed for a single ref update. Set new_sha1 to the * new value or to zero to delete the ref. To check the old value @@ -3350,6 +3354,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) * value or to zero to ensure the ref does not exist before update. */ struct ref_update { + enum transaction_update_type update_type; unsigned char new_sha1[20]; unsigned char old_sha1[20]; int flags; /* REF_NODEREF? */ @@ -3413,12 +3418,14 @@ void transaction_free(struct ref_transaction *transaction) } static struct ref_update *add_update(struct ref_transaction *transaction, -const char *refname) +const char *refname, +enum transaction_update_type update_type) { size_t len = strlen(refname); struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1); strcpy((char *)update-refname, refname); + update-update_type = update_type; ALLOC_GROW(transaction-updates, transaction-nr + 1, transaction-alloc); transaction-updates[transaction-nr++] = update; return update; @@ -3439,7 +3446,7 @@ int transaction_update_sha1(struct ref_transaction *transaction, if (have_old !old_sha1) die(BUG: have_old is true but old_sha1 is NULL); - update = add_update(transaction, refname); + update = add_update(transaction, refname, UPDATE_SHA1); hashcpy(update-new_sha1, new_sha1); update-flags = flags; update-have_old = have_old; @@ -3529,7 +3536,9 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n, struct strbuf *err) { int i; - for (i = 1; i n; i++) + for (i = 1; i n; i++) { + if (updates[i - 1]-update_type != UPDATE_SHA1) + continue; if (!strcmp(updates[i - 1]-refname, updates[i]-refname)) { const char *str = Multiple updates for ref '%s' not allowed.; @@ -3538,6 +3547,7 @@ static int ref_update_reject_duplicates(struct ref_update **updates, int n, return 1; } + } return 0; } @@ -3567,10 +3577,12 @@ int transaction_commit(struct ref_transaction *transaction, goto cleanup; } - /* Acquire all locks while verifying old values */ + /* Acquire all ref locks while verifying old values */ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; + if (update-update_type != UPDATE_SHA1) + continue; update-lock = lock_ref_sha1_basic(update-refname, (update-have_old ? update-old_sha1 : @@ -3593,6 +3605,8 @@ int transaction_commit(struct ref_transaction *transaction, for (i = 0; i n; i++) { struct ref_update *update = updates[i]; + if (update-update_type != UPDATE_SHA1) + continue; if (!is_null_sha1(update-new_sha1)) { ret = write_ref_sha1(update-lock, update-new_sha1, update-msg); @@ -3612,6 +3626,8 @@ int transaction_commit(struct ref_transaction *transaction, for (i = 0; i n; i++) { struct ref_update *update = updates[i]; + if (update-update_type != UPDATE_SHA1) + continue; if (update-lock) { if (delete_ref_loose(update-lock, update-type, err)) ret = -1; -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/11] refs.c: make ref_transaction_delete a wrapper for ref_transaction_update
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 15 +-- refs.h | 2 +- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/refs.c b/refs.c index 2dfedf4..0c382f3 100644 --- a/refs.c +++ b/refs.c @@ -3482,16 +3482,11 @@ int ref_transaction_delete(struct ref_transaction *transaction, if (have_old !old_sha1) die(BUG: have_old is true but old_sha1 is NULL); - update = add_update(transaction, refname); - update-flags = flags; - update-have_old = have_old; - if (have_old) { - assert(!is_null_sha1(old_sha1)); - hashcpy(update-old_sha1, old_sha1); - } - if (msg) - update-msg = xstrdup(msg); - return 0; + if (have_old is_null_sha1(old_sha1)) + die(BUG: have_old is true but old_sha1 is null_sha1); + + return ref_transaction_update(transaction, refname, null_sha1, + old_sha1, flags, have_old, msg, err); } int update_ref(const char *action, const char *refname, diff --git a/refs.h b/refs.h index eedddbf..02bbc21 100644 --- a/refs.h +++ b/refs.h @@ -272,7 +272,7 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); /* * Add a reference update to transaction. new_sha1 is the value that - * the reference should have after the update, or zeros if it should + * the reference should have after the update, or null_sha1 if it should * be deleted. If have_old is true and old_sha is not the null_sha1 * then the previous value of the ref must match or the update will fail. * If have_old is true and old_sha1 is the null_sha1 then the ref must not -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line 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 05/11] refs.c: add a function to append a reflog entry to a fd
Break out the code to create the string and writing it to the file descriptor from log_ref_write and into a dedicated function log_ref_write_fd. For now this is only used from log_ref_write but later on we will call this function from reflog transactions too which means that we will end up with only a single place where we write a reflog entry to a file instead of the current two places (log_ref_write and builtin/reflog.c). Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 48 ++-- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/refs.c b/refs.c index 0c3e83b..e89a81c 100644 --- a/refs.c +++ b/refs.c @@ -2835,15 +2835,37 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize) return 0; } +static int log_ref_write_fd(int fd, const unsigned char *old_sha1, + const unsigned char *new_sha1, + const char *committer, const char *msg) +{ + int msglen, written; + unsigned maxlen, len; + char *logrec; + + msglen = msg ? strlen(msg) : 0; + maxlen = strlen(committer) + msglen + 100; + logrec = xmalloc(maxlen); + len = sprintf(logrec, %s %s %s\n, + sha1_to_hex(old_sha1), + sha1_to_hex(new_sha1), + committer); + if (msglen) + len += copy_msg(logrec + len - 1, msg) - 1; + + written = len = maxlen ? write_in_full(fd, logrec, len) : -1; + free(logrec); + if (written != len) + return -1; + + return 0; +} + static int log_ref_write(const char *refname, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *msg) { - int logfd, result, written, oflags = O_APPEND | O_WRONLY; - unsigned maxlen, len; - int msglen; + int logfd, result, oflags = O_APPEND | O_WRONLY; char log_file[PATH_MAX]; - char *logrec; - const char *committer; if (log_all_ref_updates 0) log_all_ref_updates = !is_bare_repository(); @@ -2855,19 +2877,9 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1, logfd = open(log_file, oflags); if (logfd 0) return 0; - msglen = msg ? strlen(msg) : 0; - committer = git_committer_info(0); - maxlen = strlen(committer) + msglen + 100; - logrec = xmalloc(maxlen); - len = sprintf(logrec, %s %s %s\n, - sha1_to_hex(old_sha1), - sha1_to_hex(new_sha1), - committer); - if (msglen) - len += copy_msg(logrec + len - 1, msg) - 1; - written = len = maxlen ? write_in_full(logfd, logrec, len) : -1; - free(logrec); - if (written != len) { + result = log_ref_write_fd(logfd, old_sha1, new_sha1, + git_committer_info(0), msg); + if (result) { int save_errno = errno; close(logfd); error(Unable to append to %s, log_file); -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 09/11] refs.c: only write reflog update if msg is non-NULL
When performing a reflog transaction update, only write to the reflog iff msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform an update that only truncates but does not write. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 5 +++-- refs.h | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index f2619e1..8faf5c1 100644 --- a/refs.c +++ b/refs.c @@ -3736,8 +3736,9 @@ int transaction_commit(struct ref_transaction *transaction, update-reflog_fd = -1; continue; } - if (log_ref_write_fd(update-reflog_fd, update-old_sha1, -update-new_sha1, + if (update-msg + log_ref_write_fd(update-reflog_fd, +update-old_sha1, update-new_sha1, update-committer, update-msg)) { error(Could write to reflog: %s. %s, update-refname, strerror(errno)); diff --git a/refs.h b/refs.h index 5748cde..1b6a055 100644 --- a/refs.h +++ b/refs.h @@ -321,6 +321,7 @@ int transaction_delete_sha1(struct ref_transaction *transaction, /* * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set * this update will first truncate the reflog before writing the entry. + * If msg is NULL no update will be written to the log. */ int transaction_update_reflog(struct ref_transaction *transaction, const char *refname, -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/11] refs.c make ref_transaction_create a wrapper to ref_transaction_update
Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 11 ++- refs.h | 7 --- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/refs.c b/refs.c index f8a6c9a..2dfedf4 100644 --- a/refs.c +++ b/refs.c @@ -3464,15 +3464,8 @@ int ref_transaction_create(struct ref_transaction *transaction, if (!new_sha1 || is_null_sha1(new_sha1)) die(BUG: create ref with null new_sha1); - update = add_update(transaction, refname); - - hashcpy(update-new_sha1, new_sha1); - hashclr(update-old_sha1); - update-flags = flags; - update-have_old = 1; - if (msg) - update-msg = xstrdup(msg); - return 0; + return ref_transaction_update(transaction, refname, new_sha1, + null_sha1, flags, 1, msg, err); } int ref_transaction_delete(struct ref_transaction *transaction, diff --git a/refs.h b/refs.h index 8177052..eedddbf 100644 --- a/refs.h +++ b/refs.h @@ -273,9 +273,10 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err); /* * Add a reference update to transaction. new_sha1 is the value that * the reference should have after the update, or zeros if it should - * be deleted. If have_old is true, then old_sha1 holds the value - * that the reference should have had before the update, or zeros if - * it must not have existed beforehand. + * be deleted. If have_old is true and old_sha is not the null_sha1 + * then the previous value of the ref must match or the update will fail. + * If have_old is true and old_sha1 is the null_sha1 then the ref must not + * already exist and a new ref will be created with new_sha1. * Function returns 0 on success and non-zero on failure. A failure to update * means that the transaction as a whole has failed and will need to be * rolled back. -- 2.0.0.578.gb9e379f -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/11] refs.c: add a transaction function to append a reflog entry
Define a new transaction update type, UPDATE_LOG, and a new function transaction_update_reflog. This function will lock the reflog and append an entry to it during transaction commit. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 101 - refs.h | 12 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/refs.c b/refs.c index e89a81c..b99fcd9 100644 --- a/refs.c +++ b/refs.c @@ -3357,6 +3357,7 @@ int for_each_reflog(each_ref_fn fn, void *cb_data) enum transaction_update_type { UPDATE_SHA1 = 0, + UPDATE_LOG = 1, }; /** @@ -3374,6 +3375,12 @@ struct ref_update { struct ref_lock *lock; int type; char *msg; + + /* used by reflog updates */ + int reflog_fd; + struct lock_file reflog_lock; + char *committer; + const char refname[FLEX_ARRAY]; }; @@ -3423,6 +3430,7 @@ void transaction_free(struct ref_transaction *transaction) for (i = 0; i transaction-nr; i++) { free(transaction-updates[i]-msg); + free(transaction-updates[i]-committer); free(transaction-updates[i]); } free(transaction-updates); @@ -3443,6 +3451,42 @@ static struct ref_update *add_update(struct ref_transaction *transaction, return update; } +int transaction_update_reflog(struct ref_transaction *transaction, + const char *refname, + const unsigned char *new_sha1, + const unsigned char *old_sha1, + const unsigned char *email, + unsigned long timestamp, int tz, + const char *msg, int flags, + struct strbuf *err) +{ + struct ref_update *update; + + if (transaction-state != REF_TRANSACTION_OPEN) + die(BUG: update_reflog called for transaction that is not + open); + + update = add_update(transaction, refname, UPDATE_LOG); + hashcpy(update-new_sha1, new_sha1); + hashcpy(update-old_sha1, old_sha1); + update-reflog_fd = -1; + if (email) { + struct strbuf buf = STRBUF_INIT; + char sign = (tz 0) ? '-' : '+'; + int zone = (tz 0) ? (-tz) : tz; + + strbuf_addf(buf, %s %lu %c%04d, email, timestamp, sign, + zone); + update-committer = xstrdup(buf.buf); + strbuf_release(buf); + } + if (msg) + update-msg = xstrdup(msg); + update-flags = flags; + + return 0; +} + int transaction_update_sha1(struct ref_transaction *transaction, const char *refname, const unsigned char *new_sha1, @@ -3613,7 +3657,28 @@ int transaction_commit(struct ref_transaction *transaction, } } - /* Perform updates first so live commits remain referenced */ + /* Lock all reflog files */ + for (i = 0; i n; i++) { + struct ref_update *update = updates[i]; + + if (update-update_type != UPDATE_LOG) + continue; + update-reflog_fd = hold_lock_file_for_append( + update-reflog_lock, + git_path(logs/%s, update-refname), + 0); + if (update-reflog_fd 0) { + const char *str = Cannot lock reflog for '%s'. %s; + + ret = -1; + if (err) + strbuf_addf(err, str, update-refname, + strerror(errno)); + goto cleanup; + } + } + + /* Perform ref updates first so live commits remain referenced */ for (i = 0; i n; i++) { struct ref_update *update = updates[i]; @@ -3649,6 +3714,40 @@ int transaction_commit(struct ref_transaction *transaction, } } + /* Update all reflog files */ + for (i = 0; i n; i++) { + struct ref_update *update = updates[i]; + + if (update-update_type != UPDATE_LOG) + continue; + if (update-reflog_fd == -1) + continue; + + if (log_ref_write_fd(update-reflog_fd, update-old_sha1, +update-new_sha1, +update-committer, update-msg)) { + error(Could write to reflog: %s. %s, + update-refname, strerror(errno)); + rollback_lock_file(update-reflog_lock); + update-reflog_fd = -1; + } + } + + /* Commit all reflog
[PATCH 10/11] refs.c: allow multiple reflog updates during a single transaction
Allow to make multiple reflog updates to the same ref during a transaction. This means we only need to lock the reflog once, during the first update that touches the reflog, and that all further updates can just write the reflog entry since the reflog is already locked. This allows us to write code such as: t = transaction_begin() transaction_reflog_update(t, foo, REFLOG_TRUNCATE, NULL); loop-over-somehting... transaction_reflog_update(t, foo, 0, message); transaction_commit(t) where we first truncate the reflog and then build the new content one line at a time. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 45 - 1 file changed, 36 insertions(+), 9 deletions(-) diff --git a/refs.c b/refs.c index 8faf5c1..eb64d8b 100644 --- a/refs.c +++ b/refs.c @@ -34,6 +34,11 @@ static inline int bad_ref_char(int ch) * pruned. */ #define REF_ISPRUNING 0x0100 +/* + * Only the first reflog update needs to lock the reflog file. Further updates + * just use the lock taken by the first update. + */ +#define UPDATE_REFLOG_NOLOCK 0x0200 /* * Try to read one refname component from the front of refname. Return @@ -3360,7 +3365,7 @@ enum transaction_update_type { UPDATE_LOG = 1, }; -/** +/* * Information needed for a single ref update. Set new_sha1 to the * new value or to zero to delete the ref. To check the old value * while locking the ref, set have_old to 1 and set old_sha1 to the @@ -3370,7 +3375,7 @@ struct ref_update { enum transaction_update_type update_type; unsigned char new_sha1[20]; unsigned char old_sha1[20]; - int flags; /* REF_NODEREF? */ + int flags; /* REF_NODEREF? or private flags */ int have_old; /* 1 if old_sha1 is valid, 0 otherwise */ struct ref_lock *lock; int type; @@ -3378,8 +3383,9 @@ struct ref_update { /* used by reflog updates */ int reflog_fd; - struct lock_file reflog_lock; + struct lock_file *reflog_lock; char *committer; + struct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */ const char refname[FLEX_ARRAY]; }; @@ -3461,12 +3467,27 @@ int transaction_update_reflog(struct ref_transaction *transaction, struct strbuf *err) { struct ref_update *update; + int i; if (transaction-state != REF_TRANSACTION_OPEN) die(BUG: update_reflog called for transaction that is not open); update = add_update(transaction, refname, UPDATE_LOG); + update-flags = flags; + for (i = 0; i transaction-nr - 1; i++) { + if (transaction-updates[i]-update_type != UPDATE_LOG) + continue; + if (!strcmp(transaction-updates[i]-refname, + update-refname)) { + update-flags |= UPDATE_REFLOG_NOLOCK; + update-orig_update = transaction-updates[i]; + break; + } + } + if (!(update-flags UPDATE_REFLOG_NOLOCK)) + update-reflog_lock = xcalloc(1, sizeof(struct lock_file)); + hashcpy(update-new_sha1, new_sha1); hashcpy(update-old_sha1, old_sha1); update-reflog_fd = -1; @@ -3482,7 +3503,6 @@ int transaction_update_reflog(struct ref_transaction *transaction, } if (msg) update-msg = xstrdup(msg); - update-flags = flags; return 0; } @@ -3663,10 +3683,15 @@ int transaction_commit(struct ref_transaction *transaction, if (update-update_type != UPDATE_LOG) continue; + if (update-flags UPDATE_REFLOG_NOLOCK) { + update-reflog_fd = update-orig_update-reflog_fd; + update-reflog_lock = update-orig_update-reflog_lock; + continue; + } update-reflog_fd = hold_lock_file_for_append( - update-reflog_lock, + update-reflog_lock, git_path(logs/%s, update-refname), - 0); + LOCK_NODEREF); if (update-reflog_fd 0) { const char *str = Cannot lock reflog for '%s'. %s; @@ -3732,7 +3757,7 @@ int transaction_commit(struct ref_transaction *transaction, ftruncate(update-reflog_fd, 0)) { error(Could not truncate reflog: %s. %s, update-refname, strerror(errno)); - rollback_lock_file(update-reflog_lock); + rollback_lock_file(update-reflog_lock); update-reflog_fd = -1; continue;
[PATCH 03/11] refs.c: rename the transaction functions
Rename the transaction functions. Remove the leading ref_ from the names and append _sha1 to the names for functions that create/delete/update sha1 refs. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- branch.c | 11 +++ builtin/commit.c | 14 - builtin/fetch.c| 12 builtin/receive-pack.c | 14 - builtin/replace.c | 10 +++ builtin/tag.c | 10 +++ builtin/update-ref.c | 22 +++--- fast-import.c | 23 +++--- refs.c | 81 +- refs.h | 40 - sequencer.c| 12 walker.c | 17 +-- 12 files changed, 133 insertions(+), 133 deletions(-) diff --git a/branch.c b/branch.c index e0439af..6fa6d78 100644 --- a/branch.c +++ b/branch.c @@ -298,13 +298,14 @@ void create_branch(const char *head, struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, ref.buf, sha1, - null_sha1, 0, !forcing, msg, err) || - ref_transaction_commit(transaction, err)) + transaction_update_sha1(transaction, ref.buf, sha1, + null_sha1, 0, !forcing, msg, + err) || + transaction_commit(transaction, err)) die(%s, err.buf); - ref_transaction_free(transaction); + transaction_free(transaction); } if (real_ref track) diff --git a/builtin/commit.c b/builtin/commit.c index e01b333..a54488c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1748,17 +1748,17 @@ int cmd_commit(int argc, const char **argv, const char *prefix) strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg)); strbuf_insert(sb, strlen(reflog_msg), : , 2); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, HEAD, sha1, - current_head ? - current_head-object.sha1 : NULL, - 0, !!current_head, sb.buf, err) || - ref_transaction_commit(transaction, err)) { + transaction_update_sha1(transaction, HEAD, sha1, + current_head ? + current_head-object.sha1 : NULL, + 0, !!current_head, sb.buf, err) || + transaction_commit(transaction, err)) { rollback_index_files(); die(%s, err.buf); } - ref_transaction_free(transaction); + transaction_free(transaction); unlink(git_path(CHERRY_PICK_HEAD)); unlink(git_path(REVERT_HEAD)); diff --git a/builtin/fetch.c b/builtin/fetch.c index 52f1ebc..baf7929 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -385,22 +385,22 @@ static int s_update_ref(const char *action, rla = default_rla.buf; snprintf(msg, sizeof(msg), %s: %s, rla, action); - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, ref-name, ref-new_sha1, - ref-old_sha1, 0, check_old, msg, err)) + transaction_update_sha1(transaction, ref-name, ref-new_sha1, + ref-old_sha1, 0, check_old, msg, err)) goto fail; - ret = ref_transaction_commit(transaction, err); + ret = transaction_commit(transaction, err); if (ret == UPDATE_REFS_NAME_CONFLICT) df_conflict = 1; if (ret) goto fail; - ref_transaction_free(transaction); + transaction_free(transaction); return 0; fail: - ref_transaction_free(transaction); + transaction_free(transaction); error(%s, err.buf); strbuf_release(err); return df_conflict ? STORE_REF_ERROR_DF_CONFLICT diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 5653fa2..6529776 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -583,24 +583,24 @@ static const char *update(struct command *cmd, struct shallow_info *si) update_shallow_ref(cmd, si)) return shallow error; - transaction = ref_transaction_begin(err); + transaction = transaction_begin(err); if (!transaction || - ref_transaction_update(transaction, namespaced_name, -
Re: [PATCH] t9001: avoid not portable '\n' with sed
John Keeping j...@keeping.me.uk writes: Note that quoted section of POSIX says embedded in the pattern space; under the description of the s command, it says: The replacement string shall be scanned from beginning to end. [...] The meaning of a backslash immediately followed by any character other than '', backslash, a digit, or the delimiter character used for this command, is unspecified. Very true. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] clone: add clone.recursesubmodules config option
Am 04.06.2014 19:24, schrieb Junio C Hamano: Chris Packham judge.pack...@gmail.com writes: On 04/06/14 09:05, Junio C Hamano wrote: Also, going --recursive when the user did not want is a lot more expensive mistake to fix than not being --recursive when the user wanted to. Having said all that, I do not mean to say that I am opposed to introduce some mechanism to let the users express their preference between recursive and non-recursive better, so that git clone without an explicit --recursive (or --no-recursive) can work to their taste. A configuration in $HOME/.gitconfig might be a place to start, even though that has the downside of assuming that the given user would want to use the same settings for all his projects, which may not be the case in practice. And here's a quick proof of concept. Not sure about the config variable name and it could probably do with a negative test as well. I would be more worried about the semantics than the name, though; re-read the part you quoted with extra stress on has the downside. I think I heard the submodule folks (cc'ed) discuss an approach to allow various submodules to be marked with tags with a new type of entry in .gitmodules file in the superproject, and use these tags to signal by default, a new clone will recurse into this submodule. E.g. if projects standardized on defaultClone to mark such submodules, then $HOME/.gitconfig could say [clone] recursesubmodules = defaultClone Or the projects may mark platform specific submodules with tags, e.g. a .gitmodules in a typical superproject might say something like this: [submodule posix] path = ports/posix tags = linux obsd fbsd osx [submodule windows] path = ports/windows tags = win32 [submodule doc] path = documentation tags = defaultClone and then the user's $HOME/.gitconfig might say [clone] recursesubmodules = defaultClone win32 to tell a git clone of such a superproject to clone the top-level, read its .gitmodules, and choose documentation/ and ports/windows submodules but not ports/posix submodule to be further cloned into the working tree of the superproject. Of course, if this kind of project organization proves to be useful, we should try to standardize the set of tags early before people start coming up with random variations of the same thing, spelling the same concept in different ways only to be different, and if that happens, then we could even give a non-empty default value for the clone.recursesubmodules when $HOME/.gitconfig is missing one. Yes, but maybe we can define how the user wants to set the global or per-repo default (that is honored as long as upstream or local config doesn't provide more specific settings, e.g. via tags) and implement that for clone as a first step, even when we do not now how e.g. the tags setting might look like in the end. I believe we should have one or two switches telling Git I want my submodules be updated without having to use the 'git submodule' command. And after that submodule specific overrides can kick in, e.g. when submodule.name.update is set to none the submodule won't be updated no matter how the default is. We had two settings in mind, first submodule.autoinit (which would automate the git submodule --init step and also control that a new submodule is fetched into .git/modules; it'd be fetched there soon as the fetch in the superproject sees a commit introducing it). That would kick in on clone, fetch and pull, as the underlying fetch honors it. And the submodule.autoupdate setting which will make running git submodule update obsolete by updating all init'ed submodules on each clone, checkout, merge, reset etc.. Together they'd achieve for all relevant commands what Chris' proposed option would only do for clone. So what if clone would just do an git submodule init for now when submodule.autoinit is set but submodule.autoupdate isn't (and as soon as fetch learns to honor autoinit we could remove that one again). And if both are set it'd do a git submodule update --init --recursive, just like it does when the --recurse-submodules option is used. As soon as we also have recursive submodule update, we could remove the latter from clone. But maybe we are to close to the implementation side of things (where fetch and checkout just like init and update are two separate things) and a single submodule.auto setting would be what users really want? Comments welcome. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2014, #01; Tue, 3)
Am 04.06.2014 00:16, schrieb Junio C Hamano: * jl/status-added-submodule-is-never-ignored (2014-04-07) 2 commits - commit -m: commit staged submodules regardless of ignore config - status/commit: show staged submodules regardless of ignore config There also are a few patches Ronald Weiss and Jens are working on polishing around this topic, and a patch from Jens each for gitk and git-gui. Waiting for the dust to settle until picking them up all. To me it looks like the dust settled enough around that part of the topic and I remember consensus about that change. But it would be nice to have the gitk and git-gui patches in at the same time. * jl/submodule-recursive-checkout (2013-12-26) 5 commits - Teach checkout to recursively checkout submodules - submodule: teach unpack_trees() to update submodules - submodule: teach unpack_trees() to repopulate submodules - submodule: teach unpack_trees() to remove submodule contents - submodule: prepare for recursive checkout of submodules An RFCv2 exists ($gmane/241455) with sizable review comments. Expecting a reroll. Will do, but only after I rerolled the submodule test harness ($gmane/245048) soonish, as I intend to reuse the infrastructure introduced there for tests. * jh/submodule-tests (2014-04-17) 1 commit - t7410: 210 tests for various 'git submodule update' scenarios Will look deeper into that one in the next days, we really need more test coverage in that area. -- To unsubscribe from this list: send the line unsubscribe 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] t9001: avoid not portable '\n' with sed
John Keeping j...@keeping.me.uk writes: So the portable way to do it is: sed s/^From:/$header: ex...@address.com\ From:/ cover-to-edit.patch $cover That wouldn't work as \newline is removed in double quotes. You either need to double the backslash or put it in single quotes. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Re: [RFC PATCH] clone: add clone.recursesubmodules config option
On Wed, Jun 04, 2014 at 10:24:06AM -0700, Junio C Hamano wrote: Chris Packham judge.pack...@gmail.com writes: On 04/06/14 09:05, Junio C Hamano wrote: Also, going --recursive when the user did not want is a lot more expensive mistake to fix than not being --recursive when the user wanted to. Having said all that, I do not mean to say that I am opposed to introduce some mechanism to let the users express their preference between recursive and non-recursive better, so that git clone without an explicit --recursive (or --no-recursive) can work to their taste. A configuration in $HOME/.gitconfig might be a place to start, even though that has the downside of assuming that the given user would want to use the same settings for all his projects, which may not be the case in practice. And here's a quick proof of concept. Not sure about the config variable name and it could probably do with a negative test as well. I would be more worried about the semantics than the name, though; re-read the part you quoted with extra stress on has the downside. I think I heard the submodule folks (cc'ed) discuss an approach to allow various submodules to be marked with tags with a new type of entry in .gitmodules file in the superproject, and use these tags to signal by default, a new clone will recurse into this submodule. E.g. if projects standardized on defaultClone to mark such submodules, then $HOME/.gitconfig could say [clone] recursesubmodules = defaultClone Or the projects may mark platform specific submodules with tags, e.g. a .gitmodules in a typical superproject might say something like this: [submodule posix] path = ports/posix tags = linux obsd fbsd osx [submodule windows] path = ports/windows tags = win32 [submodule doc] path = documentation tags = defaultClone and then the user's $HOME/.gitconfig might say [clone] recursesubmodules = defaultClone win32 to tell a git clone of such a superproject to clone the top-level, read its .gitmodules, and choose documentation/ and ports/windows submodules but not ports/posix submodule to be further cloned into the working tree of the superproject. Of course, if this kind of project organization proves to be useful, we should try to standardize the set of tags early before people start coming up with random variations of the same thing, spelling the same concept in different ways only to be different, and if that happens, then we could even give a non-empty default value for the clone.recursesubmodules when $HOME/.gitconfig is missing one. Just a random thought. I like this idea of specifying different views by giving tags. But does it rule out a boolean clone.recursesubmodules? For the simple case some people might not want to worry about specifying tags but just want to configure: Yes give me everything. So if we were to do this I would like it if we could have both. Also because the option for clone is --recurse-submodules and our typical schema is that a configuration option is named similar so clone.recursesubmodules would fit here. So either we do this magically and all valid boolean values are forbidden as tags or we would need a different config option. Further thinking about it: Maybe a general option that does not only apply to clone would suit the views use-case more. E.g. submodule.tags or similar. Also please note: We have been talking about adding two configurations for submodules: submodule.name.autoclone (IIRC) I am not sure whether that was the correct name, but this option should tell recursive fetch / clone whether to automatically clone a submodule when it appears on a fetch in the history. submodule.name.autoinit And this one is for recursive checkout and tells whether an appearing submodule should automatically be initialized. These options fullfill a similar use-case and are planned for the future when recursive fetch/clone and checkout are in place (which is not that far away). We might need to rethink these to incoporate the views from tags idea nicely and since we do not want a configuration nightmare. Cheers Heiko -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] t9001: avoid not portable '\n' with sed
t9001 used a '\n' in a sed expression to split one line into two lines. Some versions of sed (/usr/bin/sed under Mac OS X) simply ignore the '\' before the 'n', treating '\n' as 'n'. As the test already requires perl as a prerequisite, use perl instead of sed. Signed-off-by: Torsten Bögershausen tbo...@web.de --- t/t9001-send-email.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 64d9434..19a3ced 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -1342,7 +1342,7 @@ test_cover_addresses () { git format-patch --cover-letter -2 -o outdir cover=`echo outdir/-*.patch` mv $cover cover-to-edit.patch - sed s/^From:/$header: ex...@address.com\nFrom:/ cover-to-edit.patch $cover + perl -pe s/^From:/$header: extra\@address.com\nFrom:/ cover-to-edit.patch $cover git send-email \ --force \ --from=Example nob...@example.com \ -- 2.0.0.553.ged01b91 -- To unsubscribe from this list: send the line 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 v5] Add an explicit GIT_DIR to the list of excludes
When an explicit '--git-dir' option points to a directory inside the work tree, git treats it as if it were any other directory. In particular, 'git status' lists it as untracked, while 'git add -A' stages the metadata directory entirely Add GIT_DIR to the list of excludes in a dedicated function add_git_dir_exclude(), while checking that GIT_DIR is not just '.git' or its basename is not '.git', in which cases it would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE. Function add_git_dir_exclude() is invoked at the beginning of dir.c:setup_standard_excludes() Although an analogous comparison of any given path against '.git' is done in treat_path(), this does not seem to be the right place to compare against GIT_DIR. Instead, the excludes provide an effective mechanism of ignoring a file/directory, and adding GIT_DIR as an exclude is equivalent to putting it into '.gitignore'. Function setup_standard_excludes() was chosen because that is the place where the excludes are initialized by the commands that are concerned about excludes Signed-off-by: Pasha Bolokhov pasha.bolok...@gmail.com --- basename() is not needed anymore, as we are excluding a full path GIT_DIR but relative to GIT_WORK_TREE. Now, a full path ensures that nothing else gets excluded (i.e. files with the same basename), and this behaviour is checked in tests Docs (git-grep, git-ls-files, gitignore) will be updated in the subsequent version, wanted to check right now if you agree with code. Docs kind of depend on what we decide about the code Documentation/technical/api-directory-listing.txt | 4 +- dir.c | 33 t/t2205-add-gitdir.sh | 187 ++ 3 files changed, 222 insertions(+), 2 deletions(-) create mode 100755 t/t2205-add-gitdir.sh diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt index 7f8e78d..fd4a178 100644 --- a/Documentation/technical/api-directory-listing.txt +++ b/Documentation/technical/api-directory-listing.txt @@ -90,8 +90,8 @@ marked. If you to exclude files, make sure you have loaded index first. `add_exclude()`. * To add patterns from a file (e.g. `.git/info/exclude`), call - `add_excludes_from_file()` , and/or set `dir.exclude_per_dir`. A - short-hand function `setup_standard_excludes()` can be used to set + `add_excludes_from_file()` , and/or set `dir.exclude_per_dir`. The + short-hand function `setup_standard_excludes()` must be used to set up the standard set of exclude settings. * Set options described in the Data Structure section above. diff --git a/dir.c b/dir.c index eb6f581..300ce1c 100644 --- a/dir.c +++ b/dir.c @@ -1604,11 +1604,44 @@ int remove_dir_recursively(struct strbuf *path, int flag) return remove_dir_recurse(path, flag, NULL); } +static void add_git_dir_exclude(struct dir_struct *dir) +{ + const char *r_git, *gitdir = get_git_dir(); + char *n_git; + int len; + + r_git = real_path(absolute_path(gitdir)); + n_git = xmalloc(strlen(r_git) + 1 + 1); + normalize_path_copy(n_git, r_git); + len = strlen(n_git); + + /* only add it if GIT_DIR does not end with '.git' or '/.git' */ + if (len 4 || strcmp(n_git + len - 4, .git) || + (len 4 n_git[len - 5] != '/')) { + const char *worktree = get_git_work_tree(); + + if (!worktree || + dir_inside_of(n_git, worktree) = 0) { + struct exclude_list *el = add_exclude_list(dir, EXC_CMDL, + GIT_DIR setup); + char *reldir = worktree ? n_git + strlen(worktree) : n_git; + + /* append a trailing slash to exclude directories only */ + n_git[len] = '/'; + n_git[len + 1] = '\0'; + add_exclude(reldir, , 0, el, 0); + } + } + free(n_git); +} + void setup_standard_excludes(struct dir_struct *dir) { const char *path; char *xdg_path; + add_git_dir_exclude(dir); + dir-exclude_per_dir = .gitignore; path = git_path(info/exclude); if (!excludes_file) { diff --git a/t/t2205-add-gitdir.sh b/t/t2205-add-gitdir.sh new file mode 100755 index 000..721970e --- /dev/null +++ b/t/t2205-add-gitdir.sh @@ -0,0 +1,187 @@ +#!/bin/sh +# +# Copyright (c) 2014 Pasha Bolokhov +# + +test_description='alternative repository path specified by --git-dir is ignored by add and status' + +. ./test-lib.sh + +# +# Create a tree: +# +# repo-inside/ repo-outside/ +# +# +# repo-inside: +# a b c d dir1/ dir2/ [meta/] +# +# repo-inside/dir1: +# e f g h meta/ ssubdir/ +# +# repo-inside/dir1/meta: +# aa +# +# repo-inside/dir1/ssubdir: +# meta/ +# +# repo-inside/dir1/ssubdir/meta: +# aaa +# +#
Re: [PATCH] gitweb: Harden UTF-8 handling in generated links
On Wed, Jun 04, 2014 at 08:47:54PM +0200, Jakub Narębski wrote: Michael Wagner wrote: On Tue, May 27, 2014 at 04:22:42PM +0200, Jakub Narębski wrote: Subject: [PATCH] gitweb: Harden UTF-8 handling in generated links esc_html() ensures that its input is properly UTF-8 encoded and marked as UTF-8 with to_utf8(). Make esc_param() (used for query parameters in generated URLs), esc_path_info() (for escaping path_info components) and esc_url() use it too. This hardens gitweb against errors in UTF-8 handling; because to_utf8() is idempotent it won't change correct output. [...] sub esc_param { my $str = shift; return undef unless defined $str; + + $str = to_utf8($str); $str =~ s/([^A-Za-z0-9\-_.~()\/:@ ]+)/CGI::escape($1)/eg; $str =~ s/ /\+/g; + return $str; } While trying to view a blob_plain of Gütekritierien.txt, a 404 error occured. git_get_hash_by_path tries to resolve the hash with the wrong filename (git ls-tree -z HEAD -- Gütekriterien.txt) and fails. The filename needs the correct encoding. Something like this is probably needed for all filenames and should be done at a prior stage: True. First, I wonder why the tests I did for this situation didn't show any errors even before the harden href() patch. What is different in your config that you see those errors? Nothing special. It is reproducible with git 1.9.3 (Fedora 20), git instaweb (lighttpd) and LANG=de_DE.UTF-8. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2014, #01; Tue, 3)
Jens Lehmann jens.lehm...@web.de writes: Am 04.06.2014 00:16, schrieb Junio C Hamano: * jl/status-added-submodule-is-never-ignored (2014-04-07) 2 commits - commit -m: commit staged submodules regardless of ignore config - status/commit: show staged submodules regardless of ignore config There also are a few patches Ronald Weiss and Jens are working on polishing around this topic, and a patch from Jens each for gitk and git-gui. Waiting for the dust to settle until picking them up all. To me it looks like the dust settled enough around that part of the topic and I remember consensus about that change. But it would be nice to have the gitk and git-gui patches in at the same time. Yes, what I meant was that after the dust settled, it may turn out that these two may need to be adjusted. If these two commits can be used without any change as a base for any further development, that is good---shall I move it back to Cooking category? * jl/submodule-recursive-checkout (2013-12-26) 5 commits - Teach checkout to recursively checkout submodules - submodule: teach unpack_trees() to update submodules - submodule: teach unpack_trees() to repopulate submodules - submodule: teach unpack_trees() to remove submodule contents - submodule: prepare for recursive checkout of submodules An RFCv2 exists ($gmane/241455) with sizable review comments. Expecting a reroll. Will do, but only after I rerolled the submodule test harness ($gmane/245048) soonish, as I intend to reuse the infrastructure introduced there for tests. * jh/submodule-tests (2014-04-17) 1 commit - t7410: 210 tests for various 'git submodule update' scenarios Will look deeper into that one in the next days, we really need more test coverage in that area. OK. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2014, #01; Tue, 3)
Am 04.06.2014 22:50, schrieb Junio C Hamano: Jens Lehmann jens.lehm...@web.de writes: Am 04.06.2014 00:16, schrieb Junio C Hamano: * jl/status-added-submodule-is-never-ignored (2014-04-07) 2 commits - commit -m: commit staged submodules regardless of ignore config - status/commit: show staged submodules regardless of ignore config There also are a few patches Ronald Weiss and Jens are working on polishing around this topic, and a patch from Jens each for gitk and git-gui. Waiting for the dust to settle until picking them up all. To me it looks like the dust settled enough around that part of the topic and I remember consensus about that change. But it would be nice to have the gitk and git-gui patches in at the same time. Yes, what I meant was that after the dust settled, it may turn out that these two may need to be adjusted. If these two commits can be used without any change as a base for any further development, that is good---shall I move it back to Cooking category? I'm not aware of any necessary adjustments, so I'd appreciate if they'd be moved back into Cooking. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 00/11] Zsh prompt tests
Changes from v1: * fix a bug that caused the Zsh test cases to run in Zsh's sh emulation mode, not Zsh native mode Description: This series adds test cases for running __git_ps1 (see contrib/completion/git-prompt.sh) from Zsh. This series also adds more Bash test cases to test how __git_ps1 reacts to disabling Bash's PS1 parameter expansion. (This is related to adding Zsh test cases: Zsh doesn't perform parameter expansion on PS1 by default but many users turn it on, so the Zsh test script must test __git_ps1 in both states. Bash expands PS1 by default and users rarely turn it off, but testing both states in Bash improves the symmetry with the Zsh test cases.) This is the approach I took: 1. delete the last test case in t9903 (prompt - zsh color pc mode) 2. add two new functions to t/lib-bash.sh: ps1_expansion_enable () { shopt -s promptvars; } ps1_expansion_disable () { shopt -u promptvars; } 3. loop over the relevant test cases twice: once after calling ps1_expansion_enable and once after calling ps1_expansion_disable (with appropriate adjustments to the expected output) 4. move the test cases in t9903 to a separate library file and source it from t9903-bash-prompt.sh 5. create two new files: * t/lib-zsh.sh (same as t/lib-bash.sh but tweaked for zsh) * t/t9904-zsh-prompt.sh (same as t/t9903-bash-prompt.sh but tweaked for zsh) There are a lot of indendation changes, so I recommend examining the changes via diff -w. Richard Hansen (11): t9903: remove Zsh test from the suite of Bash prompt tests t9903: put the Bash pc mode prompt test cases in a function t9903: move test name prefix to a separate variable t9903: run pc mode tests again with PS1 expansion disabled t9903: include Bash in test names via new $shellname var t9903: move PS1 color code variable definitions to lib-bash.sh t9903: move prompt tests to a new lib-prompt-tests.sh file lib-prompt-tests.sh: put all tests inside a function lib-prompt-tests.sh: add variable for string that encodes percent in PS1 test-lib: make it possible to override how test code is eval'd t9904: new __git_ps1 tests for Zsh t/lib-bash.sh | 12 + t/lib-prompt-tests.sh | 654 + t/lib-zsh.sh | 52 t/t9903-bash-prompt.sh | 582 +-- t/t9904-zsh-prompt.sh | 10 + t/test-lib.sh | 7 +- 6 files changed, 736 insertions(+), 581 deletions(-) create mode 100644 t/lib-prompt-tests.sh create mode 100644 t/lib-zsh.sh create mode 100755 t/t9904-zsh-prompt.sh -- 2.0.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
[PATCH v2 01/11] t9903: remove Zsh test from the suite of Bash prompt tests
This test is about to become redundant: All of the Bash prompt tests will be moved into a separate library file that will also be used by a new Zsh-specific test script. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/t9903-bash-prompt.sh | 11 --- 1 file changed, 11 deletions(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 9150984..335383d 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -577,15 +577,4 @@ test_expect_success 'prompt - bash color pc mode - untracked files status indica test_cmp expected $actual ' -test_expect_success 'prompt - zsh color pc mode' ' - printf BEFORE: (%%F{green}master%%f):AFTER expected - ( - ZSH_VERSION=5.0.0 - GIT_PS1_SHOWCOLORHINTS=y - __git_ps1 BEFORE: :AFTER - printf %s $PS1 $actual - ) - test_cmp expected $actual -' - test_done -- 2.0.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
[PATCH v2 10/11] test-lib: make it possible to override how test code is eval'd
If a command named 'test_eval_override' exists, use it instead of 'eval' to run the test code. This is needed to support zsh test cases: test-lib.sh must be sourced in sh emulation mode due to fundamental incompatibilities between the POSIX sh language and the zsh language. When a function is defined while zsh is emulating some shell, zsh notes the shell that is being emulated and records it along with the function definition. This enables zsh to temporarily re-enable the shell emulation mode whenever the function is called, allowing zsh scripts to mix and match code written for different shell languages. (This description of zsh shell emulation is not completely accurate, but it's close enough.) Because test_eval_ is defined while zsh is in sh emulation mode, the shell code passed as an argument to test_expect_success would normally be evaluated in sh emulation mode. However, with this change, it is now possible to evaluate the test code in zsh mode by adding the following line to a zsh-based test script: emulate -R zsh -c 'test_eval_override () { eval $*; }' With test_eval_override defined in zsh emulation mode, the call to test_eval_override from test_eval_ will temporarily cause zsh to switch from sh emulation mode to zsh emulation mode. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/test-lib.sh | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index c081668..3779634 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -414,7 +414,12 @@ maybe_setup_valgrind () { test_eval_ () { # This is a separate function because some tests use # return to end a test_expect_success block early. - eval /dev/null 3 24 $* + if command -v test_eval_override /dev/null 21 + then + test_eval_override $* + else + eval $* + fi /dev/null 3 24 } test_run_ () { -- 2.0.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
[PATCH v2 04/11] t9903: run pc mode tests again with PS1 expansion disabled
Bash has a shell option that makes it possible to disable parameter expansion in PS1. Test __git_ps1's ability to detect and react to disabled PS1 expansion by running the pc mode tests twice: once with PS1 parameter expansion enabled and once with it disabled. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/lib-bash.sh | 3 ++ t/t9903-bash-prompt.sh | 96 ++ 2 files changed, 76 insertions(+), 23 deletions(-) diff --git a/t/lib-bash.sh b/t/lib-bash.sh index 2be955f..37a48fd 100644 --- a/t/lib-bash.sh +++ b/t/lib-bash.sh @@ -14,4 +14,7 @@ else exit 0 fi +ps1_expansion_enable () { shopt -s promptvars; } +ps1_expansion_disable () { shopt -u promptvars; } + . ./test-lib.sh diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index d29dd2b..eb5a167 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -451,57 +451,97 @@ test_expect_success 'prompt - format string starting with dash' ' test_cmp expected $actual ' -run_pcmode_tests () { - test_expect_success 'prompt - pc mode' ' - printf BEFORE: (\${__git_ps1_branch_name}):AFTER\\nmaster expected +pcmode_expected () { + case $ps1expansion in + on) printf $1 '${__git_ps1_branch_name}' $2;; + off) printf $1 $2 ;; + esac expected +} + +pcmode_actual () { + case $ps1expansion in + on) printf %s\\n%s $PS1 ${__git_ps1_branch_name};; + off) printf %s\\n $PS1;; + esac $actual +} + +set_ps1expansion () { + case $ps1expansion in + on) ps1_expansion_enable;; + off) ps1_expansion_disable;; + *) error invalid argument to _run_pcmode_tests: $ps1expansion;; + esac +} + +_run_pcmode_tests () { + ps1expansion=$1; shift + + # Test whether the shell supports enabling/disabling PS1 + # expansion by running set_ps1expansion. If not, quietly skip + # this set of tests. + # + # Even though set_ps1expansion is run here, it must also be + # run inside each individual test case because the state of + # the shell might be reset in some fashion before executing + # the test code. (Notably, Zsh shell emulation causes the + # PROMPT_SUBST option to be reset each time a test is run.) + set_ps1expansion || return 0 + + test_expect_success prompt - pc mode (PS1 expansion $ps1expansion) ' + set_ps1expansion + pcmode_expected BEFORE: (%s):AFTER\\n%s master printf expected_output ( __git_ps1 BEFORE: :AFTER $actual test_cmp expected_output $actual - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual + pcmode_actual ) test_cmp expected $actual ' - pfx=prompt - bash color pc mode + pfx=prompt - bash color pc mode (PS1 expansion $ps1expansion) test_expect_success $pfx - branch name ' - printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmaster expected + set_ps1expansion + pcmode_expected BEFORE: (${c_green}%s${c_clear}):AFTER\\n%s master ( GIT_PS1_SHOWCOLORHINTS=y __git_ps1 BEFORE: :AFTER $actual - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual + pcmode_actual ) test_cmp expected $actual ' test_expect_success $pfx - detached head ' - printf BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...) $(git log -1 --format=%h b1^) expected + set_ps1expansion + pcmode_expected BEFORE: (${c_red}%s${c_clear}):AFTER\\n%s ($(git log -1 --format=%h b1^)...) git checkout b1^ test_when_finished git checkout master ( GIT_PS1_SHOWCOLORHINTS=y __git_ps1 BEFORE: :AFTER - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual + pcmode_actual ) test_cmp expected $actual ' test_expect_success $pfx - dirty status indicator - dirty worktree ' - printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmaster expected + set_ps1expansion + pcmode_expected BEFORE: (${c_green}%s${c_clear} ${c_red}*${c_clear}):AFTER\\n%s master echo dirty file test_when_finished git reset --hard ( GIT_PS1_SHOWDIRTYSTATE=y GIT_PS1_SHOWCOLORHINTS=y __git_ps1 BEFORE: :AFTER - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual +
[PATCH v2 03/11] t9903: move test name prefix to a separate variable
This is a step toward reusing the same test cases after disabling PS1 parameter expansion. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/t9903-bash-prompt.sh | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index c691869..d29dd2b 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -463,7 +463,9 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - branch name' ' + pfx=prompt - bash color pc mode + + test_expect_success $pfx - branch name ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmaster expected ( GIT_PS1_SHOWCOLORHINTS=y @@ -473,7 +475,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - detached head' ' + test_expect_success $pfx - detached head ' printf BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...) $(git log -1 --format=%h b1^) expected git checkout b1^ test_when_finished git checkout master @@ -485,7 +487,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' + test_expect_success $pfx - dirty status indicator - dirty worktree ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmaster expected echo dirty file test_when_finished git reset --hard @@ -498,7 +500,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' + test_expect_success $pfx - dirty status indicator - dirty index ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmaster expected echo dirty file test_when_finished git reset --hard @@ -512,7 +514,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index and worktree' ' + test_expect_success $pfx - dirty status indicator - dirty index and worktree ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_green}+${c_clear}):AFTER\\nmaster expected echo dirty index file test_when_finished git reset --hard @@ -527,7 +529,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - dirty status indicator - before root commit' ' + test_expect_success $pfx - dirty status indicator - before root commit ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}#${c_clear}):AFTER\\nmaster expected ( GIT_PS1_SHOWDIRTYSTATE=y @@ -539,7 +541,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - inside .git directory' ' + test_expect_success $pfx - inside .git directory ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nGIT_DIR! expected echo dirty file test_when_finished git reset --hard @@ -553,7 +555,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - stash status indicator' ' + test_expect_success $pfx - stash status indicator ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_lblue}\$${c_clear}):AFTER\\nmaster expected echo 2 file git stash @@ -567,7 +569,7 @@ run_pcmode_tests () { test_cmp expected $actual ' - test_expect_success 'prompt - bash color pc mode - untracked files status indicator' ' + test_expect_success $pfx - untracked files status indicator ' printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}%%${c_clear}):AFTER\\nmaster expected ( GIT_PS1_SHOWUNTRACKEDFILES=y -- 2.0.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
[PATCH v2 02/11] t9903: put the Bash pc mode prompt test cases in a function
This is a step toward invoking the same pc mode test cases twice: once with PS1 parameter expansion enabled and once with it disabled. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/t9903-bash-prompt.sh | 236 + 1 file changed, 120 insertions(+), 116 deletions(-) diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 335383d..c691869 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -451,130 +451,134 @@ test_expect_success 'prompt - format string starting with dash' ' test_cmp expected $actual ' -test_expect_success 'prompt - pc mode' ' - printf BEFORE: (\${__git_ps1_branch_name}):AFTER\\nmaster expected - printf expected_output - ( - __git_ps1 BEFORE: :AFTER $actual - test_cmp expected_output $actual - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual - ) - test_cmp expected $actual -' +run_pcmode_tests () { + test_expect_success 'prompt - pc mode' ' + printf BEFORE: (\${__git_ps1_branch_name}):AFTER\\nmaster expected + printf expected_output + ( + __git_ps1 BEFORE: :AFTER $actual + test_cmp expected_output $actual + printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual + ) + test_cmp expected $actual + ' -test_expect_success 'prompt - bash color pc mode - branch name' ' - printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmaster expected - ( - GIT_PS1_SHOWCOLORHINTS=y - __git_ps1 BEFORE: :AFTER $actual - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual - ) - test_cmp expected $actual -' + test_expect_success 'prompt - bash color pc mode - branch name' ' + printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear}):AFTER\\nmaster expected + ( + GIT_PS1_SHOWCOLORHINTS=y + __git_ps1 BEFORE: :AFTER $actual + printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual + ) + test_cmp expected $actual + ' -test_expect_success 'prompt - bash color pc mode - detached head' ' - printf BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...) $(git log -1 --format=%h b1^) expected - git checkout b1^ - test_when_finished git checkout master - ( - GIT_PS1_SHOWCOLORHINTS=y - __git_ps1 BEFORE: :AFTER - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual - ) - test_cmp expected $actual -' + test_expect_success 'prompt - bash color pc mode - detached head' ' + printf BEFORE: (${c_red}\${__git_ps1_branch_name}${c_clear}):AFTER\\n(%s...) $(git log -1 --format=%h b1^) expected + git checkout b1^ + test_when_finished git checkout master + ( + GIT_PS1_SHOWCOLORHINTS=y + __git_ps1 BEFORE: :AFTER + printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual + ) + test_cmp expected $actual + ' -test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' - printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmaster expected - echo dirty file - test_when_finished git reset --hard - ( - GIT_PS1_SHOWDIRTYSTATE=y - GIT_PS1_SHOWCOLORHINTS=y - __git_ps1 BEFORE: :AFTER - printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual - ) - test_cmp expected $actual -' + test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty worktree' ' + printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_red}*${c_clear}):AFTER\\nmaster expected + echo dirty file + test_when_finished git reset --hard + ( + GIT_PS1_SHOWDIRTYSTATE=y + GIT_PS1_SHOWCOLORHINTS=y + __git_ps1 BEFORE: :AFTER + printf %s\\n%s $PS1 ${__git_ps1_branch_name} $actual + ) + test_cmp expected $actual + ' -test_expect_success 'prompt - bash color pc mode - dirty status indicator - dirty index' ' - printf BEFORE: (${c_green}\${__git_ps1_branch_name}${c_clear} ${c_green}+${c_clear}):AFTER\\nmaster expected - echo dirty file - test_when_finished git reset --hard - git add -u - ( - GIT_PS1_SHOWDIRTYSTATE=y - GIT_PS1_SHOWCOLORHINTS=y - __git_ps1 BEFORE: :AFTER - printf %s\\n%s $PS1
[PATCH v2 06/11] t9903: move PS1 color code variable definitions to lib-bash.sh
Define a new 'set_ps1_format_vars' function in lib-bash.sh that sets the c_red, c_green, c_lblue, and c_clear variables. Call this function from run_pcmode_tests(). This is a step toward moving the shell prompt tests to a separate library file so that they can be reused to test prompting in Zsh. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/lib-bash.sh | 6 ++ t/t9903-bash-prompt.sh | 5 + 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/t/lib-bash.sh b/t/lib-bash.sh index a0f4e16..9d428bd 100644 --- a/t/lib-bash.sh +++ b/t/lib-bash.sh @@ -18,5 +18,11 @@ shellname=Bash ps1_expansion_enable () { shopt -s promptvars; } ps1_expansion_disable () { shopt -u promptvars; } +set_ps1_format_vars () { + c_red='\\[\\e[31m\\]' + c_green='\\[\\e[32m\\]' + c_lblue='\\[\\e[1;34m\\]' + c_clear='\\[\\e[0m\\]' +} . ./test-lib.sh diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 27135a1..fe60cf9 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -10,10 +10,7 @@ test_description='test git-specific bash prompt functions' . $GIT_BUILD_DIR/contrib/completion/git-prompt.sh actual=$TRASH_DIRECTORY/actual -c_red='\\[\\e[31m\\]' -c_green='\\[\\e[32m\\]' -c_lblue='\\[\\e[1;34m\\]' -c_clear='\\[\\e[0m\\]' +set_ps1_format_vars test_expect_success setup for $shellname prompt tests ' git init otherrepo -- 2.0.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
[PATCH v2 05/11] t9903: include Bash in test names via new $shellname var
Define a new 'shellname' variable in lib-bash.sh and use it in the prompt test names. This is a step toward moving the shell prompt tests to a separate library file so that they can be reused to test prompting in Zsh. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/lib-bash.sh | 2 ++ t/t9903-bash-prompt.sh | 86 ++ 2 files changed, 46 insertions(+), 42 deletions(-) diff --git a/t/lib-bash.sh b/t/lib-bash.sh index 37a48fd..a0f4e16 100644 --- a/t/lib-bash.sh +++ b/t/lib-bash.sh @@ -14,6 +14,8 @@ else exit 0 fi +shellname=Bash + ps1_expansion_enable () { shopt -s promptvars; } ps1_expansion_disable () { shopt -u promptvars; } diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index eb5a167..27135a1 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -15,7 +15,7 @@ c_green='\\[\\e[32m\\]' c_lblue='\\[\\e[1;34m\\]' c_clear='\\[\\e[0m\\]' -test_expect_success 'setup for prompt tests' ' +test_expect_success setup for $shellname prompt tests ' git init otherrepo echo 1 file git add file @@ -38,13 +38,15 @@ test_expect_success 'setup for prompt tests' ' git checkout master ' -test_expect_success 'prompt - branch name' ' +pfx=$shellname prompt + +test_expect_success $pfx - branch name ' printf (master) expected __git_ps1 $actual test_cmp expected $actual ' -test_expect_success SYMLINKS 'prompt - branch name - symlink symref' ' +test_expect_success SYMLINKS $pfx - branch name - symlink symref ' printf (master) expected test_when_finished git checkout master test_config core.preferSymlinkRefs true @@ -53,7 +55,7 @@ test_expect_success SYMLINKS 'prompt - branch name - symlink symref' ' test_cmp expected $actual ' -test_expect_success 'prompt - unborn branch' ' +test_expect_success $pfx - unborn branch ' printf (unborn) expected git checkout --orphan unborn test_when_finished git checkout master @@ -72,7 +74,7 @@ else say 'Your filesystem does not allow newlines in filenames.' fi -test_expect_success FUNNYNAMES 'prompt - with newline in path' ' +test_expect_success FUNNYNAMES $pfx - with newline in path ' printf (master) expected git init $repo_with_newline test_when_finished rm -rf \$repo_with_newline\ @@ -84,7 +86,7 @@ test_expect_success FUNNYNAMES 'prompt - with newline in path' ' test_cmp expected $actual ' -test_expect_success 'prompt - detached head' ' +test_expect_success $pfx - detached head ' printf ((%s...)) $(git log -1 --format=%h --abbrev=13 b1^) expected test_config core.abbrev 13 git checkout b1^ @@ -93,7 +95,7 @@ test_expect_success 'prompt - detached head' ' test_cmp expected $actual ' -test_expect_success 'prompt - describe detached head - contains' ' +test_expect_success $pfx - describe detached head - contains ' printf ((t2~1)) expected git checkout b1^ test_when_finished git checkout master @@ -104,7 +106,7 @@ test_expect_success 'prompt - describe detached head - contains' ' test_cmp expected $actual ' -test_expect_success 'prompt - describe detached head - branch' ' +test_expect_success $pfx - describe detached head - branch ' printf ((b1~1)) expected git checkout b1^ test_when_finished git checkout master @@ -115,7 +117,7 @@ test_expect_success 'prompt - describe detached head - branch' ' test_cmp expected $actual ' -test_expect_success 'prompt - describe detached head - describe' ' +test_expect_success $pfx - describe detached head - describe ' printf ((t1-1-g%s)) $(git log -1 --format=%h b1^) expected git checkout b1^ test_when_finished git checkout master @@ -126,7 +128,7 @@ test_expect_success 'prompt - describe detached head - describe' ' test_cmp expected $actual ' -test_expect_success 'prompt - describe detached head - default' ' +test_expect_success $pfx - describe detached head - default ' printf ((t2)) expected git checkout --detach b1 test_when_finished git checkout master @@ -134,7 +136,7 @@ test_expect_success 'prompt - describe detached head - default' ' test_cmp expected $actual ' -test_expect_success 'prompt - inside .git directory' ' +test_expect_success $pfx - inside .git directory ' printf (GIT_DIR!) expected ( cd .git @@ -143,7 +145,7 @@ test_expect_success 'prompt - inside .git directory' ' test_cmp expected $actual ' -test_expect_success 'prompt - deep inside .git directory' ' +test_expect_success $pfx - deep inside .git directory ' printf (GIT_DIR!) expected ( cd .git/refs/heads @@ -152,7 +154,7 @@ test_expect_success 'prompt - deep inside .git directory' ' test_cmp expected $actual '
[PATCH v2 07/11] t9903: move prompt tests to a new lib-prompt-tests.sh file
This is a step toward creating a new test script that runs the same prompt tests as t9903 but with Zsh instead of Bash. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/lib-prompt-tests.sh | 653 + t/t9903-bash-prompt.sh | 626 +-- 2 files changed, 654 insertions(+), 625 deletions(-) create mode 100644 t/lib-prompt-tests.sh diff --git a/t/lib-prompt-tests.sh b/t/lib-prompt-tests.sh new file mode 100644 index 000..7aaeb8e --- /dev/null +++ b/t/lib-prompt-tests.sh @@ -0,0 +1,653 @@ +# Copyright (c) 2012 SZEDER Gábor + +# To use this library: +# 1. set the variable shellname to the name of the shell (e.g., +# Bash) +# 2. define functions named ps1_expansion_enable and +# ps1_expansion_disable that, upon return, guarantee that the +# shell will and will not (respectively) perform parameter +# expansion on PS1, if supported by the shell. If it is not +# possible to configure the shell to disable (enable) PS1 +# expansion, ps1_expansion_enable should simply return 0 +# (non-zero) and ps1_expansion_disable should simply return +# non-zero (0) +# 3. define a function named set_ps1_format_vars that sets the +# variables c_red, c_green, c_lblue, and c_clear to the strings +# that __git_ps1 uses to add color to the prompt. The values of +# these variables are used in the first argument to the printf +# command, so they must be escaped appropriately. +# 4. source this library + +# sanity checks +[ -n $shellname ] || error shellname must be set to the name of the shell +for i in ps1_expansion_enable ps1_expansion_disable set_ps1_format_vars +do + command -v $i /dev/null 21 || error function $i not defined +done +(ps1_expansion_enable || ps1_expansion_disable) \ + || error either ps1_expansion_enable or ps1_expansion_disable must return true + +. $GIT_BUILD_DIR/contrib/completion/git-prompt.sh + +actual=$TRASH_DIRECTORY/actual +set_ps1_format_vars + +test_expect_success setup for $shellname prompt tests ' + git init otherrepo + echo 1 file + git add file + test_tick + git commit -m initial + git tag -a -m msg1 t1 + git checkout -b b1 + echo 2 file + git commit -m second b1 file + echo 3 file + git commit -m third b1 file + git tag -a -m msg2 t2 + git checkout -b b2 master + echo 0 file + git commit -m second b2 file + echo 00 file + git commit -m another b2 file + echo 000 file + git commit -m yet another b2 file + git checkout master +' + +pfx=$shellname prompt + +test_expect_success $pfx - branch name ' + printf (master) expected + __git_ps1 $actual + test_cmp expected $actual +' + +test_expect_success SYMLINKS $pfx - branch name - symlink symref ' + printf (master) expected + test_when_finished git checkout master + test_config core.preferSymlinkRefs true + git checkout master + __git_ps1 $actual + test_cmp expected $actual +' + +test_expect_success $pfx - unborn branch ' + printf (unborn) expected + git checkout --orphan unborn + test_when_finished git checkout master + __git_ps1 $actual + test_cmp expected $actual +' + +repo_with_newline='repo +with +newline' + +if mkdir $repo_with_newline 2/dev/null +then + test_set_prereq FUNNYNAMES +else + say 'Your filesystem does not allow newlines in filenames.' +fi + +test_expect_success FUNNYNAMES $pfx - with newline in path ' + printf (master) expected + git init $repo_with_newline + test_when_finished rm -rf \$repo_with_newline\ + mkdir $repo_with_newline/subdir + ( + cd $repo_with_newline/subdir + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success $pfx - detached head ' + printf ((%s...)) $(git log -1 --format=%h --abbrev=13 b1^) expected + test_config core.abbrev 13 + git checkout b1^ + test_when_finished git checkout master + __git_ps1 $actual + test_cmp expected $actual +' + +test_expect_success $pfx - describe detached head - contains ' + printf ((t2~1)) expected + git checkout b1^ + test_when_finished git checkout master + ( + GIT_PS1_DESCRIBE_STYLE=contains + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success $pfx - describe detached head - branch ' + printf ((b1~1)) expected + git checkout b1^ + test_when_finished git checkout master + ( + GIT_PS1_DESCRIBE_STYLE=branch + __git_ps1 $actual + ) + test_cmp expected $actual +' + +test_expect_success $pfx - describe detached head - describe ' + printf ((t1-1-g%s)) $(git log -1
[PATCH v2 11/11] t9904: new __git_ps1 tests for Zsh
These are the same tests as in t9903, but run in zsh instead of bash. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/lib-zsh.sh | 52 +++ t/t9904-zsh-prompt.sh | 10 ++ 2 files changed, 62 insertions(+) create mode 100644 t/lib-zsh.sh create mode 100755 t/t9904-zsh-prompt.sh diff --git a/t/lib-zsh.sh b/t/lib-zsh.sh new file mode 100644 index 000..1fd69fd --- /dev/null +++ b/t/lib-zsh.sh @@ -0,0 +1,52 @@ +# Shell library sourced instead of ./test-lib.sh by tests that need to +# run under Zsh; primarily intended for tests of the git-prompt.sh +# script. + +if test -n $ZSH_VERSION test -z $POSIXLY_CORRECT [[ ! -o FUNCTION_ARGZERO ]]; then + true +elif command -v zsh /dev/null 21; then + unset POSIXLY_CORRECT + # Run Zsh with the FUNCTION_ARGZERO option disabled so that + # test-lib.sh sees the test script pathname when it examines + # $0 instead of ./lib-zsh.sh. (This works around a Zsh + # limitation: 'emulate sh -c' does not restore $0 to the value + # specified by POSIX.) + exec zsh +o FUNCTION_ARGZERO $0 $@ +else + echo '1..0 #SKIP skipping Zsh-specific tests; zsh not available' + exit 0 +fi + +# ensure that we are in full-on Zsh mode. note: this re-enables the +# FUNCTION_ARGZERO option +emulate -R zsh || exit 1 + +shellname=Zsh + +ps1_expansion_enable () { setopt PROMPT_SUBST; } +ps1_expansion_disable () { unsetopt PROMPT_SUBST; } +set_ps1_format_vars () { + percent='' + c_red='%%F{red}' + c_green='%%F{green}' + c_lblue='%%F{blue}' + c_clear='%%f' +} + +# Due to language incompatibilities between POSIX sh and Zsh, +# test-lib.sh must be sourced in sh emulation mode. +# +# Note: Although the FUNCTION_ARGZERO option is currently enabled, sh +# emulation mode temporarily turns it off ($0 is left alone when +# sourcing test-lib.sh) +emulate -R sh -c '. ./test-lib.sh' + +# Ensure that the test code is run in Zsh mode. Because test_eval_() +# was defined by test-lib.sh inside the above 'emulate sh -c', the Zsh +# shell options that implement sh emulation will be temporarily +# toggled when test_eval_() executes. Normally this would cause the +# test code to run in sh emulation mode, not Zsh mode. By defining +# test_eval_override() in zsh emulation mode, the options are +# temporarily toggled back to the Zsh defaults when evaluating the +# test code. +emulate -R zsh -c 'test_eval_override () { eval $*; }' diff --git a/t/t9904-zsh-prompt.sh b/t/t9904-zsh-prompt.sh new file mode 100755 index 000..a38a3fd --- /dev/null +++ b/t/t9904-zsh-prompt.sh @@ -0,0 +1,10 @@ +#!/bin/sh + +test_description='test git-specific Zsh prompt functions' + +. ./lib-zsh.sh +. $TEST_DIRECTORY/lib-prompt-tests.sh + +run_prompt_tests + +test_done -- 2.0.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
[PATCH v2 08/11] lib-prompt-tests.sh: put all tests inside a function
Modify lib-prompt-tests.sh so that it does nothing when sourced except define a function for running the prompt tests (plus some private helper functions). Signed-off-by: Richard Hansen rhan...@bbn.com --- t/lib-prompt-tests.sh | 802 - t/t9903-bash-prompt.sh | 2 + 2 files changed, 403 insertions(+), 401 deletions(-) diff --git a/t/lib-prompt-tests.sh b/t/lib-prompt-tests.sh index 7aaeb8e..ba22acc 100644 --- a/t/lib-prompt-tests.sh +++ b/t/lib-prompt-tests.sh @@ -17,6 +17,7 @@ # these variables are used in the first argument to the printf # command, so they must be escaped appropriately. # 4. source this library +# 5. invoke the run_prompt_tests function # sanity checks [ -n $shellname ] || error shellname must be set to the name of the shell @@ -27,448 +28,445 @@ done (ps1_expansion_enable || ps1_expansion_disable) \ || error either ps1_expansion_enable or ps1_expansion_disable must return true -. $GIT_BUILD_DIR/contrib/completion/git-prompt.sh +_run_non_pcmode_tests () { + test_expect_success setup for $shellname prompt tests ' + git init otherrepo + echo 1 file + git add file + test_tick + git commit -m initial + git tag -a -m msg1 t1 + git checkout -b b1 + echo 2 file + git commit -m second b1 file + echo 3 file + git commit -m third b1 file + git tag -a -m msg2 t2 + git checkout -b b2 master + echo 0 file + git commit -m second b2 file + echo 00 file + git commit -m another b2 file + echo 000 file + git commit -m yet another b2 file + git checkout master + ' -actual=$TRASH_DIRECTORY/actual -set_ps1_format_vars + pfx=$shellname prompt -test_expect_success setup for $shellname prompt tests ' - git init otherrepo - echo 1 file - git add file - test_tick - git commit -m initial - git tag -a -m msg1 t1 - git checkout -b b1 - echo 2 file - git commit -m second b1 file - echo 3 file - git commit -m third b1 file - git tag -a -m msg2 t2 - git checkout -b b2 master - echo 0 file - git commit -m second b2 file - echo 00 file - git commit -m another b2 file - echo 000 file - git commit -m yet another b2 file - git checkout master -' + test_expect_success $pfx - branch name ' + printf (master) expected + __git_ps1 $actual + test_cmp expected $actual + ' -pfx=$shellname prompt + test_expect_success SYMLINKS $pfx - branch name - symlink symref ' + printf (master) expected + test_when_finished git checkout master + test_config core.preferSymlinkRefs true + git checkout master + __git_ps1 $actual + test_cmp expected $actual + ' -test_expect_success $pfx - branch name ' - printf (master) expected - __git_ps1 $actual - test_cmp expected $actual -' + test_expect_success $pfx - unborn branch ' + printf (unborn) expected + git checkout --orphan unborn + test_when_finished git checkout master + __git_ps1 $actual + test_cmp expected $actual + ' -test_expect_success SYMLINKS $pfx - branch name - symlink symref ' - printf (master) expected - test_when_finished git checkout master - test_config core.preferSymlinkRefs true - git checkout master - __git_ps1 $actual - test_cmp expected $actual -' - -test_expect_success $pfx - unborn branch ' - printf (unborn) expected - git checkout --orphan unborn - test_when_finished git checkout master - __git_ps1 $actual - test_cmp expected $actual -' - -repo_with_newline='repo + repo_with_newline='repo with newline' -if mkdir $repo_with_newline 2/dev/null -then - test_set_prereq FUNNYNAMES -else - say 'Your filesystem does not allow newlines in filenames.' -fi + if mkdir $repo_with_newline 2/dev/null + then + test_set_prereq FUNNYNAMES + else + say 'Your filesystem does not allow newlines in filenames.' + fi -test_expect_success FUNNYNAMES $pfx - with newline in path ' - printf (master) expected - git init $repo_with_newline - test_when_finished rm -rf \$repo_with_newline\ - mkdir $repo_with_newline/subdir - ( - cd $repo_with_newline/subdir - __git_ps1 $actual - ) - test_cmp expected $actual -' + test_expect_success FUNNYNAMES $pfx - with newline in path
[PATCH v2 09/11] lib-prompt-tests.sh: add variable for string that encodes percent in PS1
To add a literal percent character to a Zsh prompt, the string %% is used in PS1. Bash and POSIX shells simply use %. To accommodate this difference, use ${percent} where a percent character is expected and define the percent variable in the set_ps1_format_vars function. Signed-off-by: Richard Hansen rhan...@bbn.com --- t/lib-bash.sh | 1 + t/lib-prompt-tests.sh | 15 --- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/t/lib-bash.sh b/t/lib-bash.sh index 9d428bd..8a030ac 100644 --- a/t/lib-bash.sh +++ b/t/lib-bash.sh @@ -19,6 +19,7 @@ shellname=Bash ps1_expansion_enable () { shopt -s promptvars; } ps1_expansion_disable () { shopt -u promptvars; } set_ps1_format_vars () { + percent='%%' c_red='\\[\\e[31m\\]' c_green='\\[\\e[32m\\]' c_lblue='\\[\\e[1;34m\\]' diff --git a/t/lib-prompt-tests.sh b/t/lib-prompt-tests.sh index ba22acc..c6226b1 100644 --- a/t/lib-prompt-tests.sh +++ b/t/lib-prompt-tests.sh @@ -12,10 +12,11 @@ # (non-zero) and ps1_expansion_disable should simply return # non-zero (0) # 3. define a function named set_ps1_format_vars that sets the -# variables c_red, c_green, c_lblue, and c_clear to the strings -# that __git_ps1 uses to add color to the prompt. The values of -# these variables are used in the first argument to the printf -# command, so they must be escaped appropriately. +# variables percent, c_red, c_green, c_lblue, and c_clear to the +# strings that __git_ps1 uses to add percent characters and color +# to the prompt. The values of these variables are used in the +# first argument to the printf command, so they must be escaped +# appropriately. # 4. source this library # 5. invoke the run_prompt_tests function @@ -403,7 +404,7 @@ newline' ' test_expect_success $pfx - untracked files status indicator - untracked files ' - printf (master %%) expected + printf (master ${percent}) expected ( GIT_PS1_SHOWUNTRACKEDFILES=y __git_ps1 $actual @@ -442,7 +443,7 @@ newline' ' test_expect_success $pfx - untracked files status indicator - shell variable set with config enabled ' - printf (master %%) expected + printf (master ${percent}) expected test_config bash.showUntrackedFiles true ( GIT_PS1_SHOWUNTRACKEDFILES=y @@ -632,7 +633,7 @@ _run_pcmode_tests () { test_expect_success $pfx - untracked files status indicator ' set_ps1expansion - pcmode_expected BEFORE: (${c_green}%s${c_clear} ${c_red}%%${c_clear}):AFTER\\n%s master + pcmode_expected BEFORE: (${c_green}%s${c_clear} ${c_red}${percent}${c_clear}):AFTER\\n%s master ( GIT_PS1_SHOWUNTRACKEDFILES=y GIT_PS1_SHOWCOLORHINTS=y -- 2.0.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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component
On Wed, 2014-06-04 at 10:04 +0200, Torsten Bögershausen wrote: [snip discussion of compiler flags; I'll look into a cpuid approach] --- a/git-compat-util.h +++ b/git-compat-util.h @@ -668,6 +668,28 @@ void git_qsort(void *base, size_t nmemb, size_t size, #endif #endif +#ifndef NO_SSE42 +#include nmmintrin.h +/* + * Clang ships with a version of nmmintrin.h that's incomplete; if + * necessary, we define the constants that we're going to use. + */ +#ifndef _SIDD_UBYTE_OPS +#define _SIDD_UBYTE_OPS 0x00 +#define _SIDD_CMP_EQUAL_ANY 0x00 +#define _SIDD_CMP_RANGES0x04 +#define _SIDD_CMP_EQUAL_ORDERED 0x0c +#define _SIDD_NEGATIVE_POLARITY 0x10 +#endif Why do this defines end up in git-compat-util.h when they are needed by one file? (see even below) Because Junio told me to: We would prefer not to add inclusion of any system header files in random *.c files, as there often are system dependencies (order of inclusion, definition of feature macros, etc.) we would rather want to encapsulate in one place, that is git-compat-util.h. --- a/refs.c +++ b/refs.c @@ -24,6 +24,25 @@ static unsigned char refname_disposition[256] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4 }; +static int check_refname_component_trailer(const char *cp, const char *refname, int flags) +{ + if (cp == refname) + return 0; /* Component has zero length. */ + if (refname[0] == '.') { + if (!(flags REFNAME_DOT_COMPONENT)) + return -1; /* Component starts with '.'. */ + /* +* Even if leading dots are allowed, don't allow . +* as a component (.. is prevented by a rule above). +*/ + if (refname[1] == '\0') + return -1; /* Component equals .. */ + } + if (cp - refname = 5 !memcmp(cp - 5, .lock, 5)) + return -1; /* Refname ends with .lock. */ + return cp - refname; +} + /* * Try to read one refname component from the front of refname. * Return the length of the component found, or -1 if the component is @@ -37,7 +56,7 @@ static unsigned char refname_disposition[256] = { * - it ends with .lock * - it contains a \ (backslash) */ -static int check_refname_component(const char *refname, int flags) +static int check_refname_component_1(const char *refname, int flags) The name check_refname_component_1() doesn't tell too much, (check_refname_component_sse42() or check_refname_component_nonsse42() say more) I'll go with _bytewise, since that's how it works. can I suggest to move all SSE code out to a file under compat/, like compat/refs_sse42.c, or something similar ? Since this is a relatively small section of code, I think that would be overkill. Does anyone else have an opinion? -- To unsubscribe from this list: send the line unsubscribe 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component
On Wed, 2014-06-04 at 16:25 +0200, Torsten Bögershausen wrote: On the other hand, looking here: http://sourceware.org/ml/libc-alpha/2009-10/msg00063.html and looking into refs.c, it seems as if we can try to run strcspn(refname, bad_characters) and strstr(refname, @{ and strstr(refname, .. on each refname, instead of checking each char in a loop. The library will pick the fastest version for strcspn() automatically. Yes, you could try that, but I worry that it would be less efficient, because it duplicates the looping machinery. David, the repo you run the tests on, is it public? Unfortunately, it is an internal Twitter repo. Or is there a public repo with this many refs ? I do not know of one. Or can you make a dummy repo with 60k refs ? Sure! I actually went with 120k to make measurement easier: https://github.com/dturner-tw/many-refs -- To unsubscribe from this list: send the line unsubscribe 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 v6 2/2] refs.c: SSE4.2 optimizations for check_refname_component
David Turner dtur...@twopensource.com writes: On Wed, 2014-06-04 at 10:04 +0200, Torsten Bögershausen wrote: [snip discussion of compiler flags; I'll look into a cpuid approach] H, I am not sure if the complexity is really worth it. In any case, [PATCH 1/2] is fairly uncontroversial, so I am inclined to queue it by itself early without waiting for the discussion on 2/2 to settle. The name check_refname_component_1() doesn't tell too much, (check_refname_component_sse42() or check_refname_component_nonsse42() say more) I'll go with _bytewise, since that's how it works. That naming assumes that there will never be any alternative implementation of the bytewise checker other than the one that uses sse42, no? can I suggest to move all SSE code out to a file under compat/, like compat/refs_sse42.c, or something similar ? Since this is a relatively small section of code, I think that would be overkill. Does anyone else have an opinion? If we foresee people on other architectures to invent different vectorized implementations on their favourite archs, we may end up separating it out into compat/. I have no opinion on how likely that will happen, though, and because this is a small piece of code right now, it shouldn't be too painful to reorganize when the time comes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
GOOD DAY,
My name is Mr Yao Yuta from Hong Kong, I want you to be my partner in a business project. If Interested Contact me back via my email address. Thank you, Mr. Yao Yuta. -- To unsubscribe from this list: send the line unsubscribe 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 8/9] fetch doc: add a section on configured remote-tracking branches
Marc Branchaud mbranch...@xiplink.com writes: [jc: omitted good suggestions I'll use in amending] + the refspecs to be used to fetch. The example above will fetch /to be used// I have a problem with that change, actually, because you do not fetch refspec from anywhere. A refspec is what is used to determine what histories to fetch (i.e. left-hand side of it before the colon) and which local refs to update with what is fetched (i.e. right-hand side of it after the colon), and this description of the traditional behaviour is meant to highlight the difference from the second usage, which is relatively new since f2690487 (fetch: opportunistically update tracking refs, 2013-05-11), i.e. how the variable is *not* used as a refspec when the command line already has one. Perhaps ... `remote.repository.fetch` values are used as the refspecs, i.e. they specify what refs to fetch and what local refs to update. or something? -- To unsubscribe from this list: send the line unsubscribe 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 9/9] fetch: allow explicit --refmap to override configuration
Marc Branchaud mbranch...@xiplink.com writes: Teach the command to pay attention to the --refmap=lhs:rhs command-line options that can be used to override the use of configured remote.*.fetch as the refmap. (Your 0/9 message merely said The new patches at the end clarifies how remote.*.fetch configuration variables are used in two conceptually different ways. so I was not expecting fetch to get a new option.) This is more about conceptual consistency completeness than new and useful addition, in that configured values and the feature they enable ought to be expressible and overridable from the command line options but we so far lacked a way to trigger the do not affect what gets fetched, only affect where they go locally feature, which is offered by the second way to use remote.*.fetch variable. I do not think we absolutely need it and that is why it is at the end as an optional addition. [jc: again, good suggestions I'll use when amending snipped] 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: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
On Wed, Jun 4, 2014 at 11:16 PM, Stepan Kasal ka...@ucw.cz wrote: Hi dscho, your arguments seem really strong. (Especially the four years of battle testing, with the memories of constant problems with HOME before.) I hope they are strong enough to convince Junio to accept this patch; that would help. I think you should include the problems Dscho described in the commit message too. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
GOOD DAY,
My name is Mr Yao Yuta from Hong Kong, I want you to be my partner in a business project. If Interested Contact me back via my email address. Thank you, Mr. Yao Yuta. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH] Add a Windows-specific fallback to getenv(HOME);
Am 04.06.2014 17:46, schrieb Johannes Schindelin: Hi kusma, On Wed, 4 Jun 2014, Johannes Schindelin wrote: The problem arises whenever git.exe calls subprocesses. You can pollute the environment by setting HOME, I do not recall the details, but I remember that we had to be very careful *not* to do that, hence the patch. Sorry, has been a long time. Actually, a quick search in my Applegate vaults^W^Wmail archives suggests that we had tons of troubles with non-ASCII characters in the path. After a bit of digging in the history and the old googlegroups issue tracker, I think this patch is completely unrelated to the non-ASCII problems. In summary, this patch fixes 'git config' for the portable version only, and it only does so partially. Thus I don't think its ready for upstream, at least not in its current form. See below for the nasty details. I would be strongly in favor of fixing the problem by the root: avoiding to have Git rely on the HOME environment variable to be set, but instead add a clean API call that even says what it is supposed to do: gimme the user's home directory's path. And that is exactly what the patch does. By that argument we'd have to introduce API abstractions for every environment variable that could possibly resemble a path (PATH, TMPDIR, GIT_DIR, GIT_WORK_DIR, GIT_TRACE* etc.). We already have similar fallback logic for TMPDIR that is completely non-intrusive to core git code (fully encapsulated in mingw.c, see mingw_getenv (upstream) or mingw_startup (msysgit)). IMO such a solution would be hugely preferable over adding an additional get_home_directory() API (and continuously checking that no new upstream code accidentally introduces another 'getenv(HOME)'). Cheers, Karsten Analysis of $HOME-realted issues: 1. mangled non-ASCII characters in environment variables E.g. issue 491 [1], reportedly fixed in v1.7.10 ([1] comment #10). This is actually a bug in msys.dll, and there's nothing that can be done about it from within git.exe. It is also not a problem if git is launched from cmd.exe. The root cause is that the msys environment is initialized using GetEnvironmentStringsA(), which returns GetOEMCP()-encoded strings (e.g. cp850), rather than GetACP() (e.g. cp1252) as all other *A API functions do [2]. This adds one level of mangling whenever a native Windows program starts an msys program (so e.g. the call chain bash-git-bash-wish would mangle twice, see [1] comment #3). For the fixed GetEnvironmentStringsA(), see [3] lines 459ff. (As a side note, $HOMEDRIVE and $HOMEPATH originally did not have this problem, as they were separately initialized from NetUserGetInfoA(). This was changed in v1.6.3, however, at that time etc/profile was still using the broken $USERPROFILE. See [4], [5].) 2. 'git config' doesn't work with disconnected network drives Issues 259 [6], 497 [7] and 512 [8], fixed in v1.7.0.2 for bash and v1.7.2.3 for cmd. Apparently, $HOMEDRIVE$HOMEPATH is the home directory on the network, and $USERPROFILE is local. To be able to work offline, we need to check if $HOMEDRIVE$HOMEPATH exists and fall back to $USERPROFILE otherwise. Note that git-wrapper does _not_ check if $HOMEDRIVE$HOMEPATH actually exists, as the original git.cmd did. This is probably a regression wrt issue 259. 3. HOME is not set when using the portable version Issue 482 [9], partially fixed in v1.7.2.3 by this patch. 'Partially' because: - there's no fallback to $USERPROFILE, so it doesn't work with disconnected network drives (see problem 2.) - it doesn't setenv(HOME) for child processes (at least git-gui accesses $env(HOME) directly, but I haven't checked what happens if HOME is not set) Incidentally, this patch was first released with v1.7.2.3, which also sets $HOME correctly in both etc/profile and git.cmd. So I suspect that this patch has always been essentially dead code (except perhaps for the portable version, I've never used that). [1] https://code.google.com/p/msysgit/issues/detail?id=491 [2] http://msdn.microsoft.com/en-us/library/windows/desktop/ms683187%28v=vs.85%29.aspx [3] https://github.com/msysgit/msysgit/blob/msys/src/rt/patches/0013-msys.dll-basic-Unicode-support.patch [4] https://github.com/msysgit/msysgit/blob/msys/src/rt/patches/0007-only-override-the-variables-HOMEPATH-and-HOMEDRIVE-i.patch [5] https://github.com/msysgit/msysgit/commit/6b096c9 [6] https://code.google.com/p/msysgit/issues/detail?id=259 [7] https://code.google.com/p/msysgit/issues/detail?id=497 [8] https://code.google.com/p/msysgit/issues/detail?id=512 [9] https://code.google.com/p/msysgit/issues/detail?id=482 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] Re: [PATCH v2] t5000, t5003: do not use test_cmp to compare binary files
I have the problem that the overridden test_cmp crashes on a couple of places where it is doing a binary compare, so this is definitely needed. I actually used cmp -q in my override as it's the return code that is most important. //. On Wed, 4 Jun 2014 11:22:56 AM Junio C Hamano wrote: Stepan Kasal ka...@ucw.cz writes: test_cmp() is primarily meant to compare text files (and display the difference for debug purposes). Raw cmp is better suited to compare binary files (tar, zip, etc.). On MinGW, test_cmp is a shell function mingw_test_cmp that tries to read both files into environment, stripping CR characters (introduced in commit 4d715ac0). This function usually speeds things up, as fork is extremly slow on Windows. But no wonder that this function is extremely slow and sometimes even crashes when comparing large tar or zip files. Signed-off-by: Stepan Kasal ka...@ucw.cz --- Hi Thomas, On Wed, Jun 04, 2014 at 02:59:44PM +0200, Thomas Braun wrote: Using test_cmp_bin instead of cmp would result in then four assertions for comparing arbitrary data test_cmp test_i18ncmp test_cmp_text test_cmp_bin where I think the purpose of each function is clear from its name. [test_cmp_text does not exist (yet)] OK, I agree, hence this modified version of the patch. Yeah, I think the above reasoning is sound. And I do not think we ever need to have test_cmp_text -- our payload and our messages compared by tests to make sure our expectations hold are text by default. 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
Git autocorrect bug
$ cd [some existing git repo] $ git git foo WARNING: You called a Git command named 'git', which does not exist. Continuing under the assumption that you meant 'init' in 0.1 seconds automatically... fatal: internal error: work tree has already been set Current worktree: /home/dturner/git New worktree: /home/dturner/git/foo (I am extremely unlikely to fix this bug myself, since it only arises in very rare circumstances). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
GOOD DAY,
My name is Mr Yao Yuta from Hong Kong, I want you to be my partner in a business project. If Interested Contact me back via my email address. Thank you, Mr. Yao Yuta. -- To unsubscribe from this list: send the line 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] t/t7810-grep.sh: remove duplicate test_config()
t/t7810-grep.sh had its own test_config() function which served the same purpose as the one in t/test-lib-functions.sh. Removed, all tests pass. Signed-off-by: Jeremiah Mahler jmmah...@gmail.com --- t/t7810-grep.sh | 5 - 1 file changed, 5 deletions(-) diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index 63b3039..40615de 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -1096,11 +1096,6 @@ test_expect_success 'grep -E pattern with grep.patternType=fixed' ' test_cmp expected actual ' -test_config() { - git config $1 $2 - test_when_finished git config --unset $1 -} - cat expected EOF hello.cRED:RESETint main(int argc, const char **argv) hello.cRED-RESET{ -- 2.0.0.153.g79d -- To unsubscribe from this list: send the line 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 v3 2/4] replace: add test for --graft
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t6050-replace.sh | 12 1 file changed, 12 insertions(+) diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh index 68b3cb2..ca45a84 100755 --- a/t/t6050-replace.sh +++ b/t/t6050-replace.sh @@ -351,4 +351,16 @@ test_expect_success 'replace ref cleanup' ' test -z $(git replace) ' +test_expect_success '--graft with and without already replaced object' ' + test $(git log --oneline | wc -l) = 7 + git replace --graft $HASH5 + test $(git log --oneline | wc -l) = 3 + git cat-file -p $HASH5 | test_must_fail grep parent + test_must_fail git replace --graft $HASH5 $HASH4 $HASH3 + git replace --force -g $HASH5 $HASH4 $HASH3 + git cat-file -p $HASH5 | grep parent $HASH4 + git cat-file -p $HASH5 | grep parent $HASH3 + git replace -d $HASH5 +' + test_done -- 2.0.0.rc0.40.gd30ccc4 -- To unsubscribe from this list: send the line 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 v3 3/4] Documentation: replace: add --graft option
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 8 1 file changed, 8 insertions(+) diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 61461b9..491875e 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git replace' [-f] object replacement 'git replace' [-f] --edit object +'git replace' [-f] --graft commit [parent...] 'git replace' -d object... 'git replace' [--format=format] [-l [pattern]] @@ -73,6 +74,13 @@ OPTIONS newly created object. See linkgit:git-var[1] for details about how the editor will be chosen. +--graft commit [parent...]:: + Create a graft commit. A new commit is created with the same + content as commit except that its parents will be + [parent...] instead of commit's parents. A replacement ref + is then created to replace commit with the newly created + commit. + -l pattern:: --list pattern:: List replace refs for objects that match the given pattern (or -- 2.0.0.rc0.40.gd30ccc4 -- To unsubscribe from this list: send the line 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 v3 1/4] replace: add --graft option
The usage string for this option is: git replace [-f] --graft commit [parent...] First we create a new commit that is the same as commit except that its parents are [parents...] Then we create a replace ref that replace commit with the commit we just created. With this new option, it should be straightforward to convert grafts to replace refs. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- builtin/replace.c | 84 ++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/builtin/replace.c b/builtin/replace.c index 1bb491d..9d1e82f 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -17,6 +17,7 @@ static const char * const git_replace_usage[] = { N_(git replace [-f] object replacement), N_(git replace [-f] --edit object), + N_(git replace [-f] --graft commit [parent...]), N_(git replace -d object...), N_(git replace [--format=format] [-l [pattern]]), NULL @@ -294,6 +295,76 @@ static int edit_and_replace(const char *object_ref, int force) return replace_object_sha1(object_ref, old, replacement, new, force); } +static int read_sha1_commit(const unsigned char *sha1, struct strbuf *dst) +{ + void *buf; + enum object_type type; + unsigned long size; + + buf = read_sha1_file(sha1, type, size); + if (!buf) + return error(_(cannot read object %s), sha1_to_hex(sha1)); + if (type != OBJ_COMMIT) { + free(buf); + return error(_(object %s is not a commit), sha1_to_hex(sha1)); + } + strbuf_attach(dst, buf, size, size + 1); + return 0; +} + +static int create_graft(int argc, const char **argv, int force) +{ + unsigned char old[20], new[20]; + const char *old_ref = argv[0]; + struct commit *commit; + struct strbuf buf = STRBUF_INIT; + struct strbuf new_parents = STRBUF_INIT; + const char *parent_start, *parent_end; + int i; + + if (get_sha1(old_ref, old) 0) + die(_(Not a valid object name: '%s'), old_ref); + commit = lookup_commit_or_die(old, old_ref); + if (read_sha1_commit(old, buf)) + die(_(Invalid commit: '%s'), old_ref); + + /* make sure the commit is not corrupt */ + if (parse_commit_buffer(commit, buf.buf, buf.len)) + die(_(Could not parse commit: '%s'), old_ref); + + /* find existing parents */ + parent_start = buf.buf; + parent_start += 46; /* tree + hex sha1 + \n */ + parent_end = parent_start; + + while (starts_with(parent_end, parent )) + parent_end += 48; /* parent + hex sha1 + \n */ + + /* prepare new parents */ + for (i = 1; i argc; i++) { + unsigned char sha1[20]; + if (get_sha1(argv[i], sha1) 0) + die(_(Not a valid object name: '%s'), argv[i]); + lookup_commit_or_die(sha1, argv[i]); + strbuf_addf(new_parents, parent %s\n, sha1_to_hex(sha1)); + } + + /* replace existing parents with new ones */ + strbuf_splice(buf, parent_start - buf.buf, parent_end - parent_start, + new_parents.buf, new_parents.len); + + if (write_sha1_file(buf.buf, buf.len, commit_type, new)) + die(_(could not write replacement commit for: '%s'), old_ref); + + strbuf_release(new_parents); + strbuf_release(buf); + + if (!hashcmp(old, new)) + return error(new commit is the same as the old one: '%s', sha1_to_hex(old)); + + return replace_object_sha1(old_ref, old, replacement, new, force); +} + int cmd_replace(int argc, const char **argv, const char *prefix) { int force = 0; @@ -303,12 +374,14 @@ int cmd_replace(int argc, const char **argv, const char *prefix) MODE_LIST, MODE_DELETE, MODE_EDIT, + MODE_GRAFT, MODE_REPLACE } cmdmode = MODE_UNSPECIFIED; struct option options[] = { OPT_CMDMODE('l', list, cmdmode, N_(list replace refs), MODE_LIST), OPT_CMDMODE('d', delete, cmdmode, N_(delete replace refs), MODE_DELETE), OPT_CMDMODE('e', edit, cmdmode, N_(edit existing object), MODE_EDIT), + OPT_CMDMODE('g', graft, cmdmode, N_(change a commit's parents), MODE_GRAFT), OPT_BOOL('f', force, force, N_(replace the ref if it exists)), OPT_STRING(0, format, format, N_(format), N_(use this format)), OPT_END() @@ -325,7 +398,10 @@ int cmd_replace(int argc, const char **argv, const char *prefix) usage_msg_opt(--format cannot be used when not listing, git_replace_usage, options); - if (force cmdmode != MODE_REPLACE cmdmode != MODE_EDIT) + if (force + cmdmode != MODE_REPLACE +
[PATCH v3 4/4] contrib: add convert-grafts-to-replace-refs.sh
This patch adds into contrib/ an example script to convert grafts from an existing grafts file into replace refs using the new --graft option of git replace. While at it let's mention this new script in the git replace documentation for the --graft option. Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- Documentation/git-replace.txt | 4 +++- contrib/convert-grafts-to-replace-refs.sh | 29 + 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt index 491875e..e1be2e6 100644 --- a/Documentation/git-replace.txt +++ b/Documentation/git-replace.txt @@ -79,7 +79,9 @@ OPTIONS content as commit except that its parents will be [parent...] instead of commit's parents. A replacement ref is then created to replace commit with the newly created - commit. + commit. See contrib/convert-grafts-to-replace-refs.sh for an + example script based on this option that can convert grafts to + replace refs. -l pattern:: --list pattern:: diff --git a/contrib/convert-grafts-to-replace-refs.sh b/contrib/convert-grafts-to-replace-refs.sh new file mode 100755 index 000..8472879 --- /dev/null +++ b/contrib/convert-grafts-to-replace-refs.sh @@ -0,0 +1,29 @@ +#!/bin/sh + +# You should execute this script in the repository where you +# want to convert grafts to replace refs. + +die () { + echo 2 $@ + exit 1 +} + +GRAFTS_FILE=${GIT_DIR:-.git}/info/grafts + +test -f $GRAFTS_FILE || die Could not find graft file: '$GRAFTS_FILE' + +grep '^[^# ]' $GRAFTS_FILE | while read definition +do + test -n $definition { + echo Converting: $definition + git replace --graft $definition || + die Conversion failed for: $definition + } +done + +mv $GRAFTS_FILE $GRAFTS_FILE.bak || + die Could not rename '$GRAFTS_FILE' to '$GRAFTS_FILE.bak' + +echo Success! +echo All the grafts in '$GRAFTS_FILE' have been converted to replace refs! +echo The grafts file '$GRAFTS_FILE' has been renamed: '$GRAFTS_FILE.bak' -- 2.0.0.rc0.40.gd30ccc4 -- To unsubscribe from this list: send the line 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 v3 0/4] Add --graft option to git replace
Here is a small patch series to implement: git replace [-f] --graft commit [parent...] This patch series goes on top of the patch series that implements --edit. There is only one change since v2 thanks to Eric: - improve error messages in convert-grafts-to-replace-refs.sh (patch 4/4) Christian Couder (4): replace: add --graft option replace: add test for --graft Documentation: replace: add --graft option contrib: add convert-grafts-to-replace-refs.sh Documentation/git-replace.txt | 10 builtin/replace.c | 84 ++- contrib/convert-grafts-to-replace-refs.sh | 29 +++ t/t6050-replace.sh| 12 + 4 files changed, 134 insertions(+), 1 deletion(-) create mode 100755 contrib/convert-grafts-to-replace-refs.sh -- 2.0.0.rc0.40.gd30ccc4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html