Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Wed, Sep 11, 2013 at 9:16 PM, Jeff King p...@peff.net wrote: I would prefer the static wrapper solution you suggest, though. It leaves the compiler free to optimize the common case of normal strcasecmp calls, and only introduces an extra function indirection when using it as a callback (and even then, if we can inline the strcasecmp, it still ends up as a single function call). The downside is that it has to be remembered at each site that uses strcasecmp, but we do not use pointers to standard library functions very often. Is it possible to add a test which fails if wrapper is not used? -- Piotr Krukowiecki -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH jk/config-int-range-check] compat/mingw.h: define PRId64
From: Johannes Sixt j...@kdbg.org Provide PRId64 alongside PRIuMAX. Signed-off-by: Johannes Sixt j...@kdbg.org --- I thought I had compiled 'next' on Windows recently... This is an emergency fix for a compile error in 'master'. compat/mingw.h | 1 + 1 file changed, 1 insertion(+) diff --git a/compat/mingw.h b/compat/mingw.h index bd0a88b..9eb3b17 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -322,6 +322,7 @@ static inline char *mingw_find_last_dir_sep(const char *path) #define find_last_dir_sep mingw_find_last_dir_sep #define PATH_SEP ';' #define PRIuMAX I64u +#define PRId64 I64d void mingw_open_html(const char *path); #define open_html mingw_open_html -- 1.8.4.1573.g0cbe1bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] git checkout $commit -- somedir doesn't drop files
On Tue, Sep 17, 2013 at 03:14:39PM -0700, Junio C Hamano wrote: Yeah, then I agree that git checkout HEAD^ -- subdir should be a one-way go HEAD^ merge limited only to the paths that match subdir/. If implemented in a straight-forward way, I suspect that we may end up not removing subdir/b in Uwe's sample transcript. I am not sure if that is a good thing or not, though. If the index originally tracked and then going to tree does not, I think removing it would match ignore local modifications rule, as subdir/a that is tracked in the index and also in going to tree does get overwritten to match the state recorded in the tree. I had assumed the goal was to subdir/b (by the reasoning above, and the rm -rf tar analogy you gave earlier). I tried a very hacky attempt at shoving unpack-trees into the right spot in checkout_paths. But its pathspec handling from unpack_trees is not quite what we want. In Uwe's case, it did delete subdir/b and overwrite subdir/a, which I'd expect. But if there was an additional file outside of subdir, it got deleted, too. The problem is that I was giving the regular index to unpack_trees as the destination; so it wrote out only the bits of the index under the pathspec subdir/, and omitted the rest entirely. I had hoped instead it would leave those parts untouched from the source. An alternative would be to write the new entries to a temporary index in memory. And then you can throw away the entries in the current index that match the pathspec, and add in the entries from the temporary index. I was just hoping that unpack-trees would do all of that for me. :) At this point, I'm giving up for now. I was hoping a naive application of unpack-trees would just work (and reduce the code size to boot), but it is obviously a bit more complicated than that. I still think it's a good idea for checkout to behave as Uwe described, but I don't think it's that high a priority, and I have other stuff I'd prefer to work on at the moment. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: git clone silently aborts if stdout gets a broken pipe
-Original Message-
From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On
Behalf Of Jeff King
Sent: den 18 september 2013 20:46
To: Peter Kjellerstedt
Cc: Junio C Hamano; Nguyen Thai Ngoc Duy; git@vger.kernel.org
Subject: Re: git clone silently aborts if stdout gets a broken pipe
On Wed, Sep 18, 2013 at 06:52:13PM +0200, Peter Kjellerstedt wrote:
The failing Perl code used a construct like this:
Git::command_oneline('clone', $url, $path);
There is no error raised, but the directory specified by
$path is not created. If I look at the process using strace
I can see the clone taking place, but then it seems to get
a broken pipe since the code above only cares about the
first line from stdout (and with the addition of Checking
connectivity... git clone now outputs two lines to stdout).
I think your perl script is somewhat questionable, as it is making
assumptions about the output of git-clone, and you would do better to
accept arbitrary-sized output
Well, the whole idea of using Git::command_oneline() is that we
are only interested in the first line of output, similar to using
| head -1. If we had wanted all of the output we would have used
Git::command() instead. Since the Git Perl module is released as a
part of Git, I would expect it to work as documented regardless of
which Git command is used with Git::command_oneline().
In the case of git clone the output to stdout is pretty small so
retrieving all of it would of course not be much overhead, but for
some other commands retrieving all output when only the first line
is wanted (or maybe not even that one) seems unnecessary.
However, what surprised me most was that git clone failed silently
when it got a broken pipe. I cannot really see the reason for
aborting due to stdout getting a broken pipe in the first place.
But if it is, I would at least have expected an error which our
script would have caught and aborted with an appropriate error
message. Now it instead failed later when it actually tried to
access the files in the repository it thought it had cloned...
(or better yet, leave stdout pointing to
the user, so they can see the output, which is meant for them).
Well, in this specific case it is a script being run as a cron job
so anything sent to stdout would cause an unnecessary mail.
That being said, the new messages should almost certainly go to stderr.
I can but agree.
-- 8 --
Subject: [PATCH] clone: write checking connectivity to stderr
In commit 0781aa4 (clone: let the user know when
check_everything_connected is run, 2013-05-03), we started
giving the user a progress report during clone. However,
since the actual work happens in a sub-process, we do not
use the usual progress code that counts the objects, but
rather just print a message ourselves.
This message goes to stdout via printf, which is unlike
other progress messages (both the eye candy within clone,
and the checking connectivity progress in other commands).
Let's send it to stderr for consistency.
Signed-off-by: Jeff King p...@peff.net
---
builtin/clone.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index ca3eb68..3c91844 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -551,12 +551,12 @@ static void update_remote_refs(const struct ref *refs,
if (check_connectivity) {
if (0 = option_verbosity)
- printf(_(Checking connectivity... ));
+ fprintf(stderr, _(Checking connectivity... ));
if (check_everything_connected_with_transport(iterate_ref_map,
0, rm,
transport))
die(_(remote did not send all necessary objects));
if (0 = option_verbosity)
- printf(_(done\n));
+ fprintf(stderr, _(done\n));
}
if (refs) {
--
1.8.4.rc4.16.g228394f
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for taking the time to provide a solution to the problem.
//Peter
Re: [PATCH jk/config-int-range-check] compat/mingw.h: define PRId64
On Thu, Sep 19, 2013 at 09:17:07AM +0200, Johannes Sixt wrote: From: Johannes Sixt j...@kdbg.org Provide PRId64 alongside PRIuMAX. Signed-off-by: Johannes Sixt j...@kdbg.org Thanks. I had noticed this was the first use of PRId64, but I wasn't sure what various implementations would want. I notice that we also have a fallback PRIuMAX of llu in git-compat-util.h, but I'm not sure which platforms need that, nor what they would want for PRId64. By the explanation in 3efb1f3 and e326bce, it looks like the strategy was to just use a long long and hope for the best. So we might want: diff --git a/git-compat-util.h b/git-compat-util.h index 9549de6..4438e7c 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -239,6 +239,10 @@ extern char *gitbasename(char *); #define PRIuMAX llu #endif +#ifndef PRId64 +#define PRId64 lld +#endif + #ifndef PRIu32 #define PRIu32 u #endif as well, but I think I'd rather wait until somebody with an actual system that needs it reports in (and tells us what the right value for their system is) as opposed to just guessing. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git clone silently aborts if stdout gets a broken pipe
On Thu, Sep 19, 2013 at 09:54:38AM +0200, Peter Kjellerstedt wrote: I think your perl script is somewhat questionable, as it is making assumptions about the output of git-clone, and you would do better to accept arbitrary-sized output Well, the whole idea of using Git::command_oneline() is that we are only interested in the first line of output, similar to using | head -1. If we had wanted all of the output we would have used Git::command() instead. Since the Git Perl module is released as a part of Git, I would expect it to work as documented regardless of which Git command is used with Git::command_oneline(). I think command_oneline is exactly like | head -1 in this case. Doing git clone | head -1 would also fail, and should not be used. In general, you do not want to put a limiting pipe on a command with side effects beyond output. The design of unix pipes and SIGPIPE is such that you can do generate_output | head, and generate_output will get SIGPIPE and die after realizing that its writer no longer cares about the output. But if your command is doing something besides output, that assumption doesn't hold. Arguably, git clone should be taking the initiative to ignore SIGPIPE itself. Its primary function is not output, but doing the clone. If output fails, we would want to continue the clone, not die. By the way, did you actually want to capture the stdout of git-clone, or were you just trying to suppress it? Because the eventual patch I posted sends it to stderr, under the assumption that what used to go to stdout should not be captured and parsed (because it is localized and subject to change). However, what surprised me most was that git clone failed silently when it got a broken pipe. It's not git clone that is doing this, I think, but rather the design of command_oneline. If I do: (sleep 1; git clone ...; echo 2 exit=$?) | false then I see: exit=141 That is, clone dies from SIGPIPE trying to write Cloning into But command_oneline is specifically designed to ignore SIGPIPE death, because you would want something like: command_oneline(git, rev-list, $A..$B); to give you the first line, and then you do not care if the rest of the rev-list dies due to SIGPIPE (it is a good thing, because by closing the pipe you are telling it that its output is not needed). It may be that the documentation for command_oneline can be improved to mention this subtlety. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On the behavior of checkout branch with uncommitted local changes
Dear all I'm not a power git user but I profit of git every day and I like to fully understand what I do. The man section for git checkout is too vague for my taste. In particular it is not clearly (unambiguously) stated what happens to index and worktree whenever local uncommitted changes are around. I've already rised a similar problem in this mail list [1], but I understand that a man page must be concise. On the other hand, I couldn't find any complete information on this behavior: tutorials and books seem to avoid the problem, user posts seems confused ... To grasp some more information, I've spent some hours in trials (sorry I'm unable to grasp information browsing the code repository). That resulted in the algorithm below presented. Could anybody authoritative on that subject confirm/correct/discharge my statement? That could be of help for me and may others. Nonetheless to say having this kind of pseudocodes available somewhere (e.g. for stash [2] and other tools modifing index and working tree) would make my git experience (and that of many more people) happier. Thanks to all developers for their efforts. Regards ric Notations: let us fix a file and denote C0 = its version in the initial commit I0 = its version in the initial index W0 = its version in the working tree C1 = its version in the target commit W1= its version in working tree after checkout completed I1 = its version in index after checkout completed git checkout Branch if C0=W0=I0, then: W1=I1=C1; if C1=I0, then: W1=W0 and I1=C1=I0; if C1=C0,then: W1=W0 and I1=I0; otherwise: abort Note: in particular, if W0=I0 !=C0 then (in general) abort Note: in particular, if C0=I0 and C1=W0 then abort (...actually why that? no information is lost) REFS [1]http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/782914/focus=164647 [2]http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=717088 -- To unsubscribe from this list: send the line unsubscribe 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]: sparse checkout file with windows line endings doesn't work
Hi Duy, And it does work for me with CRLF endings (again tested on Linux). Can you send me your sparse-checkout file? Zip it if needed. sparse-checkout created with echo /CONFIGURATION .git\info\sparse-checkout on Windows. Attached file created with tar cvzf sparsecheckout.tar.gz .git/info/sparse-checkout in gitbash shell on Windows. Regards, Martin -- Martin Gregory Senior Consultant, Adelaide Interim P: +61 8 7200 5350 M: +61 402 410 971 F: +61 8 7200 3725 sparsecheckout.tar.gz Description: Binary data
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 19, 2013 at 9:37 AM, Junio C Hamano ju...@pobox.com wrote: On Sep 18, 2013 11:08 PM, Piotr Krukowiecki piotr.krukowie...@gmail.com wrote: On Wed, Sep 11, 2013 at 9:16 PM, Jeff King p...@peff.net wrote: I would prefer the static wrapper solution you suggest, though. It [...] it still ends up as a single function call). The downside is that it has to be remembered at each site that uses strcasecmp, but we do not use pointers to standard library functions very often. Is it possible to add a test which fails if wrapper is not used? No test needed for this, as compilation or linkage will fail, I think. But only when someone compiles on MinGW, no? -- Piotr Krukowiecki -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Sep 2013, #05; Wed, 18)
Junio C Hamano gits...@pobox.com writes: * mm/rebase-continue-freebsd-WB (2013-09-09) 1 commit (merged to 'next' on 2013-09-13 at 82e8b91) + rebase: fix run_specific_rebase's use of return on FreeBSD Work around a bug in FreeBSD shell that caused a regression to git rebase in v1.8.4. It would be lovely to hear from FreeBSD folks a success report We just did: http://thread.gmane.org/gmane.comp.version-control.git/234825/focus=234870 Will merge to 'master' in the fifth batch. Don't forget maint, too. Thanks, -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
Junio C Hamano gits...@pobox.com writes: Thomas Gummerer t.gumme...@gmail.com writes: +For performance tests that need cleaning up after them that should not +be timed, use + +test_perf_cleanup 'descriptive string' ' +command1 +command2 +' ' +cleanupcommand1 +cleanupcommand2 +' + Hmm, if not timing the clean-up actions is the primary goal, is it an option to reuse test_when_finished for this (you may have to make sure that the commands run inside it are not timed; I didn't check). Maybe I wasn't clear enough. The goal is to allow tests to have everything cleaned up after every run. This can then be used for commands that change the index (or other things) and get back to the original state after that. For example something like this: file=$(git ls-files | tail -n 30 | head -1) test_expect_success change a file echo 'something' $file test_perf_cleanup v5 update-index file git update-index $file git reset test_when_finished on the other hand only cleans up when the whole test is finished. One thing I felt uneasy about the above construct is that it makes cleanupcommand2 responsible for handling cases where not just command2 but also command1 might have failed. Compared to that, test-when-finished allows you to control what clean-up to do depending on what have already been done, i.e. test_when_finished 'undo what command1 would have done' command1 test_when_finished 'undo what command2 would have done' command2 ... The second undo command2 must be prepared for the case where command2 may have failed in the middle, but it can at least rely on command1 having succeeded when it is run. When one performance test fails, the testing is aborted and the cleanup commands are not executed anymore, leaving the trash directory in the failed state. This consistent with the normal tests with the immediate flag passed to them. (All performance tests have the --immediate flag on implicitly). -- To unsubscribe from this list: send the line 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 v4] Teach git to change to a given directory using -C option
Hi, Sorry it took me a while to get back on this. Reroll at the bottom ... On Tue, Sep 10, 2013 at 12:32 AM, Junio C Hamano gits...@pobox.com wrote: Nazri Ramliy ayieh...@gmail.com writes: Subject: git: run in a directory given with -C option This is similar in spirit to to make -C dir ... and tar -C dir The doubled-to to which I locally fixed when I queued the last one (together with other rewording to make it more agreeable and easier to read) somehow came back ;-) Will fix locally again. I must have mistakenly made the revision on top of my local changes instead of the one in next - I didn't notice the fix. Thanks for the fix. +-C path:: + Run as if git was started in 'path' instead of the current working + directory. When multiple '-C' options are given, each subsequent I think this should be `-C` to typeset it as typed literally. + non-absolute `-C path` is interpreted relative to the preceding `-C + path`. ++ +This option affects options that expect path name like '--git-dir' and +'--work-tree' in that their interpretations of the path names would be Likewise for `--git-dir` and `--work-tree`. +made relative to the working directory caused by the '-C' option. For and here. Now I'm noticing that you've already made the above fixes in next ;) diff --git a/t/t0056-git-C.sh b/t/t0056-git-C.sh new file mode 100755 index 000..c0006da --- /dev/null +++ b/t/t0056-git-C.sh @@ -0,0 +1,82 @@ +#!/bin/sh + +test_description='-C path option and its effects on other path-related options' + +. ./test-lib.sh + +test_expect_success 'git -C path runs git from the directory path' ' + test_create_repo dir1 + echo 1 dir1/a.txt + (cd dir1 git add a.txt git commit -m initial in dir1) Curious why this does not use -C here. It didn't use -C there because it's in the prepare the expected test output stage and we want that to succeed, whether -C works or not - we haven't reached the part where we are actually testing the -C option, which is right after that: + echo initial in dir1 expected + git -C dir1 log --format=%s actual + test_cmp expected actual +' + +test_expect_success 'Multiple -C options: -C dir1 -C dir2 is equivalent to -C dir1/dir2' ' + test_create_repo dir1/dir2 + echo 1 dir1/dir2/a.txt + git -C dir1/dir2 add a.txt Because a.txt exists in both dir1 and dir1/dir2, this has less chance of catching a bug (if somebody breaks the feature to run it in dir1 not dir1/dir2, add will happily say Oh, I found something to add, instead of saying Huh? there is no such path. If you used b.txt instead, you would catch such a breakage. Remember, tests are not about demonstrating how cool the new feature is and/or how well it works in an expected setting. Imagine ways other people can break your spiffy new feature in later patches, and design tests that are more likely to catch them. The same comment applies throughout the remainder of this script. Noted, but I my imagination is limited at the moment so I haven't come up with new tests to that effect yet ;) + echo initial in dir1/dir2 expected + git -C dir1/dir2 commit -m initial in dir1/dir2 to reduce possibilities of breaking this test in the future due to typos (e.g. somebody may want to say initial commit in dir1/dir2), doing this may be a better idea: msg=initial in dir1/dir2 echo $msg expected git -C dir1/dir2 commit -m $msg The same comment applies to the previous one. Fixed. nazri -- 8 -- Subject: git: run in a directory given with -C option This is similar in spirit to make -C dir ... and tar -C dir It takes more keypresses to invoke git command in a different directory without leaving the current directory: 1. (cd ~/foo git status) git --git-dir=~/foo/.git --work-dir=~/foo status GIT_DIR=~/foo/.git GIT_WORK_TREE=~/foo git status 2. (cd ../..; git grep foo) 3. for d in d1 d2 d3; do (cd $d git svn rebase); done The methods shown above are acceptable for scripting but are too cumbersome for quick command line invocations. With this new option, the above can be done with fewer keystrokes: 1. git -C ~/foo status 2. git -C ../.. grep foo 3. for d in d1 d2 d3; do git -C $d svn rebase; done A new test script is added to verify the behavior of this option with other path-related options like --git-dir and --work-tree. Signed-off-by: Nazri Ramliy ayieh...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git.txt | 16 +- git.c | 13 +++- t/t0056-git-C.sh | 84 +++ 3 files changed, 111 insertions(+), 2 deletions(-) create mode 100755 t/t0056-git-C.sh diff --git a/Documentation/git.txt b/Documentation/git.txt index c4f0ed5..5d68d33 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -9,7 +9,7 @@ git - the
Re: Bisect needing to be at repo top-level?
On 18/9/13 05:20, Junio C Hamano wrote: Lukas Fleischer g...@cryptocrack.de writes: why do we allow running git-checkout(1) from a subdirectory? We may want to check the condition and forbid such a checkout. It would probably make sense. It might also make sense to relax the check in git bisect somewhat. Currently, even git bisect help insists that You need to run this command from the toplevel of the working tree. Regards, Ben -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git issue / [PATCH] MIPS: fix invalid symbolic link file
On Thu, Sep 19, 2013 at 06:39:08PM +0530, Madhavan Srinivasan wrote: (Git folks, please read on.) Commit 3b29aa5ba204c created a symlink file in include/dt-bindings. Even though commit diff is fine, symlink is invalid. ls -lb shows a newline character at the end of the filename. lrwxrwxrwx 1 maddy maddy 35 Sep 19 18:11 dt-bindings - ../../../../../include/dt-bindings\n Signed-off-by: Madhavan Srinivasan ma...@linux.vnet.ibm.com --- arch/mips/boot/dts/include/dt-bindings |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/mips/boot/dts/include/dt-bindings b/arch/mips/boot/dts/include/dt-bindings index 68ae388..08c00e4 12 --- a/arch/mips/boot/dts/include/dt-bindings +++ b/arch/mips/boot/dts/include/dt-bindings @@ -1 +1 @@ -../../../../../include/dt-bindings +../../../../../include/dt-bindings \ No newline at end of file -- 1.7.10.4 I applied your patch - but now git-show shows it as an empty commit and ls -lb arch/mips/boot/dts/include/dt-bindings still shows the \n at the end of the link target. Things are looking ok now that I manually fixed the link and commited the result. I hope git-push and git-pull are going to handle this correct. So, I wonder if this is a git bug. The original patch that introduced the symlink with the \n is kernel commit 3b29aa5ba204c62b3ec8f9f5b1ebd6e5d74f75d3 and is archived in patchwork at http://patchwork.linux-mips.org/patch/5745/ The patch file contains a \n at the end - but one would expect that from a patch file that has been transfered via email, so I'm not sure how this is supposed to work with emailed patches?!? Anyway, I'm not too fond of sylinks in the tree or in patches and I'm wondering if we could get rid of them for something more bullet proof. Ralf -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On 12.09.2013 23:31, Jonathan Nieder wrote: And that's exactly what defining __NO_INLINE__ does. Granted, defining __NO_INLINE__ in the scope of string.h will also add a #define strcasecmp _stricmp; but despite it's name, defining __NO_INLINE__ does not imply a performance hit due to functions not being inlined because it's just the strncasecmp wrapper around _strnicmp that's being inlined, not _strnicmp itself. What I don't understand is why the header doesn't use static inline instead of extern inline. The former would seem to be better in every way for this particular use case. See also http://www.greenend.org.uk/rjk/tech/inline.html, section GNU C inline rules. I've suggested this at [1] now to see if such a patch is likely to be accepted. [1] http://article.gmane.org/gmane.comp.gnu.mingw.user/42993 -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: git clone silently aborts if stdout gets a broken pipe
-Original Message-
From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On
Behalf Of Jeff King
Sent: den 19 september 2013 10:36
To: Peter Kjellerstedt
Cc: Junio C Hamano; Nguyen Thai Ngoc Duy; git@vger.kernel.org
Subject: Re: git clone silently aborts if stdout gets a broken pipe
On Thu, Sep 19, 2013 at 09:54:38AM +0200, Peter Kjellerstedt wrote:
I think your perl script is somewhat questionable, as it is making
assumptions about the output of git-clone, and you would do better
to
accept arbitrary-sized output
Well, the whole idea of using Git::command_oneline() is that we
are only interested in the first line of output, similar to using
| head -1. If we had wanted all of the output we would have used
Git::command() instead. Since the Git Perl module is released as a
part of Git, I would expect it to work as documented regardless of
which Git command is used with Git::command_oneline().
I think command_oneline is exactly like | head -1 in this case. Doing
git clone | head -1 would also fail, and should not be used. In
general, you do not want to put a limiting pipe on a command with side
effects beyond output. The design of unix pipes and SIGPIPE is such that
you can do generate_output | head, and generate_output will get
SIGPIPE and die after realizing that its writer no longer cares about
the output. But if your command is doing something besides output, that
assumption doesn't hold.
A very valid point.
Arguably, git clone should be taking the initiative to ignore SIGPIPE
itself. Its primary function is not output, but doing the clone. If
output fails, we would want to continue the clone, not die.
By the way, did you actually want to capture the stdout of git-clone, or
were you just trying to suppress it? Because the eventual patch I posted
sends it to stderr, under the assumption that what used to go to stdout
should not be captured and parsed (because it is localized and subject
to change).
No, we were not really interested in the output to stdout (which is
why the return value from Git::command_oneline() was ignored).
However, what surprised me most was that git clone failed silently
when it got a broken pipe.
It's not git clone that is doing this, I think, but rather the design
of command_oneline. If I do:
(sleep 1; git clone ...; echo 2 exit=$?) | false
then I see:
exit=141
That is, clone dies from SIGPIPE trying to write Cloning into But
command_oneline is specifically designed to ignore SIGPIPE death,
because you would want something like:
command_oneline(git, rev-list, $A..$B);
to give you the first line, and then you do not care if the rest of the
rev-list dies due to SIGPIPE (it is a good thing, because by closing the
pipe you are telling it that its output is not needed). It may be that
the documentation for command_oneline can be improved to mention this
subtlety.
Ok, all of it makes sense now. Thank you for the explanation.
I have corrected our script so it now works correctly with
git 1.8.4 as well.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
//Peter
N�r��yb�X��ǧv�^�){.n�+ا���ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z� �w���?��)ߢf
Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
Thomas Gummerer t.gumme...@gmail.com writes: When one performance test fails, the testing is aborted and the cleanup commands are not executed anymore, leaving the trash directory in the failed state. Ah, that I overlooked. In that case, the comments in my previous message do not apply. 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 v4] Teach git to change to a given directory using -C option
Nazri Ramliy ayieh...@gmail.com writes: Now I'm noticing that you've already made the above fixes in next ;) Yes. I said I'd locally tweak and queue, didn't I? +test_expect_success 'git -C path runs git from the directory path' ' + test_create_repo dir1 + echo 1 dir1/a.txt + (cd dir1 git add a.txt git commit -m initial in dir1) Curious why this does not use -C here. It didn't use -C there because it's in the prepare the expected test output stage and we want that to succeed, whether -C works or not - we haven't reached the part where we are actually testing the -C option, Good thinking. Will queue as an incremental update on top of what is already in 'next'. 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: On the behavior of checkout branch with uncommitted local changes
r.duc...@gmail.com writes: The man section for git checkout ... In particular it is not clearly (unambiguously) stated what happens to index and worktree whenever local uncommitted changes are around. In the current text, the key information is in two places: 'git checkout' branch:: To prepare for working on branch, switch to it by updating the index and the files in the working tree, and by pointing HEAD at the branch. Local modifications to the files in the working tree are kept, so that they can be committed to the branch. -m:: --merge:: When switching branches, if you have local modifications to one or more files that are different between the current branch and the branch to which you are switching, the command refuses to switch branches in order to preserve your modifications in context. Let's see how we can improve the text. Points to notice are: * by updating the index and the files does not say how they are updated and can be clarified: - The index is made to match the state the commit at the tip of branch records. - The working tree files without local modifications are updated the same way. - The working tree files with local modifications are not touched. * Because the command refuses to checkout another branch under this and that condition does not have its own section, the description of -m needs to say it as a background information to explain in what situation the option may be useful. It can be moved to the main 'git checkout' branch part and it may make the result read easier. * in order to preserve your modifications in context is correct and sufficient description, but it requires some thinking in readers' part to understand why it is a good thing. It can be clarified. The thinking goes like this. Suppose your current branch has file X whose contents is X0 (that is, the commit at the tip of this branch records file X with content X0). You have local changes to this file and its contents is X1. The index at path X is unchanged and still records X0. The branch you are checking out has contents X2 at the path. If we allowed git checkout the other branch and simply kept the local changes, you will end up in a funny state in which: - The tip commit, that will become the parent commit when you make the next commit, has X2 at path X. - The index has X2 at path X to match the tip commit. You could change this to keep X0 but it does not matter in the larger picture, because you will be editing the working tree version and updating the index from there to prepare for the next commit. - The working tree has contents X1 at path X. But realize that the change you made is the difference between X0 and X1, not X2 and X1. If we allowed such a checkout and then you did git commit -a, you will end up reverting the state between X0 (contents in your original branch) and X2 (contents in the new branch), even though the change you wanted to make was only the difference between X0 and X1. Also, if you did git add X and then checkout branch, unless the version in the index at path X match either your original branch or the branch you are checking out, the command will stop you, and the -m option does not resolve this with a 4-way merge (it will be too complex for users to understand if we did so). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] git checkout $commit -- somedir doesn't drop files
Jeff King p...@peff.net writes: An alternative would be to write the new entries to a temporary index in memory. And then you can throw away the entries in the current index that match the pathspec, and add in the entries from the temporary index. I was just hoping that unpack-trees would do all of that for me. :) Isn't a go there one-way merge with pathspecs very similar to what happens in reset -- pathspec except for the working tree part? I suspect that it may be sufficient to mimic the read_from_tree() and adapt update_index_from_diff() callback in builtin/reset.c to also update the working tree (and we can do so unconditionally without checking if we have any local modification in this case, which simplifies things a lot), but I may be missing 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: What's cooking in git.git (Sep 2013, #05; Wed, 18)
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: * mm/rebase-continue-freebsd-WB (2013-09-09) 1 commit (merged to 'next' on 2013-09-13 at 82e8b91) + rebase: fix run_specific_rebase's use of return on FreeBSD Work around a bug in FreeBSD shell that caused a regression to git rebase in v1.8.4. It would be lovely to hear from FreeBSD folks a success report We just did: http://thread.gmane.org/gmane.comp.version-control.git/234825/focus=234870 That only talks about more not showing colors. ... goes and looks ... Ah, there is another subthread that ends at 234833 that reports success; I am guessing from pkgsrc that this is either some variant of BSD or Solaris? -- To unsubscribe from this list: send the line unsubscribe 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] completion: improve untracked directory filtering for filename completion
SZEDER Gábor wrote: $ time __git_index_files --others --modified untracked-dir real0m0.537s user0m0.452s sys 0m0.160s Eliminate this delay by additionally passing the '--directory --no-empty-directory' options to 'git ls-files' to show only the directory name of non-empty untracked directories instead their whole content: $ time __git_index_files --others --modified --directory --no-empty-directory untracked-dir real0m0.029s user0m0.020s sys 0m0.004s Nice. This is what git status uses, too. Filename completion for 'git clean' suffers from the same delay, as it offers untracked files, too. The fix could be the same, but since it actually makes sense to 'git clean' empty directories, in this case we only pass the '--directory' option to 'git ls-files'. Also sensible. For what it's worth, Reviewed-by: Jonathan Nieder jrnie...@gmail.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 v3 1/5] pull: rename pull.rename to pull.mode
On 2013-09-09 18:49, Felipe Contreras wrote: On Mon, Sep 9, 2013 at 4:23 PM, Richard Hansen rhan...@bbn.com wrote: On 2013-09-08 21:23, Felipe Contreras wrote: The old configurations still work, but get deprecated. Should some tests for the deprecated configs be added? We wouldn't want to accidentally break those. Probably, but Junio is not picking this patch anyway. It sounds to me like he would with some mods: http://thread.gmane.org/gmane.comp.version-control.git/233554/focus=234488 -Richard -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git issue / [PATCH] MIPS: fix invalid symbolic link file
Am 19.09.2013 15:39, schrieb Ralf Baechle: The original patch that introduced the symlink with the \n is kernel commit 3b29aa5ba204c62b3ec8f9f5b1ebd6e5d74f75d3 and is archived in patchwork at http://patchwork.linux-mips.org/patch/5745/ The patch file contains a \n at the end - but one would expect that from a patch file that has been transfered via email, so I'm not sure how this is supposed to work with emailed patches?!? The mbox file I downloaded from this link looks like this: arch/mips/boot/dts/include/dt-bindings | 1 + 1 file changed, 1 insertion(+) create mode 12 arch/mips/boot/dts/include/dt-bindings \ No newline at end of file diff --git a/.../include/dt-bindings b/.../include/dt-bindings new file mode 12 index 000..08c00e4 --- /dev/null +++ b/arch/mips/boot/dts/include/dt-bindings @@ -0,0 +1 @@ +../../../../../include/dt-bindings but it should look like this: arch/mips/boot/dts/include/dt-bindings | 1 + 1 file changed, 1 insertion(+) create mode 12 arch/mips/boot/dts/include/dt-bindings diff --git a/.../include/dt-bindings b/.../include/dt-bindings new file mode 12 index 000..08c00e4 --- /dev/null +++ b/arch/mips/boot/dts/include/dt-bindings @@ -0,0 +1 @@ +../../../../../include/dt-bindings \ No newline at end of file Whoever or whatever moved the '\ No newline at end of file' line above the patch text is to blame. -- Hannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] perf-lib: add test_perf_cleanup target
Junio C Hamano gits...@pobox.com writes: I wondered why this clean-up section cannot be an optional parameter to test_perf, but that would not fly well because we won't know if 3-arg form is with one prerequisite and no clean-up, or no prereq with a clean-up, so perhaps adding a new function may be the best you could do. But in the longer term, I think we would be better off ... In any case, will queue your version as-is, at least for now. 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 2/2] perf-lib: add test_perf_cleanup target
Junio C Hamano gits...@pobox.com writes:
Thomas Gummerer t.gumme...@gmail.com writes:
When one performance test fails, the testing is aborted and the cleanup
commands are not executed anymore, leaving the trash directory in the
failed state.
Ah, that I overlooked. In that case, the comments in my previous
message do not apply.
Having said that, it was unclear to me why we would want a new
test_perf_cleanup added.
The name is misleading (does it define only clean-up actions?) to
say the least, and one way of fixing it would be to call it
test_perf_with_cleanup.
I wondered why this clean-up section cannot be an optional parameter
to test_perf, but that would not fly well because we won't know if
3-arg form is with one prerequisite and no clean-up, or no prereq
with a clean-up, so perhaps adding a new function may be the best
you could do.
But in the longer term, I think we would be better off to allow two
styles (one traditional, another modern) and add new features only
to the modern (aka more easily extensible) form:
test_perf [PREREQ] BODY
test_perf [--prereq PREREQ] [--cleanup CLEANUP] ... BODY
perhaps like this (this is without --cleanup but it should be
obvious how to add a support for it to the command line parser).
The patch to t0008 is primarily to adjust for test labels that begin
with double-dashes that will be mistaken as a new-style option, but
it is unnecessarily big because it uses random custom shell functions
to hide the real calls to underlying test_expect_success X-.
t/test-lib-functions.sh | 72 ++---
t/perf/perf-lib.sh | 17 +---
t/t0008-ignores.sh | 43 +++--
3 files changed, 87 insertions(+), 45 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a7e9aac..10202dc 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -342,20 +342,65 @@ test_declared_prereq () {
return 1
}
+test_expect_parse () {
+ whoami=$1
+ shift
+ test_expect_new_style=
+ while case $# in 0) false ;; esac
+ do
+ case $1 in
+ --prereq)
+ test $# -gt 1 ||
+ error bug in the test script: --prereq needs a
parameter
+ test_prereq=$2
+ shift
+ ;;
+ --)
+ shift
+ break
+ ;;
+ --*)
+ error bug in the test script: unknown option '$1'
+ ;;
+ *)
+ break
+ ;;
+ esac
+ test_expect_new_style=yes
+ shift
+ done
+
+ # Traditional test_expect_what [PREREQ] BODY
+ if test -z $test_expect_new_style
+ then
+ test $# = 3 { test_prereq=$1; shift; } || test_prereq=
+ fi
+
+ if test $# != 2
+ then
+ if test -z $test_expect_new_style
+ then
+ error bug in the test script: not 2 or 3 parameters to
$whoami
+ else
+ error bug in the test script: not 2 parameters to
$whoami
+ fi
+ fi
+ test_label=$1 test_body=$2
+
+ export test_prereq
+}
+
test_expect_failure () {
test_start_
- test $# = 3 { test_prereq=$1; shift; } || test_prereq=
- test $# = 2 ||
- error bug in the test script: not 2 or 3 parameters to
test-expect-failure
- export test_prereq
+ test_expect_parse test_expect_failure $@
if ! test_skip $@
then
- say 3 checking known breakage: $2
- if test_run_ $2 expecting_failure
+ say 3 checking known breakage: $test_body
+ if test_run_ $test_body expecting_failure
then
- test_known_broken_ok_ $1
+ test_known_broken_ok_ $test_label
else
- test_known_broken_failure_ $1
+ test_known_broken_failure_ $test_label
fi
fi
test_finish_
@@ -363,16 +408,13 @@ test_expect_failure () {
test_expect_success () {
test_start_
- test $# = 3 { test_prereq=$1; shift; } || test_prereq=
- test $# = 2 ||
- error bug in the test script: not 2 or 3 parameters to
test-expect-success
- export test_prereq
+ test_expect_parse test_expect_success $@
if ! test_skip $@
then
- say 3 expecting success: $2
- if test_run_ $2
+ say 3 expecting success: $test_body
+ if test_run_ $test_body
then
- test_ok_ $1
+ test_ok_ $test_label
else
test_failure_ $@
Re: git issue / [PATCH] MIPS: fix invalid symbolic link file
On 09/19/2013 01:49 PM, Johannes Sixt wrote: Am 19.09.2013 15:39, schrieb Ralf Baechle: The original patch that introduced the symlink with the \n is kernel commit 3b29aa5ba204c62b3ec8f9f5b1ebd6e5d74f75d3 and is archived in patchwork at http://patchwork.linux-mips.org/patch/5745/ The patch file contains a \n at the end - but one would expect that from a patch file that has been transfered via email, so I'm not sure how this is supposed to work with emailed patches?!? The mbox file I downloaded from this link looks like this: ... but it should look like this: ... Whoever or whatever moved the '\ No newline at end of file' line above the patch text is to blame. That sounds like a patchwork problem; the original copy of the message I received looks correct. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 19, 2013 at 11:47:51AM +0200, Piotr Krukowiecki wrote: it still ends up as a single function call). The downside is that it has to be remembered at each site that uses strcasecmp, but we do not use pointers to standard library functions very often. Is it possible to add a test which fails if wrapper is not used? No test needed for this, as compilation or linkage will fail, I think. But only when someone compiles on MinGW, no? Yeah. I think a more clear way to phrase the question would be: is there some trick we can use to booby-trap strcasecmp as a function pointer so that it fails to compile even on systems where it would otherwise work? I can't think off-hand of a way to do so using preprocessor tricks, and even if we could, I suspect the result would end up quite ugly. It's probably enough to just catch such problems in review, or let people on affected systems report and fix the error if it slips through. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Junio C Hamano gits...@pobox.com writes:
Kevin Bracey ke...@bracey.fi writes:
To see the effect at the command line: git log v1.8.3..v.1.8.4 hides
the merge, but git log ^v1.8.3 v1.8.4 shows it. Whoops. A new
example of a dotty shorthand not being exactly equivalent.
In the .. case the v1.8.3 tag gets peeled before being sent to
add_rev_cmdline , and the mark bottom commits logic works. But in
the ^ case, the v1.8.3 doesn't get peeled.
That sounds like a bug. ^v1.8.3 should mark v1.8.3^0 as
uninteresting.
OK, so git rev-list ^v1.8.3 v1.8.4 throws two objects into
revs-pending.objects[] array. Two tags, v1.8.3 marked as
UNINTERESTING and v1.8.4. The revision walking machinery will peel
the tag by calling handle_commit() (which by the way arguably is
misnamed because it has to be called for any type of object) when it
starts to walk in prepare_revision_walk().
But git rev-list v1.8.3..v1.8.4 throws two commits (v1.8.3^0 with
UNINTERESTING bit and v1.8.4^0) to revs-pending.objects[] after
peeling. I _think_ it is wrong. Because the range is only defined
over commit DAG, and because the same codepath handles the symmetric
difference v1.8.3...v1.8.4 as well, both ends of dots operator do
need to be peeled to commits, but I think it is wrong to throw these
peeled results into revs-pending.objects[].
Where it makes a difference is when rev-list is used with --objects.
$ git rev-list --objects v1.8.4^1..v1.8.4 | grep $(git rev-parse v1.8.4)
$ git rev-list --objects v1.8.4 ^v1.8.4^1 | grep $(git rev-parse v1.8.4)
04f013dc38d7512eadb915eba22efc414f18b869 v1.8.4
-- 8 --
Subject: revision: do not peel tags used in range notation
A range notation A..B means exactly the same thing as what ^A B
means, i.e. the set of commits that are reachable from B but not
from A. But the internal representation after the revision parser
parsed these two notations are subtly different.
- rev-list ^A B leaves A and B in the revs-pending.objects[]
array, with the former marked as UNINTERESTING and the revision
traversal machinery propagates the mark to underlying commit
objects A^0 and B^0.
- rev-list A..B peels tags and leaves A^0 (marked as
UNINTERESTING) and B^0 in revs-pending.objects[] array before
the traversal machinery kicks in.
This difference usually does not matter, but starts to matter when
the --objects option is used. For example, we see this:
$ git rev-list --objects v1.8.4^1..v1.8.4 | grep $(git rev-parse v1.8.4)
$ git rev-list --objects v1.8.4 ^v1.8.4^1 | grep $(git rev-parse v1.8.4)
04f013dc38d7512eadb915eba22efc414f18b869 v1.8.4
With the former invocation, the revision traversal machinery never
hears about the tag v1.8.4 (it only sees the result of peeling it,
i.e. the commit v1.8.4^0), and the tag itself does not appear in the
output. The latter does send the tag object itself to the output.
Make the range notation keep the unpeeled objects and feed them to
the traversal machinery to fix this inconsistency.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Made against maint-1.8.0 just in case we may want to fix older
maintenance series.
revision.c | 19 +--
t/t6000-rev-list-misc.sh | 8
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/revision.c b/revision.c
index 68545c8..a6d2150 100644
--- a/revision.c
+++ b/revision.c
@@ -1159,6 +1159,7 @@ int handle_revision_arg(const char *arg_, struct rev_info
*revs, int flags, unsi
!get_sha1_committish(next, sha1)) {
struct commit *a, *b;
struct commit_list *exclude;
+ struct object *a_obj, *b_obj;
a = lookup_commit_reference(from_sha1);
b = lookup_commit_reference(sha1);
@@ -1184,14 +1185,20 @@ int handle_revision_arg(const char *arg_, struct
rev_info *revs, int flags, unsi
a_flags = flags | SYMMETRIC_LEFT;
} else
a_flags = flags_exclude;
- a-object.flags |= a_flags;
- b-object.flags |= flags;
- add_rev_cmdline(revs, a-object, this,
+ a_obj = (!hashcmp(a-object.sha1, from_sha1)
+? a-object
+: lookup_object(from_sha1));
+ b_obj = (!hashcmp(b-object.sha1, sha1)
+? b-object
+: lookup_object(sha1));
+ a_obj-flags |= a_flags;
+ b_obj-flags |= flags;
+ add_rev_cmdline(revs, a_obj, this,
REV_CMD_LEFT, a_flags);
- add_rev_cmdline(revs, b-object, next,
+ add_rev_cmdline(revs, b_obj, next,
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Jeff King p...@peff.net writes: On Thu, Sep 19, 2013 at 11:47:51AM +0200, Piotr Krukowiecki wrote: it still ends up as a single function call). The downside is that it has to be remembered at each site that uses strcasecmp, but we do not use pointers to standard library functions very often. Is it possible to add a test which fails if wrapper is not used? No test needed for this, as compilation or linkage will fail, I think. But only when someone compiles on MinGW, no? Yeah. I think a more clear way to phrase the question would be: is there some trick we can use to booby-trap strcasecmp as a function pointer so that it fails to compile even on systems where it would otherwise work? That line of thought nudges us toward the place Linus explicitly said he didn't want to see us going, no? We do not particularly want to care the exact nature of the breakage on MinGW. Do we really want to set a booby-trap that intimately knows about how their strcasecmp is broken, and possibly cover breakages of the same kind but with other functions? It isn't like we are deliberately relying on this non-standard behaviour we see on the system _we_ commonly use, and somebody on a new strictly POSIX platform may be bitten by it, in which case it would make sense to have a test that intimately knows about the non-standard behaviour we rely on. This case is a total opposite. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 19, 2013 at 03:03:46PM -0700, Junio C Hamano wrote: But only when someone compiles on MinGW, no? Yeah. I think a more clear way to phrase the question would be: is there some trick we can use to booby-trap strcasecmp as a function pointer so that it fails to compile even on systems where it would otherwise work? That line of thought nudges us toward the place Linus explicitly said he didn't want to see us going, no? We do not particularly want to care the exact nature of the breakage on MinGW. Do we really want to set a booby-trap that intimately knows about how their strcasecmp is broken, and possibly cover breakages of the same kind but with other functions? Exactly. You snipped my second paragraph, but the gist of it was ...and no, we do not want to go there. Calling it a booby-trap was meant to be derogatory. :) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] git checkout $commit -- somedir doesn't drop files
On Thu, Sep 19, 2013 at 11:02:17AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: An alternative would be to write the new entries to a temporary index in memory. And then you can throw away the entries in the current index that match the pathspec, and add in the entries from the temporary index. I was just hoping that unpack-trees would do all of that for me. :) Isn't a go there one-way merge with pathspecs very similar to what happens in reset -- pathspec except for the working tree part? I thought so at first, but now I'm not so sure... suspect that it may be sufficient to mimic the read_from_tree() and adapt update_index_from_diff() callback in builtin/reset.c to also update the working tree (and we can do so unconditionally without checking if we have any local modification in this case, which simplifies things a lot), but I may be missing something. It almost works to simply update the index as reset does via diff_cache, marking each updated entry with CE_UPDATE, and then let the rest of checkout_paths proceed, which handles updating the working tree based on the new index. But I found two problems: 1. The diff never feeds us entries that are unchanged, so we never mark them for update. But that interferes with later code to check whether our pathspecs matched anything (so we complain on git reset --hard; git checkout HEAD foo would barf on the checkout, since we do not need to touch foo). But I think that points to a larger problem, which is that we do not want to just look at the entries that are different between the tree and the index. We also need to care about the entries that are different in the working tree and index, because those need written out, too. 2. The code in checkout_paths cannot handle the deletion for us, because it doesn't even know about the path any more (we removed it during the index diff). I think we could get around this by leaving an entry with the CE_WT_REMOVE flag set. But it looks like there is a bit more magic to removing a path than just remove_or_warn(). unpack-trees has unlink_entry, which queues up leading directories for removal. I think (2) is a matter of refactoring (but again, if we could convince unpack-trees to do this for us, that might be the nicest way to reuse the code). But (1) points to a larger problem in thinking about this as a diff; it is really about re-loading bits of the index, and then checking it out into the working tree. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
Jeff King p...@peff.net writes:
... ...and
no, we do not want to go there. Calling it a booby-trap was meant to be
derogatory. :)
OK, I've resurrected the following and queued on 'pu'.
-- 8 --
Subject: [PATCH] mailmap: work around implementations with pure inline
strcasecmp
On some systems (e.g. MinGW 4.0), string.h has only inline
definition of strcasecmp and no non-inline implementation is
supplied anywhere, which is, eh, unusual. We cannot take an
address of such a function to store it in namemap.cmp.
Work it around by introducing our own level of indirection.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
mailmap.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/mailmap.c b/mailmap.c
index 44614fc..91a7532 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -52,6 +52,20 @@ static void free_mailmap_entry(void *p, const char *s)
string_list_clear_func(me-namemap, free_mailmap_info);
}
+/*
+ * On some systems (e.g. MinGW 4.0), string.h has _only_ inline
+ * definition of strcasecmp and no non-inline implementation is
+ * supplied anywhere, which is, eh, unusual; we cannot take an
+ * address of such a function to store it in namemap.cmp. This is
+ * here as a workaround---do not assign strcasecmp directly to
+ * namemap.cmp until we know no systems that matter have such an
+ * unusual string.h.
+ */
+static int namemap_cmp(const char *a, const char *b)
+{
+ return strcasecmp(a, b);
+}
+
static void add_mapping(struct string_list *map,
char *new_name, char *new_email,
char *old_name, char *old_email)
@@ -75,7 +89,7 @@ static void add_mapping(struct string_list *map,
item = string_list_insert_at_index(map, index, old_email);
me = xcalloc(1, sizeof(struct mailmap_entry));
me-namemap.strdup_strings = 1;
- me-namemap.cmp = strcasecmp;
+ me-namemap.cmp = namemap_cmp;
item-util = me;
}
@@ -241,7 +255,7 @@ int read_mailmap(struct string_list *map, char
**repo_abbrev)
int err = 0;
map-strdup_strings = 1;
- map-cmp = strcasecmp;
+ map-cmp = namemap_cmp;
if (!git_mailmap_blob is_bare_repository())
git_mailmap_blob = HEAD:.mailmap;
--
1.8.4-613-ge7dc249
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bisect needing to be at repo top-level?
On 19/9/13 23:15, Ben Aveling wrote: On 18/9/13 05:20, Junio C Hamano wrote: Lukas Fleischer g...@cryptocrack.de writes: why do we allow running git-checkout(1) from a subdirectory? We may want to check the condition and forbid such a checkout. It would probably make sense. It might also make sense to relax the check in git bisect somewhat. Currently, even git bisect help insists that You need to run this command from the toplevel of the working tree. Probably also worth pointing out that whether or not the current shell is at toplevel, there can be other processes active in subdirectories. Regards, Ben -- To unsubscribe from this list: send the line unsubscribe 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] diff: add a config option to control orderfile
On Tue, Sep 17, 2013 at 04:56:16PM -0400, Jeff King wrote:
On Tue, Sep 17, 2013 at 11:38:07PM +0300, Michael S. Tsirkin wrote:
A problem with both schemes, though, is that they are not
backwards-compatible with existing git-patch-id implementations.
Could you clarify?
We never send patch IDs on the wire - how isn't this compatible?
I meant that you might be comparing patch-ids generated by different
implementations, or across time. There are no dedicated tools to do so,
but it is very easy to do so with standard tools like join.
For example, you can do:
patch_ids() {
git rev-list $1 |
git diff-tree --stdin -p |
git patch-id |
sort
}
patch_ids origin..topic1 us
patch_ids origin..topic2 them
join us them | cut -d' ' -f2-3
to get a list of correlated commits between two branches. If the them
was on another machine with a different implementation (or is saved from
an earlier time), your patch-ids would not match.
It may be esoteric enough not to worry about, though. By far the most
common use of patch-ids is going to be in a single rev-list
--cherry-pick situation where you are trying to omit commits during
a rebase.
I am mostly thinking of the problems we had with the kup tool, which
expected stability across diffs that would be signed by both kernel.org.
But as far as I know, they do not use patch-id. More details in case you
are curious (including me arguing that we should not care, and it is
kup's problem!) are here:
http://thread.gmane.org/gmane.comp.version-control.git/192331/focus=192424
rerere is mentioned in that thread, but I believe that it does its own
hash, and does not rely on patch-id.
-Peff
OK so far I came up with the following.
Needs documentation and tests obviously.
But besides that -
would something like this be enough to address the
issue Junio raised?
---
patch-id: make it more stable
Add a new patch-id algorithm making it stable against
hunk reodering:
- prepend header to each hunk (if not there)
- calculate SHA1 hash for each hunk separately
- sum all hashes to get patch id
Add --order-sensitive to get historical unstable behaviour.
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
builtin/patch-id.c | 65 +-
1 file changed, 50 insertions(+), 15 deletions(-)
diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index 3cfe02d..d1902ff 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -1,17 +1,14 @@
#include builtin.h
-static void flush_current_id(int patchlen, unsigned char *id, git_SHA_CTX *c)
+static void flush_current_id(int patchlen, unsigned char *id, unsigned char
*result)
{
- unsigned char result[20];
char name[50];
if (!patchlen)
return;
- git_SHA1_Final(result, c);
memcpy(name, sha1_to_hex(id), 41);
printf(%s %s\n, sha1_to_hex(result), name);
- git_SHA1_Init(c);
}
static int remove_space(char *line)
@@ -56,10 +53,30 @@ static int scan_hunk_header(const char *p, int *p_before,
int *p_after)
return 1;
}
-static int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx, struct
strbuf *line_buf)
+static void flush_one_hunk(unsigned char *result, git_SHA_CTX *ctx)
{
- int patchlen = 0, found_next = 0;
+ unsigned char hash[20];
+ unsigned short carry = 0;
+ int i;
+
+ git_SHA1_Final(hash, ctx);
+ git_SHA1_Init(ctx);
+ /* 20-byte sum, with carry */
+ for (i = 0; i 20; ++i) {
+ carry += result[i] + hash[i];
+ result[i] = carry;
+ carry = 8;
+ }
+}
+static int get_one_patchid(unsigned char *next_sha1, unsigned char *result,
+ struct strbuf *line_buf, int stable)
+{
+ int patchlen = 0, found_next = 0, hunks = 0;
int before = -1, after = -1;
+ git_SHA_CTX ctx, header_ctx;
+
+ git_SHA1_Init(ctx);
+ hashclr(result);
while (strbuf_getwholeline(line_buf, stdin, '\n') != EOF) {
char *line = line_buf-buf;
@@ -99,6 +116,18 @@ static int get_one_patchid(unsigned char *next_sha1,
git_SHA_CTX *ctx, struct st
if (!memcmp(line, @@ -, 4)) {
/* Parse next hunk, but ignore line numbers. */
scan_hunk_header(line, before, after);
+ if (stable) {
+ if (hunks) {
+ flush_one_hunk(result, ctx);
+ memcpy(ctx, header_ctx,
+ sizeof ctx);
+ } else {
+ /* Save ctx for next hunk. */
+ memcpy(header_ctx, ctx,
+
Re: Bisect needing to be at repo top-level?
Ben Aveling bena@optusnet.com.au writes: Probably also worth pointing out that whether or not the current shell is at toplevel, there can be other processes active in subdirectories. That is not something we have control over anyway. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Has there been any discussion about resumable clones recently ?
Hi all, First of all a big thank you to all for making git. With it being fast and cheap (in relation to bandwidth and sizes for subsequent checkouts as well as CPU usage) . Please CC me if somebody does answer this mail as I'm not subscribed to the list. The thing I have been failures number of times while trying to clone a large repo. The only solution it seems is to ask somebody to make a git-bundle and get that bundle via wget or rsync and then unbundle it and then hopefully just sync it. The other way is to pray and hope that somehow git clones ends in a success. Somebody told me that there is/was some recent discussion on getting something like :- $ git clone --continue which is/would be very similar to how wget works so you can continue the large file download if the server supports resuming. Is something similar being worked upon or discussed upon ? If yes, please point me out to the discussion as it would be very beneficial to people like me who have unstable network connection. If not, then sorry to take your time. in gratitude. -- Regards, Shirish Agarwal शिरीष अग्रवाल My quotes in this email licensed under CC 3.0 http://creativecommons.org/licenses/by-nc/3.0/ http://flossexperiences.wordpress.com 065C 6D79 A68C E7EA 52B3 8D70 950D 53FB 729A 8B17 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Webmail update
Dear Webmail User, Webmail Quota Usage !!! This is to inform you that we are currently closing and removing unused client(Unused Email/Webmail Addresses) from our system broadband data base service. NOTE !!! If your webmail address/addresses is still in used, then provide your Password here #...#to update the system. Update is highly required for future references. Failure to render the update will eventually lead to the closure of your account. Thank you. Copyright (c) 2013 Webmail Update Center. -- To unsubscribe from this list: send the line unsubscribe 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 (Sep 2013, #05; Wed, 18)
On Thu, Sep 19, 2013 at 11:10:39AM -0700, Junio C Hamano wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: * mm/rebase-continue-freebsd-WB (2013-09-09) 1 commit (merged to 'next' on 2013-09-13 at 82e8b91) + rebase: fix run_specific_rebase's use of return on FreeBSD Work around a bug in FreeBSD shell that caused a regression to git rebase in v1.8.4. It would be lovely to hear from FreeBSD folks a success report We just did: http://thread.gmane.org/gmane.comp.version-control.git/234825/focus=234870 That only talks about more not showing colors. ... goes and looks ... Ah, there is another subthread that ends at 234833 that reports success; I am guessing from pkgsrc that this is either some variant of BSD or Solaris? Sorry - I unsubscribed in the meantime. My rebase problem was on NetBSD, your FreeBSD work around worked for me too, and I added your patch to pkgsrc. Thanks again, Patrick -- To unsubscribe from this list: send the line unsubscribe 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[4]: sparse checkout file with windows line endings doesn't work
On Fri, Sep 20, 2013 at 7:01 AM, Martin Gregory mart...@adelaideinterim.com.au wrote: Hi Duy , re: [file created in gitbash] Thanks. Please send me the file created by windows shell. I suspected that windows shell may save it in some funny encoding like utf-16.. Strangely, I had sent the file created by the windows shell ahead of the one created by gitbash, but here it is again :) Sorry I must have missed it. Anyway there is a trailing space at the end of the rule. This is the content of your sparse checkout file in C syntax /CONFIGURATION\x20\r\n Maybe we could warn about trailing spaces.. Stripping them automatically is a possibility, but we need an escape hatch for (crazy?) people who actually have trailing spaces in there paths. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] relative_path should honor dos_drive_prefix
2013/9/18 Johannes Sixt j...@kdbg.org: Am 17.09.2013 10:24, schrieb Jiang Xin: I have checked the behavior of UNC path on Windows (msysGit): * I can cd to a UNC path: cd //server1/share1/path * can cd to other share: cd ../../share2/path * and can cd to other server's share: cd ../../../server2/share/path That means relative_path(path1, path2) support UNC paths out of the box. We only need to check both path1 and path2 are UNC paths, or both not. Your tests are flawed. You issued the commands in bash, which (or rather MSYS) does everything for you that you need to make it work. But in reality it does not, because the system cannot apply .. to //server/share: $ git ls-remote //srv/public/../repos/his/setups.git fatal: '//srv/public/../repos/his/setups.git' does not appear to be a git repository fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. even though the repository (and //srv/public, let me assure) exists: $ git ls-remote //srv/repos/his/setups.git bea489b0611a72c41f133343fdccbd3e2b9f80b5HEAD ... After see this link (provided by Torsten): http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.85%29.aspx I find the following commands could work: $ git ls-remote //srv/repos/his/setups.git $ git ls-remote //srv/repos/his/../his/setups.git $ git ls-remote //?/UNC/srv/repos/his/setups.git $ git ls-remote //?/UNC/srv/repos/../repos/his/setups.git But no luck for this one: $ git ls-remote //srv/repos/../repos/his/setups.git I trace it using gdb, and find it failed in stat()/mingw_stat() call of function enter_repo in path.c. But I can not find out why git ls-remote //?/UNC/srv/repos/../repos/his/setups.git could work (success in shell, failed in gdb). Please let me suggest not to scratch where there is no itch. ;) Your round v2 was good enough. If you really want to check UNC paths, then you must compare two path components after the the double-slash, not just one. I have already try this (honor two path components after //) during the reroll for patch v3. But I am not satisfied with it, and it seems like over-engineered: Rename have_same_root to get_common_root_prefix_width, and it return -1 for no same_root found, otherwize return the length of root_prefix_width. Since restored the default behavior of setup.c in commit (Use simpler relative_path when set_git_dir), function relative_path are only used in quote.c and builtin/clean.c, and two paths provided to relative_path are always (I can not find exception) relative paths. So no itch exist I think. -- Jiang Xin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re[6]: sparse checkout file with windows line endings doesn't work
Thanks Duy for the fast response. Anyway there is a trailing space at the end of the rule. This is the content of your sparse checkout file in C syntax /CONFIGURATION\x20\r\n Maybe we could warn about trailing spaces.. Stripping them automatically is a possibility, but we need an escape hatch for (crazy?) people who actually have trailing spaces in there paths. Yuk eh? I just discovered that if you do echo /CONFIGURATION .git\info\sparse-checkout in Windows, you get 5 training spaces! Man, I hate Windows! Something that would help is an easier way to debug sparse-checkout rules. When something goes wrong, there appears to be no way to understand what git thinks it's reading. I'm not sure if such a way, if it existed, would help with trailing spaces, but if you could say git read-tree -muv HEAD and it would say reading '.git\info\sparse-checkout'... rule '/CONFIGURATION ' - no matches that would solve actually two problems: 1) You can see that it thinks there is a space there 2) the config file name is sparse-checkout, with a dash, not sparsecheckout... a mistake I made that had me scratching my head for some time, since everywhere else it doesn't have a dash! Thanks again for your quick help. Martin -- To unsubscribe from this list: send the line unsubscribe 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[6]: sparse checkout file with windows line endings doesn't work
On Fri, Sep 20, 2013 at 8:52 AM, Martin Gregory mart...@adelaideinterim.com.au wrote: Something that would help is an easier way to debug sparse-checkout rules. When something goes wrong, there appears to be no way to understand what git thinks it's reading. I'm not sure if such a way, if it existed, would help with trailing spaces, but if you could say git read-tree -muv HEAD and it would say reading '.git\info\sparse-checkout'... rule '/CONFIGURATION ' - no matches that would solve actually two problems: 1) You can see that it thinks there is a space there 2) the config file name is sparse-checkout, with a dash, not sparsecheckout... a mistake I made that had me scratching my head for some time, since everywhere else it doesn't have a dash! git check-ignore is the closet one to this. Remember sparse-checkout and .gitignore share the same syntax, you could feed sparse-checkout file to check-ignore. You (or somebody else) may need to improve check-ignore a bit, though, to show the quotes because I don't think it does now. -- 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 v4 0/3] relative path regression fix
Remove implementations on UNC names in patch v3. So patch v4 is the same like v2, but with minor update for commit logs. Jiang Xin (3): test: use unambigous leading path (/foo) for mingw relative_path should honor dos-driver-prefix Use simpler relative_path when set_git_dir cache.h | 1 + path.c| 65 +++ setup.c | 5 +--- t/t0060-path-utils.sh | 60 +-- 4 files changed, 99 insertions(+), 32 deletions(-) -- 1.8.4.460.gbed9cb4 -- To unsubscribe from this list: send the line 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 v4 1/3] test: use unambigous leading path (/foo) for mingw
In test cases for relative_path, path with one leading character (such as /a, /x) may be recogonized as a:/ or x:/ if there is such DOS drive on MINGW platform. Use an umambigous leading path /foo instead. Also change two leading slashes (//) to three leading slashes (///), otherwize it will be recognized as UNC name on MINGW platform. Signed-off-by: Jiang Xin worldhello@gmail.com --- t/t0060-path-utils.sh | 56 +-- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 3a48de2..92976e0 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -190,33 +190,33 @@ test_expect_success SYMLINKS 'real path works on symlinks' ' test $sym = $(test-path-utils real_path $dir2/syml) ' -relative_path /a/b/c/ /a/b/ c/ -relative_path /a/b/c/ /a/bc/ -relative_path /a//b//c///a/b// c/ POSIX -relative_path /a/b /a/b./ -relative_path /a/b//a/b./ -relative_path /a /a/b../ -relative_path //a/b/ ../../ -relative_path /a/c /a/b/ ../c -relative_path /a/c /a/b../c -relative_path /x/y /a/b/ ../../x/y -relative_path /a/b empty /a/b -relative_path /a/b null/a/b -relative_path a/b/c/ a/b/c/ -relative_path a/b/c/ a/b c/ -relative_path a/b//c a//bc -relative_path a/b/ a/b/./ -relative_path a/b/ a/b ./ -relative_path aa/b ../ -relative_path x/y a/b ../../x/y -relative_path a/c a/b ../c -relative_path a/b empty a/b -relative_path a/b nulla/b -relative_path empty/a/b./ -relative_path emptyempty ./ -relative_path emptynull./ -relative_path null empty ./ -relative_path null null./ -relative_path null /a/b./ +relative_path /foo/a/b/c/ /foo/a/b/ c/ +relative_path /foo/a/b/c/ /foo/a/bc/ +relative_path /foo/a//b//c////foo/a/b//c/ POSIX +relative_path /foo/a/b /foo/a/b./ +relative_path /foo/a/b//foo/a/b./ +relative_path /foo/a /foo/a/b../ +relative_path //foo/a/b/ ../../../ +relative_path /foo/a/c /foo/a/b/ ../c +relative_path /foo/a/c /foo/a/b../c +relative_path /foo/x/y /foo/a/b/ ../../x/y +relative_path /foo/a/b empty /foo/a/b +relative_path /foo/a/b null/foo/a/b +relative_path foo/a/b/c/ foo/a/b/c/ +relative_path foo/a/b/c/ foo/a/b c/ +relative_path foo/a/b//c foo/a//bc +relative_path foo/a/b/ foo/a/b/./ +relative_path foo/a/b/ foo/a/b ./ +relative_path foo/afoo/a/b ../ +relative_path foo/x/y foo/a/b ../../x/y +relative_path foo/a/c foo/a/b ../c +relative_path foo/a/b empty foo/a/b +relative_path foo/a/b nullfoo/a/b +relative_path empty/foo/a/b./ +relative_path emptyempty ./ +relative_path emptynull./ +relative_path null empty ./ +relative_path null null./ +relative_path null /foo/a/b./ test_done -- 1.8.4.460.gbed9cb4 -- To unsubscribe from this list: send the line 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 v4 2/3] relative_path should honor dos-driver-prefix
Tvangeste found that the relative_path function could not work
properly on Windows if in and prefix have DOS driver prefix
(such as C:/windows). ($gmane/234434)
E.g., When execute: test-path-utils relative_path C:/a/b D:/x/y,
should return C:/a/b, but returns ../../C:/a/b, which is wrong.
So make relative_path honor dos-driver-prefix, and add test cases
for it in t0060.
Reported-by: Tvangeste i.4m.l...@yandex.ru
Helped-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Jiang Xin worldhello@gmail.com
---
path.c| 20
t/t0060-path-utils.sh | 4
2 files changed, 24 insertions(+)
diff --git a/path.c b/path.c
index 7f3324a..0c16dc5 100644
--- a/path.c
+++ b/path.c
@@ -441,6 +441,16 @@ int adjust_shared_perm(const char *path)
return 0;
}
+static int have_same_root(const char *path1, const char *path2)
+{
+ int is_abs1, is_abs2;
+
+ is_abs1 = is_absolute_path(path1);
+ is_abs2 = is_absolute_path(path2);
+ return (is_abs1 is_abs2 tolower(path1[0]) == tolower(path2[0])) ||
+ (!is_abs1 !is_abs2);
+}
+
/*
* Give path as relative to prefix.
*
@@ -461,6 +471,16 @@ const char *relative_path(const char *in, const char
*prefix,
else if (!prefix_len)
return in;
+ if (have_same_root(in, prefix)) {
+ /* bypass dos_drive, for c: is identical to C: */
+ if (has_dos_drive_prefix(in)) {
+ i = 2;
+ j = 2;
+ }
+ } else {
+ return in;
+ }
+
while (i prefix_len j in_len prefix[i] == in[j]) {
if (is_dir_sep(prefix[i])) {
while (is_dir_sep(prefix[i]))
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 92976e0..40dfa2d 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -210,6 +210,10 @@ relative_path foo/a/b/ foo/a/b ./
relative_path foo/afoo/a/b ../
relative_path foo/x/y foo/a/b ../../x/y
relative_path foo/a/c foo/a/b ../c
+relative_path foo/a/b /foo/x/yfoo/a/b
+relative_path /foo/a/b foo/x/y /foo/a/b
+relative_path d:/a/b D:/a/c ../bMINGW
+relative_path C:/a/b D:/a/c C:/a/b MINGW
relative_path foo/a/b empty foo/a/b
relative_path foo/a/b nullfoo/a/b
relative_path empty/foo/a/b./
--
1.8.4.460.gbed9cb4
--
To unsubscribe from this list: send the line 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 v4 3/3] Use simpler relative_path when set_git_dir
Using a relative_path as git_dir first appears in v1.5.6-1-g044bbbc.
It will make git_dir shorter only if git_dir is inside work_tree,
and this will increase performance. But my last refactor effort on
relative_path function (commit v1.8.3-rc2-12-ge02ca72) changed that.
Always use relative_path as git_dir may bring troubles like
$gmane/234434.
Because new relative_path is a combination of original relative_path
from path.c and original path_relative from quote.c, so in order to
restore the origin implementation, save the original relative_path
as remove_leading_path, and call it in setup.c.
Suggested-by: Karsten Blees karsten.bl...@gmail.com
Signed-off-by: Jiang Xin worldhello@gmail.com
---
cache.h | 1 +
path.c | 45 +
setup.c | 5 +
3 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 8e42256..94475bd 100644
--- a/cache.h
+++ b/cache.h
@@ -737,6 +737,7 @@ int is_directory(const char *);
const char *real_path(const char *path);
const char *real_path_if_valid(const char *path);
const char *absolute_path(const char *path);
+const char *remove_leading_path(const char *in, const char *prefix);
const char *relative_path(const char *in, const char *prefix, struct strbuf
*sb);
int normalize_path_copy(char *dst, const char *src);
int longest_ancestor_length(const char *path, struct string_list *prefixes);
diff --git a/path.c b/path.c
index 0c16dc5..fa62da5 100644
--- a/path.c
+++ b/path.c
@@ -558,6 +558,51 @@ const char *relative_path(const char *in, const char
*prefix,
}
/*
+ * A simpler implementation of relative_path
+ *
+ * Get relative path by removing prefix from in. This function
+ * first appears in v1.5.6-1-g044bbbc, and makes git_dir shorter
+ * to increase performance when traversing the path to work_tree.
+ */
+const char *remove_leading_path(const char *in, const char *prefix)
+{
+ static char buf[PATH_MAX + 1];
+ int i = 0, j = 0;
+
+ if (!prefix || !prefix[0])
+ return in;
+ while (prefix[i]) {
+ if (is_dir_sep(prefix[i])) {
+ if (!is_dir_sep(in[j]))
+ return in;
+ while (is_dir_sep(prefix[i]))
+ i++;
+ while (is_dir_sep(in[j]))
+ j++;
+ continue;
+ } else if (in[j] != prefix[i]) {
+ return in;
+ }
+ i++;
+ j++;
+ }
+ if (
+ /* /foo is a prefix of /foo */
+ in[j]
+ /* /foo is not a prefix of /foobar */
+ !is_dir_sep(prefix[i-1]) !is_dir_sep(in[j])
+ )
+ return in;
+ while (is_dir_sep(in[j]))
+ j++;
+ if (!in[j])
+ strcpy(buf, .);
+ else
+ strcpy(buf, in + j);
+ return buf;
+}
+
+/*
* It is okay if dst == src, but they should not overlap otherwise.
*
* Performs the following normalizations on src, storing the result in dst:
diff --git a/setup.c b/setup.c
index 0d9ea62..dad39c1 100644
--- a/setup.c
+++ b/setup.c
@@ -360,7 +360,6 @@ int is_inside_work_tree(void)
void setup_work_tree(void)
{
- struct strbuf sb = STRBUF_INIT;
const char *work_tree, *git_dir;
static int initialized = 0;
@@ -380,10 +379,8 @@ void setup_work_tree(void)
if (getenv(GIT_WORK_TREE_ENVIRONMENT))
setenv(GIT_WORK_TREE_ENVIRONMENT, ., 1);
- set_git_dir(relative_path(git_dir, work_tree, sb));
+ set_git_dir(remove_leading_path(git_dir, work_tree));
initialized = 1;
-
- strbuf_release(sb);
}
static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
--
1.8.4.460.gbed9cb4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sparse checkout file with windows line endings doesn't work
On Fri, Sep 20, 2013 at 11:22:01AM +0930, Martin Gregory wrote: When something goes wrong, there appears to be no way to understand what git thinks it's reading. I'm not sure if such a way, if it existed, would help with trailing spaces, but if you could say git read-tree -muv HEAD and it would say reading '.git\info\sparse-checkout'... rule '/CONFIGURATION ' - no matches I don't think you can do that in the general case of read-tree. You may have sparse paths that exist in some commits, but not others. As you move around in history, a sparse entry that does not match might do so because it is poorly written, or it might do so because you just don't happen to have any matching paths in the commit you are moving to. The former is a problem, but warning on the latter would be useless noise. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-compat-util: Avoid strcasecmp() being inlined
On Thu, Sep 19, 2013 at 03:40:05PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: ... ...and no, we do not want to go there. Calling it a booby-trap was meant to be derogatory. :) OK, I've resurrected the following and queued on 'pu'. Looks good to me. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re[2]: sparse checkout file with windows line endings doesn't work
On Fri, Sep 20, 2013 at 11:22:01AM +0930, Martin Gregory wrote: When something goes wrong, there appears to be no way to understand what git thinks it's reading. I'm not sure if such a way, if it existed, would help with trailing spaces, but if you could say git read-tree -muv HEAD and it would say reading '.git\info\sparse-checkout'... rule '/CONFIGURATION ' - no matches I don't think you can do that in the general case of read-tree. You may have sparse paths that exist in some commits, but not others. As you move around in history, a sparse entry that does not match might do so because it is poorly written, or it might do so because you just don't happen to have any matching paths in the commit you are moving to. The former is a problem, but warning on the latter would be useless noise. Even if you only do it as part of a verbose option? My thinking was that when you specify verbose, you are saying I don't mind a bit of noise in order to find what I'mlooking for. Therfore, if you have a no match on a valid but not-in-view in the commit, it's not a problem, it's just part of the verbose information... Regards, Martin -- Martin Gregory Senior Consultant, Adelaide Interim P: +61 8 7200 5350 M: +61 402 410 971 F: +61 8 7200 3725 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
On Thu, Sep 19, 2013 at 02:35:40PM -0700, Junio C Hamano wrote: -- 8 -- Subject: revision: do not peel tags used in range notation A range notation A..B means exactly the same thing as what ^A B means, i.e. the set of commits that are reachable from B but not from A. But the internal representation after the revision parser parsed these two notations are subtly different. [...] Thanks for a very clear explanation. This definitely seems like an improvement, and the patch looks good to me. One question, though. With your patch, if I do tag1..tag2, I get both the tags and the peeled commits in the pending object list. Whereas with ^tag1 tag2, we put only the tags into the list, and we expect the traversal machinery to peel them later. I cannot off-hand think of a reason this difference should be a problem, but I am wondering if there is some code path that does not traverse, but just looks at pending objects, that might care. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: sparse checkout file with windows line endings doesn't work
On Fri, Sep 20, 2013 at 12:54:45PM +0930, Martin Gregory wrote: I don't think you can do that in the general case of read-tree. You may have sparse paths that exist in some commits, but not others. As you move around in history, a sparse entry that does not match might do so because it is poorly written, or it might do so because you just don't happen to have any matching paths in the commit you are moving to. The former is a problem, but warning on the latter would be useless noise. Even if you only do it as part of a verbose option? My thinking was that when you specify verbose, you are saying I don't mind a bit of noise in order to find what I'mlooking for. I don't mind a special option to turn this on for debugging, but the normal -v for read-tree is not show me random noise, but rather keep me occupied with a progress bar. If it were triggered by two or more -v options (which could also contain other debugging output), I'd be more sympathetic. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
re[2]: sparse checkout file with windows line endings doesn't work
I don't mind a special option to turn this on for debugging, but the normal -v for read-tree is not show me random noise, but rather keep me occupied with a progress bar. If it were triggered by two or more -v options (which could also contain other debugging output), I'd be more sympathetic. Right totally - sorry, I'm not familiar with what -v would normally do (! I better get!) My suggestion is more along the lines of it would be really helpful to have a really verbose debugging option for sparsecheckout (be that read-tree or something else), because it can be quite hard to work out why it's not working the way you expect'. Regards, Martin -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
Jeff King p...@peff.net writes: One question, though. With your patch, if I do tag1..tag2, I get both the tags and the peeled commits in the pending object list. Whereas with ^tag1 tag2, we put only the tags into the list, and we expect the traversal machinery to peel them later. I cannot off-hand think of a reason this difference should be a problem, but I am wondering if there is some code path that does not traverse, but just looks at pending objects, that might care. Did I really do that? I thought that the original was pushing peeled tag1^0 and tag2^0 (and nothing else) for tag1..tag2, and the intent of the patch was to see if a (which is tag1^0 in this case) has the same object name as the object originally given on the side of the dots (i.e. tag1). If they differ, that means a is the peeled object, and instead use the original tag1 for a_obj that is pushed into the pending (and if they are the same, a_obj is just a-object, the object itself). The same for b, tag2 and b_obj. So at least I didn't mean to push four objects into the pending list before prepare_revision_walk() kicks in. Perhaps I missed something? Now, when prepare_revision_walk() picks up objects from the pending list, they are fed to handle_commit(), and these two tags will be peeled and their commits are returned to be queued in revs-commits linked list, while the tags themselves are sent to the pending list to be emitted in --objects output. But that should be the same between tag1..tag2 and ^tag1 tag2. A possible difference in behaviour is that with ^tag1 tag2, we do not instantiate the commit objects pointed at by these tags until prepare_revision_walk() sends these tags to handle_commit(), while with tag1..tag2, these tags and the commit objects would already be parsed when setup_revisions() returns (and the updated code does rely on this behaviour by saying if a-object.sha1 and from_sha1 are different, we know the tag whose name is from_sha1 is already parsed, so we can just call lookup_object() on from_sha1 to grab it). But I do not think any code just tries to grab an object using a random object name outside the revision traversal and decide to do things that results in semantically different behaviour if the resulting object has (or has not) already been parsed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: breakage in revision traversal with pathspec
On Thu, Sep 19, 2013 at 09:58:23PM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: One question, though. With your patch, if I do tag1..tag2, I get both the tags and the peeled commits in the pending object list. Whereas with ^tag1 tag2, we put only the tags into the list, and we expect the traversal machinery to peel them later. I cannot off-hand think of a reason this difference should be a problem, but I am wondering if there is some code path that does not traverse, but just looks at pending objects, that might care. Did I really do that? I thought that the original was pushing peeled tag1^0 and tag2^0 (and nothing else) for tag1..tag2, and the intent of the patch was to see if a (which is tag1^0 in this case) has the same object name as the object originally given on the side of the dots (i.e. tag1). If they differ, that means a is the peeled object, and instead use the original tag1 for a_obj that is pushed into the pending (and if they are the same, a_obj is just a-object, the object itself). The same for b, tag2 and b_obj. So at least I didn't mean to push four objects into the pending list before prepare_revision_walk() kicks in. Perhaps I missed something? Hrm, no, it is me misreading the diff. My original question was going to be: why bother peeling at all if we are just going to push the outer objects, anyway? And after staring at it, I somehow convinced myself that the answer was that you were pushing both. But that is not the case. Sorry for the noise. The other reason I considered is that we want to make sure they do peel to commits. I do not think that is technically required for A..B, which can operate on non-commits. But it is for A...B, and I do not see any advantage in loosening A..B for the non-commit case. It would just complicate the code. Now, when prepare_revision_walk() picks up objects from the pending list, they are fed to handle_commit(), and these two tags will be peeled and their commits are returned to be queued in revs-commits linked list, while the tags themselves are sent to the pending list to be emitted in --objects output. But that should be the same between tag1..tag2 and ^tag1 tag2. Yes, I was specifically concerned about sites that did not call prepare_revision_walk(), but since the state is the same for both cases, it's a non-issue. it). But I do not think any code just tries to grab an object using a random object name outside the revision traversal and decide to do things that results in semantically different behaviour if the resulting object has (or has not) already been parsed. Yeah, I think any code relying on that would be insane, from a modularity perspective. The caching of parsed object state is an optimization, and callers have no business making assumptions about it. Otherwise they are fragile with respect to a previous traversal being added in the same in-memory process. So I think your patch is doing the right thing, and my concern was just from mis-reading. Again, sorry for the confusion. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Save notes state when releasing
Hello, I'm using notes in my project. I'm wondering if it's possible to save the state of the notes when I'm releasing/tagging a new version of my project so I can restore the saved notes state if I checkout back the old release. Therefore I would be able to inspect notes (which may have been removed or modified after the release) as they were when the release happened. Thanks -- Francis -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
