[PATCH] branch.c: Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}"
From: DineshThe "-" shorthand can be used as a replacement for "@{-1}" to refer to the previous branch the user was on in the "git branch -d @{-1}" command. Replace "-" argument with "@{-1}" when the command line arguments are parsed. Signed-off-by: Dinesh --- builtin/branch.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 7b45b6b..98d2c4b 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -24,7 +24,7 @@ static const char * const builtin_branch_usage[] = { N_("git branch [] [-r | -a] [--merged | --no-merged]"), N_("git branch [] [-l] [-f] []"), - N_("git branch [] [-r] (-d | -D) ..."), + N_("git branch [] [-r] (-d | -D) [-] ..."), N_("git branch [] (-m | -M) [] "), N_("git branch [] [-r | -a] [--points-at]"), NULL @@ -658,8 +658,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix) filter.abbrev = -1; if (argc == 2 && !strcmp(argv[1], "-h")) - usage_with_options(builtin_branch_usage, options); - + { + usage_with_options(builtin_branch_usage, options); + } + if (argc == 3 && !strcmp(argv[2], "-")) + { + argv[2] = "@{-1}"; + } git_config(git_branch_config, NULL); track = git_branch_track; -- 2.8.0.rc0 -- To unsubscribe from this list: send the line "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] Allow "-" as a short-hand for "@{-1}" for branch deletion
From: Dinesh*** This patch allows the usage of "-" as a short-hand for "@{-1}" in "git branch -d @{-1}". Note : This is a microproject that is part of the Google Summer of Code application process. I am interested in working on the git Beginner mode implementation as part of Google Summer of Code. The mentor details for this particular project are not available on the Ideas page. The mentors are likely on this mailing list, so I request the mentors to drop me a mail so I can get in contact with you to further discuss the git Beginner mode project. *** Dinesh (1): branch.c: Allow "-" as a short-hand for "@{-1}" in "git branch -d @{-1}" builtin/branch.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) -- 2.8.0.rc0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] git reset --hard gives clean working tree
On 11.02.16 19:49, Junio C Hamano wrote: > tbo...@web.de writes: > >> From: Torsten Bögershausen>> >> We define the working tree file is clean if either: >> >> * the result of running convert_to_git() on the working tree >> contents matches what is in the index (because that would mean >> doing another "git add" on the path is a no-op); OR >> >> * the result of running convert_to_working_tree() on the content >> in the index matches what is in the working tree (because that >> would mean doing another "git checkout -f" on the path is a >> no-op). >> >> Add an extra check in ce_compare_data() in read_cache.c, and adjust >> the test cases in t0025: >> When a file has CRLF in the index, and is checked out into the working tree, >> but left unchabged, it is not normalized at the next commit. >> Whenever the file is changed in the working tree, a line is added/deleted >> or dos2unix is run, it may be normalized at the next commit, >> depending on .gitattributes. >> >> This patch is a result of a longer discussion on the mailing list, >> how to fix the flaky t0025. > Currently, the codepaths that want to stop if it would lose > information from the index and/or the working tree for the path run > an equivalent of "diff-files path" to see there is any difference. > This indeed is overly strict for a path that has contents in the > index that wouldn't have been created by "clean" conversion (I am > using this word to mean anything convert_to_git() does, not limited > to the "clean" filter). > > And it is sensible to allow them to proceed if the contents in the > working tree file for the path match what would be created by > "smudge" conversion of the contents in the index. > > But breaking "diff-files" is not the right way to do so. Teaching > "Am I safe to proceed" callers that paths that do not pass > "diff-files" test may still be safe to work on is. > > I did not continue the approach I illustrated because I realized and > finally convinced myself that touching ce_compare_data() is a wrong > solution--it breaks "diff-files". > > Imagine if you have contents in the index that wouldn't have been > left by a "clean" conversion of what is in the working tree. You > then run "git checkout -f". Now the contents in the working tree > will still not convert back to what is in the index with another > "clean" conversion, but it should pass the "Am I safe to proceed" > check, namely, it matches what convert_to_worktree() would give. > > But imagine further what would happen when you add an extra blank > line at the end of the file in the working tree (i.e. "echo >>file") > and then run "diff-files -p". > > The illustration patch I gave broke "diff-files" in such a way that > before such an addition of an extra blank line, it would have said > "No changes". And if you run "diff-files" after adding that extra > blank line, you will see whole bunch of changes, not just the extra > blank line at the end. > > This is sufficient to convince me that the approach is broken. [] Would something like this make sense? (The main part is in diff.c, the rest needs some polishing) commit e494c31fd2f0f8a638ff14d1b8ae3f3a6d56a107 Author: Torsten Bögershausen Date: Sat Mar 5 07:51:08 2016 +0100 Text files needs to be normalized before diffing Whenever a text file is stored with CRLF in the index, Git changes CRLF into LF at the next commit. (text file means the attribute "text" or "crlf" of "eol" is set). "git diff" reports that all lines with CRLF in the index as changed to LF. After cloning a repo, the work tree is not considered clean by Git, even if the user didn't change a single line. Avoid to report lines as changed by converting the content from the index into LF before running diff. diff --git a/convert.c b/convert.c index f524b8d..af8248d 100644 --- a/convert.c +++ b/convert.c @@ -231,9 +231,9 @@ static int has_cr_in_index(const char *path) return has_cr; } -static int crlf_to_git(const char *path, const char *src, size_t len, - struct strbuf *buf, - enum crlf_action crlf_action, enum safe_crlf checksafe) +static int crlf_to_git_internal(const char *path, const char *src, size_t len, +struct strbuf *buf, +enum crlf_action crlf_action, enum safe_crlf checksafe) { struct text_stat stats; char *dst; @@ -852,7 +852,17 @@ const char *get_convert_attr_ascii(const char *path) return ""; } -int convert_to_git(const char *path, const char *src, size_t len, +int convert_crlf_to_git(const char *path, const char *src, size_t len, +struct strbuf *buf, +enum safe_crlf checksafe) +{ +struct conv_attrs ca; +convert_attrs(, path); +return crlf_to_git_internal(path, src, len, buf, +ca.crlf_action, checksafe); + +} +int convert_to_git(const char *path, const char *src, size_t len,
Re: [PATCH 1/2] mergetool: honor tempfile configuration when resolving delete conflicts
David Aguilarwrites: > Teach resolve_deleted_merge() to honor the mergetool.keepBackup and > mergetool.keepTemporaries configuration knobs. > > This ensures that the worktree is kept pristine when resolving deletion > conflicts with the variables both set to false. > > Signed-off-by: David Aguilar > --- > git-mergetool.sh | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/git-mergetool.sh b/git-mergetool.sh > index 9f77e3a..615265d 100755 > --- a/git-mergetool.sh > +++ b/git-mergetool.sh > @@ -126,7 +126,12 @@ resolve_deleted_merge () { > case "$ans" in > [mMcC]*) > git add -- "$MERGED" > - cleanup_temp_files --save-backup > + if "$merge_keep_backup" = "true" The command run as the "if" condition is probably "test", like in the other hunk? > + then > + cleanup_temp_files --save-backup > + else > + cleanup_temp_files > + fi > return 0 > ;; > [dD]*) > @@ -135,6 +140,10 @@ resolve_deleted_merge () { > return 0 > ;; > [aA]*) > + if test "$merge_keep_temporaries" = "false" > + then > + cleanup_temp_files > + fi > return 1 > ;; > esac -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] mergetool: support delete/delete conflicts
If two branches each move a file into different directories then mergetool will fail because it assumes that the file being merged, and its parent directory, are present in the worktree. Create the merge file's parent directory to allow using the deleted base version of the file for merge resolution when encountering a delete/delete conflict. The end result is that a delete/delete conflict is presented for the user to resolve. Reported-by: Joe EinertsonSigned-off-by: David Aguilar --- git-mergetool.sh | 14 +++--- t/t7610-mergetool.sh | 30 ++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 615265d..000f167 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -291,8 +291,14 @@ merge_file () { return fi - mv -- "$MERGED" "$BACKUP" - cp -- "$BACKUP" "$MERGED" + if test -f "$MERGED" + then + mv -- "$MERGED" "$BACKUP" + cp -- "$BACKUP" "$MERGED" + fi + # Create a parent directory to handle delete/delete conflicts + # where the base's directory no longer exists. + mkdir -p "$(dirname "$MERGED")" checkout_staged_file 1 "$MERGED" "$BASE" checkout_staged_file 2 "$MERGED" "$LOCAL" @@ -304,7 +310,9 @@ merge_file () { describe_file "$local_mode" "local" "$LOCAL" describe_file "$remote_mode" "remote" "$REMOTE" resolve_deleted_merge - return + status=$? + rmdir -p "$(dirname "$MERGED")" 2>/dev/null + return $status fi if is_symlink "$local_mode" || is_symlink "$remote_mode" diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh index 6f12b23..f1668be 100755 --- a/t/t7610-mergetool.sh +++ b/t/t7610-mergetool.sh @@ -243,6 +243,36 @@ test_expect_success 'mergetool takes partial path' ' git reset --hard ' +test_expect_success 'mergetool delete/delete conflict' ' + git checkout -b delete-base branch1 && + mkdir -p a/a && + (echo one; echo two; echo 3; echo 4) >a/a/file.txt && + git add a/a/file.txt && + git commit -m"base file" && + git checkout -b move-to-b delete-base && + mkdir -p b/b && + git mv a/a/file.txt b/b/file.txt && + (echo one; echo two; echo 4) >b/b/file.txt && + git commit -a -m"move to b" && + git checkout -b move-to-c delete-base && + mkdir -p c/c && + git mv a/a/file.txt c/c/file.txt && + (echo one; echo two; echo 3) >c/c/file.txt && + git commit -a -m"move to c" && + ! git merge move-to-b && + echo d | git mergetool a/a/file.txt && + ! test -f a/a/file.txt && + git reset --hard HEAD && + ! git merge move-to-b && + echo m | git mergetool a/a/file.txt && + test -f b/b/file.txt && + git reset --hard HEAD && + ! git merge move-to-b && + ! echo a | git mergetool a/a/file.txt && + ! test -f a/a/file.txt && + git reset --hard HEAD +' + test_expect_success 'deleted vs modified submodule' ' git checkout -b test6 branch1 && git submodule update -N && -- 2.8.0.rc1.2.g28ba210 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mergetool: honor tempfile configuration when resolving delete conflicts
Teach resolve_deleted_merge() to honor the mergetool.keepBackup and mergetool.keepTemporaries configuration knobs. This ensures that the worktree is kept pristine when resolving deletion conflicts with the variables both set to false. Signed-off-by: David Aguilar--- git-mergetool.sh | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/git-mergetool.sh b/git-mergetool.sh index 9f77e3a..615265d 100755 --- a/git-mergetool.sh +++ b/git-mergetool.sh @@ -126,7 +126,12 @@ resolve_deleted_merge () { case "$ans" in [mMcC]*) git add -- "$MERGED" - cleanup_temp_files --save-backup + if "$merge_keep_backup" = "true" + then + cleanup_temp_files --save-backup + else + cleanup_temp_files + fi return 0 ;; [dD]*) @@ -135,6 +140,10 @@ resolve_deleted_merge () { return 0 ;; [aA]*) + if test "$merge_keep_temporaries" = "false" + then + cleanup_temp_files + fi return 1 ;; esac -- 2.8.0.rc1.2.g28ba210 -- To unsubscribe from this list: send the line "unsubscribe 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 sends first an empty authorization header
Hey Bryan, Yes that will happen he will get a prompet for username/password but he already provided them in the URL and it worked before. He could clone. I think this is a little bit confusing. My problem is that the tool I'm trying to build is trying to provide the username used to log in via an environment variable. And due to the fact that the anonymous login is always the first to be tried, even if the user provides an username/password on the URL I'm not able to retrieve it. On Sat, Mar 5, 2016 at 11:20 AM, Bryan Turnerwrote: > On Fri, Mar 4, 2016 at 9:51 PM, Guilherme wrote: >> Hi, >> >> When doing basic authentication using git clone by passing the >> username and password in the url git clone will first send a GET >> request without the authorization header set. >> >> Am i seeing this right? > > I believe this is an intentional behavior in either cURL or how Git > uses it. Credentials aren't sent until the server returns a challenge > for them, even if you include them in your clone URL or elsewhere. So > yes, you're seeing it right. > >> >> This means that if the counterpart allows anonymous cloning but not >> pushing and the user provided a wrong usernam/password, it has two >> options: > > I'm not sure why this would be true. If the remote server allows > anonymous clone/fetch, then you never get prompted for credentials > and, even if they're supplied, they're never sent to the remote > server. If you then try to push, if the server is working correctly it > should detect that anonymous users can't push and it should return a > 401 with a WWW-Authenticate header. When the client receives the 401, > it should send the credentials it has (or prompt for them if it > doesn't have them) and the push should work without issue. > > Perhaps there's an issue with how your server is setup to handle permissions? > >> >> 1. Allow the access and leave the user to figure out why he is not able to >> push. >> >> 2. Reply by setting the WWW-Authentication header and see if a >> password/username is provided. This has the downside that if no >> username and password is provided the user will still get a login >> prompt for password and username. Upon entering twice nothing he will >> still be able to clone. This can be confusing. >> >> Can this behaviour of git clone (and I guess all the other parts that >> do basic auth) be changed to provide the authentication header right >> on the first request? Or am I doing/interpreting it wrong? >> >> Thank you. >> -- >> To unsubscribe from this list: send the line "unsubscribe git" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe 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 sends first an empty authorization header
On Fri, Mar 4, 2016 at 9:51 PM, Guilhermewrote: > Hi, > > When doing basic authentication using git clone by passing the > username and password in the url git clone will first send a GET > request without the authorization header set. > > Am i seeing this right? I believe this is an intentional behavior in either cURL or how Git uses it. Credentials aren't sent until the server returns a challenge for them, even if you include them in your clone URL or elsewhere. So yes, you're seeing it right. > > This means that if the counterpart allows anonymous cloning but not > pushing and the user provided a wrong usernam/password, it has two > options: I'm not sure why this would be true. If the remote server allows anonymous clone/fetch, then you never get prompted for credentials and, even if they're supplied, they're never sent to the remote server. If you then try to push, if the server is working correctly it should detect that anonymous users can't push and it should return a 401 with a WWW-Authenticate header. When the client receives the 401, it should send the credentials it has (or prompt for them if it doesn't have them) and the push should work without issue. Perhaps there's an issue with how your server is setup to handle permissions? > > 1. Allow the access and leave the user to figure out why he is not able to > push. > > 2. Reply by setting the WWW-Authentication header and see if a > password/username is provided. This has the downside that if no > username and password is provided the user will still get a login > prompt for password and username. Upon entering twice nothing he will > still be able to clone. This can be confusing. > > Can this behaviour of git clone (and I guess all the other parts that > do basic auth) be changed to provide the authentication header right > on the first request? Or am I doing/interpreting it wrong? > > Thank you. > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Git clone sends first an empty authorization header
Hi, When doing basic authentication using git clone by passing the username and password in the url git clone will first send a GET request without the authorization header set. Am i seeing this right? This means that if the counterpart allows anonymous cloning but not pushing and the user provided a wrong usernam/password, it has two options: 1. Allow the access and leave the user to figure out why he is not able to push. 2. Reply by setting the WWW-Authentication header and see if a password/username is provided. This has the downside that if no username and password is provided the user will still get a login prompt for password and username. Upon entering twice nothing he will still be able to clone. This can be confusing. Can this behaviour of git clone (and I guess all the other parts that do basic auth) be changed to provide the authentication header right on the first request? Or am I doing/interpreting it wrong? Thank you. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] l10n: TEAMS: update Ralf Thielow's email address
That's fine. Already pulled to git-po. 2016-03-04 4:48 GMT+08:00 Ralf Thielow: > Signed-off-by: Ralf Thielow > --- > po/TEAMS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/po/TEAMS b/po/TEAMS > index df12b58..56274ad 100644 > --- a/po/TEAMS > +++ b/po/TEAMS > @@ -11,7 +11,7 @@ Leader: Alex Henrie > > Language: de (German) > Repository:https://github.com/ralfth/git-po-de > -Leader:Ralf Thielow > +Leader:Ralf Thielow > Members: Thomas Rast > Jan Krüger > Christian Stimming > -- > 2.8.0.rc0.159.g391b917 > -- 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: Change in .gitignore handling: intended or bug?
Charles Strahanwrites: > ...as Duy suggests, I think the new behavior makes a bit > more sense. After re-reading your original example, I am inclined to agree with this. > Either way, of course, I'd like for it to not change back and > forth between releases :). > > Perhaps just an announcement of the new behavior would suffice - > 2.7.0 has been out for a while anyway. If people were going to > complain, I figure they would have done so by now. Yup, I think a documentation update to clarify how positive and negative ignore patterns interact with each other may be necessary, with some examples. Care to work on a patch? 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] xdiff/xprepare: fix a memory leak
On 04/03/16 23:50, Junio C Hamano wrote: > Ramsay Joneswrites: > >> The xdl_prepare_env() function may initialise an xdlclassifier_t >> data structure via xdl_init_classifier(), which allocates memory >> to several fields, for example 'rchash', 'rcrecs' and 'ncha'. >> If this function later exits due to the failure of xdl_optimize_ctxs(), >> then this xdlclassifier_t structure, and the memory allocated to it, >> is not cleaned up. >> >> In order to fix the memory leak, insert a call to xdl_free_classifier() >> before returning. >> >> Signed-off-by: Ramsay Jones >> --- >> xdiff/xprepare.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c >> index 5ffcf99..13b55ab 100644 >> --- a/xdiff/xprepare.c >> +++ b/xdiff/xprepare.c >> @@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, >> xpparam_t const *xpp, >> >> xdl_free_ctx(>xdf2); >> xdl_free_ctx(>xdf1); >> +xdl_free_classifier(); >> return -1; >> } > > This looks obviously correct from the pattern of prepare's and > free's in the code that this part follows. This potential leak has > been that way since 3443546f (Use a *real* built-in diff generator, > 2006-03-24), i.e. the very beginning. > > I find it somewhat strange that the call to xdl_free_classifier() at > the end of this function is made conditional to XDF_HISTOGRAM_DIFF, > though. I can half-buy the argument "that is because we do not call > init-classifier for XDF_HISTOGRAM_DIFF", but in the error path we > call free-classifier unconditionally, so the code clearly knows that > it is safe to call free-classifier on a classifier that is cleared > with the initial memset(, 0, sizeof cf). Indeed, this is actually why I noticed that XDF_DIFF_ALG() wasn't used. Rather than doing patch #1, I did consider making this call unconditional. I can't remember why I didn't. (Hmm, perhaps I just chickened out! ;-)) ATB, Ramsay Jones -- To unsubscribe from this list: send the line "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] fetch-pack: fix object_id of exact sha1
Commit 58f2ed0 (remote-curl: pass ref SHA-1 to fetch-pack as well, 2013-12-05) added support for specifying a SHA-1 as well as a ref name. Add support for specifying just a SHA-1 and set the ref name to the same value in this case. Signed-off-by: Gabriel Souza Franco--- Documentation/git-fetch-pack.txt | 4 builtin/fetch-pack.c | 16 +--- t/t5500-fetch-pack.sh| 14 ++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/Documentation/git-fetch-pack.txt b/Documentation/git-fetch-pack.txt index 8680f45..239623c 100644 --- a/Documentation/git-fetch-pack.txt +++ b/Documentation/git-fetch-pack.txt @@ -104,6 +104,10 @@ be in a separate packet, and the list must end with a flush packet. The remote heads to update from. This is relative to $GIT_DIR (e.g. "HEAD", "refs/heads/master"). When unspecified, update from all heads the remote side has. ++ +If the remote has enabled the options `uploadpack.allowTipSHA1InWant` or +`uploadpack.allowReachableSHA1InWant`, they may alternatively be 40-hex +sha1s present on the remote. SEE ALSO diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 79a611f..50c9901 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -16,10 +16,20 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc, struct ref *ref; struct object_id oid; - if (!get_oid_hex(name, ) && name[GIT_SHA1_HEXSZ] == ' ') - name += GIT_SHA1_HEXSZ + 1; - else + if (!get_oid_hex(name, )) { + if (name[GIT_SHA1_HEXSZ] == ' ') { + /* , find refname */ + name += GIT_SHA1_HEXSZ + 1; + } else if (name[GIT_SHA1_HEXSZ] == '\0') { + /* , leave sha1 as name */ + } else { + /* , clear cruft from oid */ + oidclr(); + } + } else { + /* , clear cruft from get_oid_hex */ oidclr(); + } ref = alloc_ref(name); oidcpy(>old_oid, ); diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index e5f83bf..9b9bec4 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -531,6 +531,20 @@ test_expect_success 'shallow fetch with tags does not break the repository' ' git fsck ) ' + +test_expect_success 'fetch-pack can fetch a raw sha1' ' + git init hidden && + ( + cd hidden && + test_commit 1 && + test_commit 2 && + git update-ref refs/hidden/one HEAD^ && + git config transfer.hiderefs refs/hidden && + git config uploadpack.allowtipsha1inwant true + ) && + git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one) +' + check_prot_path () { cat >expected <<-EOF && Diag: url=$1 -- 2.7.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Change in .gitignore handling: intended or bug?
The fix on my side was quite easy (and my .gitignore is probably a _lot_ hairier than most), and as Duy suggests, I think the new behavior makes a bit more sense. Personally, I would be pleased with keeping the new behavior, and chalking it up to an unintentional bug fix (the best kind). Either way, of course, I'd like for it to not change back and forth between releases :). Perhaps just an announcement of the new behavior would suffice - 2.7.0 has been out for a while anyway. If people were going to complain, I figure they would have done so by now. -Charles On Fri, Mar 4, 2016, at 07:50 PM, Duy Nguyen wrote: > typo fixes > > On Sat, Mar 5, 2016 at 7:43 AM, Duy Nguyenwrote: > > On Sat, Mar 5, 2016 at 12:28 AM, Junio C Hamano wrote: > >> Duy Nguyen writes: > >> > >>> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt wrote: > Verified that it's different in 2.7.0, but 2.7.2 gives expected output. > >>> > >>> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two > >>> other regression reports before this one. I guess it's all good then > >>> (except for the people still on 2.7.0) > >> > >> Are we good at 2.8.0-rc0, too? Somehow I had an impression that we > >> queued "another attempt to do it differently" or something. > >> > >> ... goes and looks ... > >> > >> $ rungit maint status -suall > >> ?? baz/quux/corge/wibble.txt > >> ?? baz/quux/grault.txt > >> ?? foo/bar.txt > >> $ rungit master status -suall > >> ?? baz/quux/corge/wibble.txt > >> ?? baz/quux/grault.txt > >> ?? baz/waldo.txt > >> ?? foo/bar.txt > >> ?? foo/garply.txt > > > > If you swap a60ea8f and bac65a2 so that we can have tracing even > > without the problematic commit a60ea8f (dir.c: fix match_pathname() - > > 2016-02-15). Without a60ea8f I got > > > > GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null > > |grep waldo > > 07:25:05.639445 dir.c:952 exclude: baz/waldo.txt vs * at > > line 1 => yes > > > > (meaning, baz/waldo.txt matches "*" rule and is decided to be excluded) > > > > with a60ea8f > > > >> /tmp/abc $ GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 > >> >/dev/null |grep waldo > > 07:25:24.425125 dir.c:952 exclude: baz/waldo.txt vs /baz > > at line 4 => no > > > > the decision is not taken earlier from line "!/baz" and it's decided > > s/not taken/taken/ > > > to be re-included. Which I would argue is the correct thing because > > you ask to re-include the whole directory "baz". It should behave this > > way because exclude rules without '!' behave this way. > > > > Because positive any negative rules can be nested, by adding a rule to > > s/any/and/ > > > reinclude what's inside baz.. > > > > * > > !/foo > > !/foo/bar.txt > > !/baz > > /baz/* > > !/baz/quux > > !/baz/quux/**/* > > > > you'll get baz/waldo.txt excluded. > > > > Yes it's a behavior change. I think the old behavior is unintended. > > But it's probably out there long enough that many .gitignore files may > > rely on it and by making it right, I break them. Whether to revert the > > series is up to you. Let me know if I should send the revert patch. > > -- > > Duy > > > > -- > 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: Change in .gitignore handling: intended or bug?
typo fixes On Sat, Mar 5, 2016 at 7:43 AM, Duy Nguyenwrote: > On Sat, Mar 5, 2016 at 12:28 AM, Junio C Hamano wrote: >> Duy Nguyen writes: >> >>> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt wrote: Verified that it's different in 2.7.0, but 2.7.2 gives expected output. >>> >>> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two >>> other regression reports before this one. I guess it's all good then >>> (except for the people still on 2.7.0) >> >> Are we good at 2.8.0-rc0, too? Somehow I had an impression that we >> queued "another attempt to do it differently" or something. >> >> ... goes and looks ... >> >> $ rungit maint status -suall >> ?? baz/quux/corge/wibble.txt >> ?? baz/quux/grault.txt >> ?? foo/bar.txt >> $ rungit master status -suall >> ?? baz/quux/corge/wibble.txt >> ?? baz/quux/grault.txt >> ?? baz/waldo.txt >> ?? foo/bar.txt >> ?? foo/garply.txt > > If you swap a60ea8f and bac65a2 so that we can have tracing even > without the problematic commit a60ea8f (dir.c: fix match_pathname() - > 2016-02-15). Without a60ea8f I got > > GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null > |grep waldo > 07:25:05.639445 dir.c:952 exclude: baz/waldo.txt vs * at > line 1 => yes > > (meaning, baz/waldo.txt matches "*" rule and is decided to be excluded) > > with a60ea8f > >> /tmp/abc $ GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >> >/dev/null |grep waldo > 07:25:24.425125 dir.c:952 exclude: baz/waldo.txt vs /baz > at line 4 => no > > the decision is not taken earlier from line "!/baz" and it's decided s/not taken/taken/ > to be re-included. Which I would argue is the correct thing because > you ask to re-include the whole directory "baz". It should behave this > way because exclude rules without '!' behave this way. > > Because positive any negative rules can be nested, by adding a rule to s/any/and/ > reinclude what's inside baz.. > > * > !/foo > !/foo/bar.txt > !/baz > /baz/* > !/baz/quux > !/baz/quux/**/* > > you'll get baz/waldo.txt excluded. > > Yes it's a behavior change. I think the old behavior is unintended. > But it's probably out there long enough that many .gitignore files may > rely on it and by making it right, I break them. Whether to revert the > series is up to you. Let me know if I should send the revert patch. > -- > Duy -- 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: Change in .gitignore handling: intended or bug?
On Sat, Mar 5, 2016 at 12:28 AM, Junio C Hamanowrote: > Duy Nguyen writes: > >> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt wrote: >>> Verified that it's different in 2.7.0, but 2.7.2 gives expected output. >> >> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two >> other regression reports before this one. I guess it's all good then >> (except for the people still on 2.7.0) > > Are we good at 2.8.0-rc0, too? Somehow I had an impression that we > queued "another attempt to do it differently" or something. > > ... goes and looks ... > > $ rungit maint status -suall > ?? baz/quux/corge/wibble.txt > ?? baz/quux/grault.txt > ?? foo/bar.txt > $ rungit master status -suall > ?? baz/quux/corge/wibble.txt > ?? baz/quux/grault.txt > ?? baz/waldo.txt > ?? foo/bar.txt > ?? foo/garply.txt If you swap a60ea8f and bac65a2 so that we can have tracing even without the problematic commit a60ea8f (dir.c: fix match_pathname() - 2016-02-15). Without a60ea8f I got GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null |grep waldo 07:25:05.639445 dir.c:952 exclude: baz/waldo.txt vs * at line 1 => yes (meaning, baz/waldo.txt matches "*" rule and is decided to be excluded) with a60ea8f > /tmp/abc $ GIT_TRACE_EXCLUDE=1 ~/w/git/temp/git status -suall 2>&1 >/dev/null > |grep waldo 07:25:24.425125 dir.c:952 exclude: baz/waldo.txt vs /baz at line 4 => no the decision is not taken earlier from line "!/baz" and it's decided to be re-included. Which I would argue is the correct thing because you ask to re-include the whole directory "baz". It should behave this way because exclude rules without '!' behave this way. Because positive any negative rules can be nested, by adding a rule to reinclude what's inside baz.. * !/foo !/foo/bar.txt !/baz /baz/* !/baz/quux !/baz/quux/**/* you'll get baz/waldo.txt excluded. Yes it's a behavior change. I think the old behavior is unintended. But it's probably out there long enough that many .gitignore files may rely on it and by making it right, I break them. Whether to revert the series is up to you. Let me know if I should send the revert patch. -- 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: Diff filename has trailing tab if filename contains space
乙酸鋰writes: > Hi, > > Using git 2.7.1 > > Diff filename has trailing tab if filename contains space Thanks; that is very much deliberate and has been with us forever. commit 1a9eb3b9d50367bee8fe85022684d812816fe531 Author: Junio C Hamano Date: Fri Sep 22 16:17:58 2006 -0700 git-diff/git-apply: make diff output a bit friendlier to GNU patch (part 2) Somebody was wondering on #git channel why a git generated diff does not apply with GNU patch when the filename contains a SP. It is because GNU patch expects to find TAB (and trailing timestamp) on ---/+++ (old_name and new_name) lines after the filenames. The "diff --git" output format was carefully designed to be compatible with GNU patch where it can, but whitespace characters were always a pain. This adds an extra TAB (but not trailing timestamp) to old_name and new_name lines of git-diff output when the filename has a SP in it. An earlier patch updated git-apply to prepare for this. Signed-off-by: Junio C Hamano -- To unsubscribe from this list: send the line "unsubscribe 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] xdiff/xprepare: fix a memory leak
Ramsay Joneswrites: > The xdl_prepare_env() function may initialise an xdlclassifier_t > data structure via xdl_init_classifier(), which allocates memory > to several fields, for example 'rchash', 'rcrecs' and 'ncha'. > If this function later exits due to the failure of xdl_optimize_ctxs(), > then this xdlclassifier_t structure, and the memory allocated to it, > is not cleaned up. > > In order to fix the memory leak, insert a call to xdl_free_classifier() > before returning. > > Signed-off-by: Ramsay Jones > --- > xdiff/xprepare.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c > index 5ffcf99..13b55ab 100644 > --- a/xdiff/xprepare.c > +++ b/xdiff/xprepare.c > @@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, > xpparam_t const *xpp, > > xdl_free_ctx(>xdf2); > xdl_free_ctx(>xdf1); > + xdl_free_classifier(); > return -1; > } This looks obviously correct from the pattern of prepare's and free's in the code that this part follows. This potential leak has been that way since 3443546f (Use a *real* built-in diff generator, 2006-03-24), i.e. the very beginning. I find it somewhat strange that the call to xdl_free_classifier() at the end of this function is made conditional to XDF_HISTOGRAM_DIFF, though. I can half-buy the argument "that is because we do not call init-classifier for XDF_HISTOGRAM_DIFF", but in the error path we call free-classifier unconditionally, so the code clearly knows that it is safe to call free-classifier on a classifier that is cleared with the initial memset(, 0, sizeof cf). I think the latter funkiness came from 9f37c275. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Diff filename has trailing tab if filename contains space
Hi, Using git 2.7.1 Diff filename has trailing tab if filename contains space Please run below shell script and look at the output diff file 1.diff There is trailing tab chars after these lines: --- a/8 1/8.txt +++ b/8 1/8.txt b/9 86 +++ b/9 86 #!/bin/sh set -e git init echo a >> "normal" git add "normal" echo y >> "9 86" git add "9 86" # file name has space mkdir x echo d >> "x/normal" git add "x/normal" mkdir "8 1" echo u >> "8 1/8.txt" # directory name has space echo k >> "8 1/8.txt" git add "8 1/8.txt" git commit -m "Initial commit" echo b >> "normal" git add "normal" echo c >> "9 86" git add "9 86" echo e >> "x/normal" git add "x/normal" echo h >> "8 1/8.txt" git add "8 1/8.txt" git commit -m "Edit files" git diff master~1 > 1.diff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] xdiff/xprepare: fix a memory leak
The xdl_prepare_env() function may initialise an xdlclassifier_t data structure via xdl_init_classifier(), which allocates memory to several fields, for example 'rchash', 'rcrecs' and 'ncha'. If this function later exits due to the failure of xdl_optimize_ctxs(), then this xdlclassifier_t structure, and the memory allocated to it, is not cleaned up. In order to fix the memory leak, insert a call to xdl_free_classifier() before returning. Signed-off-by: Ramsay Jones--- xdiff/xprepare.c | 1 + 1 file changed, 1 insertion(+) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 5ffcf99..13b55ab 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdl_free_ctx(>xdf2); xdl_free_ctx(>xdf1); + xdl_free_classifier(); return -1; } -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] xdiff/xprepare: use the XDF_DIFF_ALG() macro to access flag bits
Commit 307ab20b3 ("xdiff: PATIENCE/HISTOGRAM are not independent option bits", 19-02-2012) introduced the XDF_DIFF_ALG() macro to access the flag bits used to represent the diff algorithm requested. In addition, code which had used explicit manipulation of the flag bits was changed to use the macros. However, one example of direct manipulation remains. Update this code to use the XDF_DIFF_ALG() macro. Signed-off-by: Ramsay Jones--- xdiff/xprepare.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c index 63a22c6..5ffcf99 100644 --- a/xdiff/xprepare.c +++ b/xdiff/xprepare.c @@ -304,7 +304,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, return -1; } - if (!(xpp->flags & XDF_HISTOGRAM_DIFF)) + if (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) xdl_free_classifier(); return 0; -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] diff: fix a memory leak
A recent memory leak patch from Patrick, commit 4867f1184 ("xdiff/xmerge: fix memory leak in xdl_merge", 23-02-2016), reminded me that I had a similar patch lying around. After checking that it wasn't the same one, I dusted it off, and split it into these two patches. One of the reasons for not sending it earlier was that it is not very likely to happen (without very large files), and I had to use the debugger to confirm the leak and the fix. [If you want to do so yourself, then I suggest that you use two files that actually have changes and do 'run --no-pager diff --no-index file-1 file-2'. set breakpoints on xdl_prepare_env, xdl_init_classifier, xdl_free_classifier, xdl_optimize_ctxs and xdl_cleanup_records. You have to force xdl_cleanup_records to return -1 to simulate an OOM. Since xdl_free_classifier does not set the allocated fields to NULL, you have to make sure that you do/don't hit that breakpoint before the return at line #304/305.] Ramsay Jones (2): xdiff/xprepare: use the XDF_DIFF_ALG() macro to access flag bits xdiff/xprepare: fix a memory leak xdiff/xprepare.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.7.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v2.8.0-rc1
A release candidate Git v2.8.0-rc1 is now available for testing at the usual places. It is comprised of 453 non-merge commits since v2.7.0, contributed by 59 people, 19 of which are new faces. There still is a known regression around "git status" (and "ls-files -o") relative to v2.7.2, which we may end up resolving by reverting a topic before v2.8.0 final. We'll see how it goes. The tarballs are found at: https://www.kernel.org/pub/software/scm/git/testing/ The following public repositories all have a copy of the 'v2.8.0-rc1' tag and the 'master' branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git New contributors whose contributions weren't in v2.7.0 are as follows. Welcome to the Git development community! 마누엘, Andrew Wheeler, Changwoo Ryu, Christoph Egger, Dan Aloni, Dave Ware, David A. Wheeler, Dickson Wong, Felipe Gonçalves Assis, GyuYong Jung, Jon Griffiths, Kazutoshi Satoda, Lars Vogel, Martin Amdisen, Matthew Kraai, Paul Wagland, Rob Mayoff, Romain Picard, and Victor Leschuk. Returning contributors who helped this release are as follows. Thanks for your continued support. Alexander Kuleshov, Alex Henrie, brian m. carlson, Christian Couder, David A. Greene, David Turner, Dennis Kaarsemaker, Edmundo Carmona Antoranz, Elia Pinto, Eric Wong, Jacob Keller, Jeff King, Johannes Schindelin, Johannes Sixt, John Keeping, Jonathan Nieder, Junio C Hamano, Karsten Blees, Karthik Nayak, Knut Franke, Lars Schneider, Matthieu Moy, Matt McCutchen, Michael J Gruber, Mike Hommey, Nguyễn Thái Ngọc Duy, Øyvind A. Holm, Patrick Steinhardt, Pat Thoyts, Sebastian Schuberth, Shawn O. Pearce, Stefan Beller, Stephen P. Smith, SZEDER Gábor, Thomas Ackermann, Thomas Braun, Thomas Gummerer, Tobias Klauser, Torsten Bögershausen, and Will Palmer. Git 2.8 Release Notes (draft) = Backward compatibility note --- The rsync:// transport has been removed. Updates since v2.7 -- UI, Workflows & Features * It turns out "git clone" over rsync transport has been broken when the source repository has packed references for a long time, and nobody noticed nor complained about it. * "branch --delete" has "branch -d" but "push --delete" does not. * "git blame" learned to produce the progress eye-candy when it takes too much time before emitting the first line of the result. * "git grep" can now be configured (or told from the command line) how many threads to use when searching in the working tree files. * Some "git notes" operations, e.g. "git log --notes=", should be able to read notes from any tree-ish that is shaped like a notes tree, but the notes infrastructure required that the argument must be a ref under refs/notes/. Loosen it to require a valid ref only when the operation would update the notes (in which case we must have a place to store the updated notes tree, iow, a ref). * "git grep" by default does not fall back to its "--no-index" behaviour outside a directory under Git's control (otherwise the user may by mistake end up running a huge recursive search); with a new configuration (set in $HOME/.gitconfig--by definition this cannot be set in the config file per project), this safety can be disabled. * "git pull --rebase" has been extended to allow invoking "rebase -i". * "git p4" learned to cope with the type of a file getting changed. * "git format-patch" learned to notice format.outputDirectory configuration variable. This allows "-o " option to be omitted on the command line if you always use the same directory in your workflow. * "interpret-trailers" has been taught to optionally update a file in place, instead of always writing the result to the standard output. * Many commands that read files that are expected to contain text that is generated (or can be edited) by the end user to control their behaviour (e.g. "git grep -f ") have been updated to be more tolerant to lines that are terminated with CRLF (they used to treat such a line to contain payload that ends with CR, which is usually not what the users expect). * "git notes merge" used to limit the source of the merged notes tree to somewhere under refs/notes/ hierarchy, which was too limiting when inventing a workflow to exchange notes with remote repositories using remote-tracking notes trees (located in e.g. refs/remote-notes/ or somesuch). * "git ls-files" learned a new "--eol" option to help diagnose end-of-line problems. * "ls-remote" learned an option to show which branch the remote repository advertises as its primary by pointing
What's cooking in git.git (Mar 2016, #02; Fri, 4)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. v2.8-rc1 has been tagged. There still is a known regression around "git status" (and "ls-files -o") relative to 2.7.2, which we may end up resolving by reverting a topic by 2.8 final. We'll see. Again, the topics that have not been cooked sufficiently in 'next' at this point will not be considered for 2.8 final, even though I might later succumb to the temptation to pick up ones that look trivially correct. Those who have their topics merged to 'master' since v2.7 are expected to focus on responding to regressions and remaining bugs in their topics in 'master', and strongly encouraged to actively hunt for regressions and remaining bugs there, not in a random shiny new feature, during the pre-release period. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * jk/pack-idx-corruption-safety (2016-02-27) 4 commits (merged to 'next' on 2016-03-01 at 49e08d3) + sha1_file.c: mark strings for translation (merged to 'next' on 2016-02-26 at ef0d6de) + use_pack: handle signed off_t overflow + nth_packed_object_offset: bounds-check extended offset + t5313: test bounds-checks of corrupted/malicious pack/idx files The code to read the pack data using the offsets stored in the pack idx file has been made more carefully check the validity of the data in the idx. * jk/tighten-alloc (2016-02-29) 1 commit (merged to 'next' on 2016-03-01 at f4df936) + compat/mingw: brown paper bag fix for 50a6c8e * js/pthread-exit-emu-windows (2016-03-02) 1 commit (merged to 'next' on 2016-03-02 at 391b917) + Mark win32's pthread_exit() as NORETURN * mg/httpd-tests-update-for-apache-2.4 (2016-02-25) 1 commit (merged to 'next' on 2016-03-01 at d2f7e8c) + t/lib-httpd: load mod_unixd (this branch is used by jk/submodule-c-credential.) The way the test scripts configure the Apache web server has been updated to work also for Apache 2.4 running on RedHat derived distros. * nd/clear-gitenv-upon-use-of-alias (2016-03-03) 1 commit (merged to 'next' on 2016-03-03 at 1c1c50f) + t0001: fix GIT_* environment variable check under --valgrind Hotfix for a test breakage made between 2.7 and 'master'. * nd/i18n-2.8.0 (2016-02-29) 4 commits (merged to 'next' on 2016-03-01 at cdf4675) + trailer.c: mark strings for translation + ref-filter.c: mark strings for translation + builtin/clone.c: mark strings for translation + builtin/checkout.c: mark strings for translation * sb/submodule-parallel-fetch (2016-03-01) 1 commit (merged to 'next' on 2016-03-01 at b47ab6e) + run-command: do not pass child process data into callbacks (this branch is used by sb/submodule-init and sb/submodule-parallel-update.) Simplify the two callback functions that are triggered when the child process terminates to avoid misuse of the child-process structure that has already been cleaned up. * tb/avoid-gcc-on-darwin-10-6 (2016-02-28) 1 commit (merged to 'next' on 2016-03-01 at e8dd08a) + config.mak.uname: use clang for Mac OS X 10.6 Out of maintenance gcc on OSX 10.6 fails to compile the code in 'master'; work it around by using clang by default on the platform. -- [New Topics] * sb/rebase-summary (2016-03-02) 1 commit (merged to 'next' on 2016-03-04 at d40714d) + Documentation: reword rebase summary Will merge to 'master' by 2.8-rc2. * jc/index-pack (2016-03-03) 2 commits - index-pack: add a helper function to derive .idx/.keep filename - Merge branch 'jc/maint-index-pack-keep' into jc/index-pack (this branch is used by jc/bundle; uses jc/maint-index-pack-keep; is tangled with jc/index-pack-clone-bundle.) Code clean-up. Will merge to 'next'. * jc/maint-index-pack-keep (2016-03-03) 1 commit (merged to 'next' on 2016-03-04 at bc1d37a) + index-pack: correct --keep[=] (this branch is used by jc/bundle, jc/index-pack and jc/index-pack-clone-bundle.) "git index-pack --keep[=] pack-$name.pack" simply did not work. Will merge to 'master' after 2.8 final. * js/close-packs-before-gc (2016-03-04) 1 commit (merged to 'next' on 2016-03-04 at fe6f6ed) + t5510: do not leave changed cwd A small future-proofing of a test added recently. Will merge to 'master' by 2.8-rc2. -- [Stalled] * ec/annotate-deleted (2015-11-20) 1 commit - annotate: skip checking working tree if a revision is provided Usability fix for annotate-specific " " syntax with deleted files. Waiting for review. * dg/subtree-rebase-test (2016-01-19) 1 commit - contrib/subtree: Add a
git-completion.bash emitting error messages
Hello, I use ./bash 4.3.42 with bundled libreadline and git/contrib/completion/git-completion.bash , as modified by commit 8716bdca268 from 22 Feb 2016. I cd to git.git (non-bare, with working tree) and type git b62c.. ori to force TAB-completion. And then on the shell comes the output (on the same line) "error: key does not contain variable name: alias.b62c.." Pressing again TAB repeats the message. Even if TAB is pressed once, the printed text cannot be deleted with the backspace, so the only way to do something useful is to press ENTER. While the command is indeed nonsense, in my understanding tab-completion shall do nothing, if completion is not possible. In particular it shall not print error messages and even the user types stupid things, he shall not be required to force ENTER to come back to a clear state, where a command can by types. Greetings Dilian -- To unsubscribe from this list: send the line "unsubscribe 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] t9700: fix test for perl older than 5.14
On Fri, Mar 4, 2016 at 1:21 PM, Dennis Kaarsemakerwrote: > On vr, 2016-03-04 at 06:43 -0500, Jeff King wrote: >> On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote: >> >> > On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote: >> > > ? Those are just guesses, but if we are tickling a bug in perl's parser, >> > > this might avoid them. I also wondered when "/r" appeared. It was in >> > > 5.14, so you're presumably good there. The "use" statement at the top of >> > > the script says "5.008", so perhaps we should be writing it out longhand >> > > anyway (that version is "only" 5 years old, so I suspect there are still >> > > systems around with 5.12 or older). >> > >> > Knowing the system Christian is testing on, I think the problem is that >> > the tests are actually being run against perl 5.10, which RHEL 6 ships >> > as system perl. As that's still a supported OS, writing tests in a form >> > compatible with it would be a good thing :) >> >> That would make sense. `perl` in t9700-perl-git.sh (and all of our >> scripts) is actually a shell function: >> >> perl () { >> command "$PERL_PATH" "$@" >> } >> >> to make sure we respect PERL_PATH everywhere. And that defaults in the >> Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH, >> but /usr/bin/perl is the system 5.10. > > Yeah, that's how our systems are set up. Yeah, you are both right. Thanks for debugging this, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Change in .gitignore handling: intended or bug?
Junio C Hamanowrites: > Duy Nguyen writes: > >> On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt wrote: >>> Verified that it's different in 2.7.0, but 2.7.2 gives expected output. >> >> Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two >> other regression reports before this one. I guess it's all good then >> (except for the people still on 2.7.0) > > Are we good at 2.8.0-rc0, too? Somehow I had an impression that we > queued "another attempt to do it differently" or something. > > ... goes and looks ... > > $ rungit maint status -suall > ?? baz/quux/corge/wibble.txt > ?? baz/quux/grault.txt > ?? foo/bar.txt > $ rungit master status -suall > ?? baz/quux/corge/wibble.txt > ?? baz/quux/grault.txt > ?? baz/waldo.txt > ?? foo/bar.txt > ?? foo/garply.txt It seems to bisect down to this one between maint..master: commit a60ea8fb66945a886ea53fd3f41e61cc5fb3201e Author: Nguyễn Thái Ngọc Duy Date: Mon Feb 15 16:03:36 2016 +0700 dir.c: fix match_pathname() Given the pattern "1/2/3/4" and the path "1/2/3/4/f", the pattern prefix is "1/2/3/4". We will compare and remove the prefix from both pattern and path and come to this code /* * If the whole pattern did not have a wildcard, * then our prefix match is all we need; we * do not need to call fnmatch at all. */ if (!patternlen && !namelen) return 1; where patternlen is zero (full pattern consumed) and the remaining path in "name" is "/f". We fail to realize it's matched in this case and fall back to fnmatch(), which also fails to catch it. Fix it. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano But I do not think this change alone is the culprit; the change itself does make sense in the context of the series. At this point, we have two choices. Either we revert the merge of the whole series: commit 5e57f9c3dfe7dd44a1b56bb5b3327d7a1356ec7c Merge: e79112d d589a67 Author: Junio C Hamano Date: Wed Feb 24 13:25:59 2016 -0800 Merge branch 'nd/exclusion-regression-fix' Another try to add support to the ignore mechanism that lets you say "this is excluded" and then later say "oh, no, this part (that is a subset of the previous part) is not excluded". * nd/exclusion-regression-fix: dir.c: don't exclude whole dir prematurely dir.c: support marking some patterns already matched dir.c: support tracing exclude dir.c: fix match_pathname() to go back to the 2.7.2 behaviour, or add a follow-on change to the nd/exclusion-regression-fix topic to fix what it wanted to fix without breaking Charles's use case. I am inclined to go for the former (unless the follow-on fix is really trivial), but I haven't dug into the codebase myself yet, so... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] stripspace: add --line-count flag
Junio C Hamanowrites: > Sidhant Sharma writes: > >> This is my first attempt at the small project listed here: >> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27. > >> Comments? > > Isn't that page somewhat stale? > > http://git.661346.n2.nabble.com/PATCH-0-3-stripspace-Implement-and-use-count-lines-option-tt7641360.html#none And its reroll: http://thread.gmane.org/gmane.comp.version-control.git/279742 The discussion seems to indicate that we weren't in favor of addition of this feature at least back then, e.g. http://thread.gmane.org/gmane.comp.version-control.git/279742/focus=279888 -- To unsubscribe from this list: send the line "unsubscribe 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] stripspace: add --line-count flag
On Saturday 05 March 2016 12:19 AM, Junio C Hamano wrote: > Sidhant Sharmawrites: > >> This is my first attempt at the small project listed here: >> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27. >> Comments? > Isn't that page somewhat stale? > > http://git.661346.n2.nabble.com/PATCH-0-3-stripspace-Implement-and-use-count-lines-option-tt7641360.html#none Oh, I should've checked first. My bad, I was just looking to get familiar with the codebase. Thanks Sidhant Sharma [:tk] -- To unsubscribe from this list: send the line "unsubscribe 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 diff does not precompose unicode file paths (OS X)
On 04/03/16 14:37, Alexander Rinass wrote: > >> On 04 Mar 2016, at 13:16, Torsten Bögershausenwrote: >> >> On 03/04/2016 10:07 AM, Alexander Rinass wrote: [snip] > > Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes > the issue. > > But I had to disable the check (precomposed_unicode != 1) in precompose_argv > to make it work. That’s probably because precompose_argv is usually called > from parse_options and is missing some other call before it? > Yes, you need to place it after the configuration is read, but before calls to diff_no_index() or setup_revisions(). Directly after the call to git_config() should be fine. [But this begs the question about other commands, including plumbing, which don't call parse_options().] Maybe this will work for you (I can't test it, since I don't have any access to a Mac): diff --git a/builtin/diff.c b/builtin/diff.c index 343c6b8..b7a9405 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -320,6 +320,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) gitmodules_config(); init_diff_ui_defaults(); git_config(git_diff_ui_config, NULL); + precompose_argv(argc, argv); init_revisions(, prefix); ATB, Ramsay Jones -- To unsubscribe from this list: send the line "unsubscribe 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] stripspace: add --line-count flag
Sidhant Sharmawrites: > This is my first attempt at the small project listed here: > https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27. > Comments? Isn't that page somewhat stale? http://git.661346.n2.nabble.com/PATCH-0-3-stripspace-Implement-and-use-count-lines-option-tt7641360.html#none -- To unsubscribe from this list: send the line "unsubscribe 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] stripspace: add --line-count flag
> builtin/stripspace.c | 22 +- > git-rebase--interactive.sh | 6 +++--- > 2 files changed, 24 insertions(+), 4 deletions(-) > This is my first attempt at the small project listed here: https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27. With this, --line-count can be used with stripspace, instead of having to pipe its output to `wc -l` in git-rebase--interactive.sh. I went with --line-count and not --count-lines since its short form (-c) is already in use, and I think -l is more apt for this. Comments? Thanks and regards, Sidhant Sharma [:tk] -- To unsubscribe from this list: send the line "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] stripspace: add --line-count flag
When used, this flag outputs number of lines after stripspace has removed trailing whitespace. With `--line-count`, git-rebase--interactive.sh need not rely on `wc -l` for line count. Signed-off-by: Sidhant Sharma [:tk]--- This the first version of the patch for the small project listed here: https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27 builtin/stripspace.c | 22 +- git-rebase--interactive.sh | 6 +++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/builtin/stripspace.c b/builtin/stripspace.c index 15e716e..e08da03 100644 --- a/builtin/stripspace.c +++ b/builtin/stripspace.c @@ -13,22 +13,38 @@ static void comment_lines(struct strbuf *buf) free(msg); } +static void count_lines(struct strbuf *buf) +{ + size_t len = 0; + int i; + + for (i = 0; i < buf->len; i++) + if (buf->buf[i] == '\n') + len++; + + sprintf(buf->buf, "%zu", len); + buf->len = strlen(buf->buf); +} + static const char * const stripspace_usage[] = { N_("git stripspace [-s | --strip-comments]"), N_("git stripspace [-c | --comment-lines]"), + N_("git stripspace [-l | --line-count]"), NULL }; enum stripspace_mode { STRIP_DEFAULT = 0, STRIP_COMMENTS, - COMMENT_LINES + COMMENT_LINES, + LINE_COUNT }; int cmd_stripspace(int argc, const char **argv, const char *prefix) { struct strbuf buf = STRBUF_INIT; enum stripspace_mode mode = STRIP_DEFAULT; + int count = 0; const struct option options[] = { OPT_CMDMODE('s', "strip-comments", , @@ -37,6 +53,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix) OPT_CMDMODE('c', "comment-lines", , N_("prepend comment character and space to each line"), COMMENT_LINES), + OPT_BOOL('l', "line-count", , N_("count number of lines")), OPT_END() }; @@ -55,6 +72,9 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix) else comment_lines(); + if (count) + count_lines(); + write_or_die(1, buf.buf, buf.len); strbuf_release(); return 0; diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index c0cfe88..e8bef37 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -120,9 +120,9 @@ mark_action_done () { sed -e 1q < "$todo" >> "$done" sed -e 1d < "$todo" >> "$todo".new mv -f "$todo".new "$todo" - new_count=$(git stripspace --strip-comments <"$done" | wc -l) + new_count=$(git stripspace --strip-comments --line-count <"$done") echo $new_count >"$msgnum" - total=$(($new_count + $(git stripspace --strip-comments <"$todo" | wc -l))) + total=$(($new_count + $(git stripspace --strip-comments --line-count <"$todo"))) echo $total >"$end" if test "$last_count" != "$new_count" then @@ -1251,7 +1251,7 @@ test -s "$todo" || echo noop >> "$todo" test -n "$autosquash" && rearrange_squash "$todo" test -n "$cmd" && add_exec_commands "$todo" -todocount=$(git stripspace --strip-comments <"$todo" | wc -l) +todocount=$(git stripspace --strip-comments --line-count <"$todo") todocount=${todocount##* } cat >>"$todo"
Re: [PATCH 2/2] t5510: do not leave changed cwd
Michael J Gruberwrites: > Jeff King venit, vidit, dixit 04.03.2016 12:52: >> On Fri, Mar 04, 2016 at 11:53:50AM +0100, Michael J Gruber wrote: >> >>> t5510 carefully keeps the cwd at the test root by using either subshells >>> or explicit cd'ing back to the root. Use a subshell for the last >>> subtest, too. >> >> I doubt this caused the heisenbug you saw, as we should have an absolute >> path for the trash-dir, and we "cd" to its containing directory before >> deleting it. But this is definitely a good thing to be doing anyway, to >> prevent surprises for new tests added to t5510. > > Absolutely ;) I'll take this one; 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: git diff does not precompose unicode file paths (OS X)
Alexander Rinasswrites: > Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff > function fixes the issue. > > But I had to disable the check (precomposed_unicode != 1) in > precompose_argv to make it work. That’s probably because > precompose_argv is usually called from parse_options and is > missing some other call before it? The "precomposed_unicode" bit comes from your configuration file, so it won't be usable before you call into git_default_core_config and that happens via a call to "git_config(git_diff_ui_config, NULL)". So perhaps you'd want to add the argv munging immediately before init_revisions() call there? -- To unsubscribe from this list: send the line "unsubscribe 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] t9700: fix test for perl older than 5.14
Jeff Kingwrites: > One workaround would therefore be for him to tweak PERL_PATH, but > obviously that does not help anyone else. I think we should do this: > > -- >8 -- > Subject: t9700: fix test for perl older than 5.14 > > Commit d53c2c6 (mingw: fix t9700's assumption about > directory separators, 2016-01-27) uses perl's "/r" regex > modifier to do a non-destructive replacement on a string, > leaving the original unmodified and returning the result. Will apply on js/mingw-tests and merge to 'master' before -rc1. Thanks. > > This feature was introduced in perl 5.14, but systems with > older perl are still common (e.g., CentOS 6.5 still has perl > 5.10). Let's work around it by providing a helper function > that does the same thing using older syntax. > > While we're at it, let's switch to using an alternate regex > separator, which is slightly more readable. > > Reported-by: Christian Couder > Helped-by: Dennis Kaarsemaker > Signed-off-by: Jeff King > --- > And yes, before anyone checks, the alternate separators are available > even in ancient versions of perl. :) > > t/t9700/test.pl | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/t/t9700/test.pl b/t/t9700/test.pl > index 7e8c40b..1b75c91 100755 > --- a/t/t9700/test.pl > +++ b/t/t9700/test.pl > @@ -17,6 +17,12 @@ BEGIN { > use Cwd; > use File::Basename; > > +sub adjust_dirsep { > + my $path = shift; > + $path =~ s{\\}{/}g; > + return $path; > +} > + > BEGIN { use_ok('Git') } > > # set up > @@ -33,7 +39,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer"); > is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent"); > ok($r->config_bool("test.booltrue"), "config_bool: true"); > ok(!$r->config_bool("test.boolfalse"), "config_bool: false"); > -is($r->config_path("test.path") =~ s/\\/\//gr, > $r->config("test.pathexpanded"), > +is(adjust_dirsep($r->config_path("test.path")), > $r->config("test.pathexpanded"), > "config_path: ~/foo expansion"); > is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"], > "config_path: multiple values"); -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Change in .gitignore handling: intended or bug?
Duy Nguyenwrites: > On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudt wrote: >> Verified that it's different in 2.7.0, but 2.7.2 gives expected output. > > Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two > other regression reports before this one. I guess it's all good then > (except for the people still on 2.7.0) Are we good at 2.8.0-rc0, too? Somehow I had an impression that we queued "another attempt to do it differently" or something. ... goes and looks ... $ rungit maint status -suall ?? baz/quux/corge/wibble.txt ?? baz/quux/grault.txt ?? foo/bar.txt $ rungit master status -suall ?? baz/quux/corge/wibble.txt ?? baz/quux/grault.txt ?? baz/waldo.txt ?? foo/bar.txt ?? foo/garply.txt -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t9700: fix test for perl older than 5.14
Hi Peff, On Fri, 4 Mar 2016, Jeff King wrote: > Subject: t9700: fix test for perl older than 5.14 > > Commit d53c2c6 (mingw: fix t9700's assumption about > directory separators, 2016-01-27) uses perl's "/r" regex > modifier to do a non-destructive replacement on a string, > leaving the original unmodified and returning the result. > > This feature was introduced in perl 5.14, but systems with > older perl are still common (e.g., CentOS 6.5 still has perl > 5.10). Let's work around it by providing a helper function > that does the same thing using older syntax. > > While we're at it, let's switch to using an alternate regex > separator, which is slightly more readable. My apologies! And thanks for cleaning up after me. Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/3] Documentation/git-pull: document --[no-]autostash option
On Fri, Mar 4, 2016 at 12:13 AM, Mehul Jainwrote: > Signed-off-by: Mehul Jain > --- > Documentation/git-pull.txt | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt > index a62a2a6..a593972 100644 > --- a/Documentation/git-pull.txt > +++ b/Documentation/git-pull.txt > @@ -128,6 +128,21 @@ unless you have read linkgit:git-rebase[1] carefully. > --no-rebase:: > Override earlier --rebase. > > +--autostash:: > +--no-autostash:: > + Automatically create a temporary stash before the operation > + begins, and apply it after the operation ends. This means > + that you can run rebase on a dirty worktree. > ++ > +This option is only valid when '--rebase' option is used. > ++ > +The default is --no-autostash, unless rebase.autoStash configuration By the way, there is a trailing space on this line. > +is set. > ++ > +[NOTE] > +Use with care: the final stash application after a successful > +rebase might result in non-trivial conflicts. > + > Options related to fetching > ~~~ > > -- > 2.7.1.340.g69eb491.dirty Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe 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/3] pull --rebase: add --[no-]autostash flag
On Fri, Mar 4, 2016 at 12:13 AM, Mehul Jainwrote: > diff --git a/builtin/pull.c b/builtin/pull.c > index 10eff03..b338b83 100644 > --- a/builtin/pull.c > +++ b/builtin/pull.c > @@ -85,6 +85,7 @@ static char *opt_squash; > static char *opt_commit; > static char *opt_edit; > static char *opt_ff; > +static int opt_autostash = -1; > static char *opt_verify_signatures; > static struct argv_array opt_strategies = ARGV_ARRAY_INIT; > static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT; > @@ -146,6 +147,8 @@ static struct option pull_options[] = { > OPT_PASSTHRU(0, "ff-only", _ff, NULL, > N_("abort if fast-forward is not possible"), > PARSE_OPT_NOARG | PARSE_OPT_NONEG), > + OPT_BOOL(0, "autostash", _autostash, > + N_("automatically stash/stash pop before and after rebase")), > OPT_PASSTHRU(0, "verify-signatures", _verify_signatures, NULL, > N_("verify that the named commit has a valid GPG signature"), > PARSE_OPT_NOARG), > @@ -789,7 +792,8 @@ static int run_rebase(const unsigned char *curr_head, > argv_array_pushv(, opt_strategy_opts.argv); > if (opt_gpg_sign) > argv_array_push(, opt_gpg_sign); > - Minor nit: but when I wrote the code for run_rebase() I separated the options for "Shared options" and "Options passed to git-rebase" into different code block groups from the other code, and I would like it if it remained that way :-( > + if (opt_autostash) > + argv_array_push(, "--autostash"); Hmm, interesting. If rebase.autostash=true !opt_autostash, we don't need to pass --no-autostash to git-rebase because it will only stash if the worktree is dirty, but a dirty worktree will be caught by git-pull's die_on_unclean_worktree() anyway. Still, it may be a problem because the worktree may become dirty in-between our "worktree is clean" check and when git-rebase is run (during the git-fetch), and the user may be surprised if git-rebase attempted to stash in that case. So we may wish to pass "--no-autostash" to git-rebase as well. > argv_array_push(, "--onto"); > argv_array_push(, sha1_to_hex(merge_head)); > > @@ -835,18 +839,25 @@ int cmd_pull(int argc, const char **argv, const char > *prefix) > hashclr(orig_head); > > if (opt_rebase) { > - int autostash = 0; > - > if (is_null_sha1(orig_head) && !is_cache_unborn()) > die(_("Updating an unborn branch with changes added > to the index.")); > > - git_config_get_bool("rebase.autostash", ); > - if (!autostash) > + if (opt_autostash == -1) > + git_config_get_bool("rebase.autostash", > _autostash); > + > + if (opt_autostash == 0 || opt_autostash == -1) > die_on_unclean_work_tree(prefix); > > if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs)) > hashclr(rebase_fork_point); > } > + else { Git code style puts the else on the same line, not on a new one. > + /* If --[no-]autostash option is called without --rebase */ Yeah, I agree with Eric that this comment should be dropped, > + if (opt_autostash == 0) > + die(_("--no-autostash option is only valid with > --rebase.")); > + else if (opt_autostash == 1) > + die(_("--autostash option is only valid with > --rebase.")); > + } and these error messages combined. > > if (run_fetch(repo, refspecs)) > return 1; > -- > 2.7.1.340.g69eb491.dirty Regards, Paul -- To unsubscribe from this list: send the line "unsubscribe 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] index-pack: --clone-bundle option
On Thu, Mar 03, 2016 at 03:20:20PM -0800, Junio C Hamano wrote: > Junio C Hamanowrites: > > > Note that this name choice does not matter very much in the larger > > picture. As an initial clone that bootstraps from a clone-bundle is > > expected to do a rough equivalent of: > > > > # create a new repository > > git init new-repository && > > git remote add origin $URL && > > > > # prime the object store and anchor the history to temporary > > # references > > git fetch $bundle 'refs/*:refs/temporary/*' && > > > > # fetch the more recent history from the true origin > > git fetch origin && > > git checkout -f && > > > > # remove the temporary refs > > git for-each-ref -z --format=%(refname) refs/temporary/ | > > xargs -0 git update-ref -d > > > > the names recorded in the bundle will not really matter to the end > > result. > > Actually, the real implementation of "bootstrap with clone-bundle" > is more likely to go like this: > > * The client gets redirected to $name.bndl file, and obtains a > fairly full $name.pack file by downloading them as static > files; > > * The client initializes an empty repository; > > * The pack file is stored at .git/objects/pack/pack-$sha1.pack; > > * When the client does a "git fetch origin" to fill the more > recent part, fetch-pack.c::find_common() would read from the > "git bundle list-heads $name.bndl" to learn the "reference" > objects. These are thrown at rev_list_insert_ref() and are > advertised as "have"s, just like we advertise objects at the > tip of refs in alternate repository. > > So there will be no refs/temporary/* hierarchy we would need to > worry about cleaning up. I don't think details like this matter much to the bundle-generation side, so this is pretty academic at this point. But I think unless we want to do a lot of surgery to git-clone, we'll end up more with something like: 1. init empty repository 2. contact the other side; find out they can redirect us to an alternate url 3. fetch the alternate url; it turns out to be a split bundle. Grab the header, and then spool the data into a temp packfile. When it's all there, we can "index-pack --fix-thin" it in-place. The reason I think we'll end up with this approach is that it keeps the details of split-bundle fetching inside remote-curl. That keeps clone cleaner, and also means we can grab a split-bundle for a fetch, too. > Another possible variant is to redirect the client directly to > download pack-$sha1.pack; "index-pack" needs to be run on the client > side anyway to create pack-$sha1.idx, so at that time it could do > the equivalent of "--clone-bundle" processing (it is not strictly > necessary to create a split bundle) to find the tips of histories, > and use that information when running "git fetch origin". > > So, even though I started working from "split bundle", we may not > have to have such a feature after all to support CDN offloadable and > resumable clone. Yeah. And I think we'd support this in my step (3) by responding to what we get at the URL. I.e., "it turns out to be..." can have many outcomes, and one of them is "a packfile". -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] index-pack: --clone-bundle option
On Thu, Mar 03, 2016 at 02:57:08PM -0800, Junio C Hamano wrote: > Teach a new option "--clone-bundle" to "git index-pack" to create a > split bundle file that uses an existing packfile as its data part. > > The expected "typical" preparation for helping initial clone would > start by preparing a packfile that contains most of the history and > add another packfile that contains the remainder (e.g. the objects > that are only reachable from reflog entries). The first pack can > then be used as the data part of a split bundle and these two files > can be served as static files to bootstrap the clients without > incurring any more CPU cycles to the server side. Just FYI, because I know our setup is anything but typical, but this probably _won't_ be what we do at GitHub. We try to keep everything in a single packfile that contains many "islands", which are not allowed to delta between each other[1]. Individual forks of a repo get their own islands, unreachable objects are in another one, and so forth. So we'd probably never want to provide a whole packfile, as it has way too much data. But we _would_ be able to take a bitmap over the set of packed objects such that if you blindly spew out every object with its bit set, the resulting pack has the desired objects. That's not as turn-key for serving with a dumb http server, obviously. And I don't expect you to write that part. I just wanted to let you know where I foresee having to take this for GitHub to get it deployed. [1] It's a little more complicated than "don't delta with each other". Each object has its own bitmap of which islands it is in, and the rule is that you can use a delta base iff it is in a subset of your own islands. The point is that a clone of a particular island should never have to throw away on-disk deltas. Cleaning up and sharing that code upstream has been on my todo list for about 2 years, but somehow there's always new code to be written. :-/ I'm happy to share if people want to look at the messy state. > Among the objects in the packfile, the ones that are not referenced > by no other objects are identified and recorded as the "references" s/no/any/ (double negative) > in the resulting bundle. As the packfile does not record any ref > information, however, the names of the "references" recorded in the > bundle need to be synthesized; we arbitrarily choose to record the > object whose name is $SHA1 as refs/objects/$SHA1. Makes sense. In an "island" world I'd want to write one bitmap per island, and then reconstruct that list on the fly by referencing the island bitmap with the sha1 names in the pack revidx. > +static void write_bundle_file(const char *filename, unsigned char *sha1, > + const char *packname) > +{ > + int fd = open(filename, O_CREAT|O_EXCL|O_WRONLY, 0600); > + struct strbuf buf = STRBUF_INIT; > + struct stat st; > + int i; > + We should probably bail here if "fd < 0", though I guess technically we will notice in write_in_full(). :) > + if (stat(packname, )) > + die(_("cannot stat %s"), packname); > + > + strbuf_addstr(, "# v3 git bundle\n"); > + strbuf_addf(, "size: %lu\n", (unsigned long) st.st_size); > + strbuf_addf(, "sha1: %s\n", sha1_to_hex(sha1)); > + strbuf_addf(, "data: %s\n\n", packname); This "sha1" field is the sha1 of the packfile, right? If so, I think it's redundant with the pack trailer found in the pack at "packname". I'd prefer not to include that here, because it makes it harder to generate these v3 bundle headers dynamically (you have to actually checksum the pack subset to come up with sha1 up front, as opposed to checksumming as you stream the pack out). It _could_ be used as an http etag, but I think it would make more sense to push implementations toward using a unique value for the "packname" (i.e., so that fetching "https://example.com/foo/1234abcd.pack; is basically immutable). And I think that comes basically for free, because the packname has that same hash in it already. -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 diff does not precompose unicode file paths (OS X)
> On 04 Mar 2016, at 13:16, Torsten Bögershausenwrote: > > On 03/04/2016 10:07 AM, Alexander Rinass wrote: >> Hallo, >> >> It appears that the git diff command does not precompose file path >> arguments, even if the option core.precomposeunicode is set to true (which >> is the default on OS X). >> >> Passing the decomposed form of a file path to the git diff command will >> yield no diff for a modified file. >> >> In my case, the decomposed form of the file path is sent by the OS X Cocoa >> framework's NSTask, wich I am using in an application. It can be simulated >> on OS X by using $(iconv -f utf-8 -t utf-8-mac <<< FILE_PATH) as file path >> argument on the shell. >> >> Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path >> forms, git diff does not. >> >> It can be tested with the following setup on OS X (as iconv's utf-8-mac >> encoding is only available on OS X): >> >> git init . >> git config core.quotepath true >> git config core.precomposeunicode true # (default on OS X) >> touch .gitignore && git add .gitignore && git commit -m "Initial commit" >> echo "." >> Ä >> git add Ä >> git commit -m "Create commit with unicode file path" >> echo "." >> Ä >> This gives the following status, showing the precomposed form of "Ä": >> >> git status --short >> M "\303\204" >> Running git add with both forms does work as expected: >> >> git add Ä >> git status --short >> M "\303\204" >> git reset HEAD -- Ä >> git add $(iconv -f utf-8 -t utf-8-mac <<< Ä) >> git status --short >> M "\303\204" >> git reset HEAD -- Ä >> However, running git diff only works with the precomposed form: >> >> git status --short >> M "\303\204" >> git --no-pager diff -- Ä >> [...shows diff...] >> git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä) >> [...shows NO diff...] >> >> I took a look at the Git source code, and the builtin/diff*.c do not contain >> the parse_options call (which does the precompose_argv call) that the other >> builtins use. >> >> But I am not really familiar with either C or the Git project structure, so >> this may not mean anything. >> >> Best regards, >> Alexander Rinass >> > Good analyzes, and thanks for the report. > It should be possible to stick in a > > precompose_arrgv(argc, argv) > > into builtin/diff.c > > Do you you can test that ? > Sticking a precompose_argv(argc, argv) into diff.c’s cmd_diff function fixes the issue. But I had to disable the check (precomposed_unicode != 1) in precompose_argv to make it work. That’s probably because precompose_argv is usually called from parse_options and is missing some other call before it? I think it is clear that diff.c and friends are definitely missing the precomposing step. I am not sure about the right way to fix though (should parse_options be used in the end?) and my C skills are basic at best, otherwise I would create a patch. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] test-lib: quote TRASH_DIRECTORY
On Fri, Mar 04, 2016 at 02:03:15PM +0100, Michael J Gruber wrote: > >> -test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY > >> +test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY" > > > > I don't think this does anything. The shell doesn't do whitespace > > splitting on the right-hand side of a variable assignment: > > > > $ foo='lots of spaces and "!'\'' funky chars' > > $ bar=$foo > > $ echo "$bar" > > lots of spaces and "!' funky chars > > > > Of course we _do_ need quotes when we refer to $remove_trash as an > > argument (as with "$bar" above), but it looks like we do so correctly > > everywhere. > > I'm used to that behavior, yes, but: > > - Is this true for every shell that we support? It better be, because we rely on it all through our scripts. :) But yes, it is in POSIX, and I think any shell which did not respect it would be broken enough to be unusable (I don't have a copy of Solaris /bin/sh handy, but that's usually our go-to for "unusably broken"). > - Having quotes there, too, is a good reminder to have it also where > necessary. We do already quote variable assignments unnecessarily in lots of places, so I don't mind it too much (it's definitely not _wrong_ to do so). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] t5510: do not leave changed cwd
Jeff King venit, vidit, dixit 04.03.2016 12:52: > On Fri, Mar 04, 2016 at 11:53:50AM +0100, Michael J Gruber wrote: > >> t5510 carefully keeps the cwd at the test root by using either subshells >> or explicit cd'ing back to the root. Use a subshell for the last >> subtest, too. > > I doubt this caused the heisenbug you saw, as we should have an absolute > path for the trash-dir, and we "cd" to its containing directory before > deleting it. But this is definitely a good thing to be doing anyway, to > prevent surprises for new tests added to t5510. Absolutely ;) Michael -- To unsubscribe from this list: send the line "unsubscribe 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] test-lib: quote TRASH_DIRECTORY
Jeff King venit, vidit, dixit 04.03.2016 12:51: > On Fri, Mar 04, 2016 at 11:53:49AM +0100, Michael J Gruber wrote: > >> We always quote $TRASH_DIRECTORY to guard against funky path names. Do >> so in one more spot >> >> Signed-off-by: Michael J Gruber>> --- >> t/test-lib.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/t/test-lib.sh b/t/test-lib.sh >> index 0b47eb6..8957916 100644 >> --- a/t/test-lib.sh >> +++ b/t/test-lib.sh >> @@ -868,7 +868,7 @@ case "$TRASH_DIRECTORY" in >> /*) ;; # absolute path is good >> *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;; >> esac >> -test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY >> +test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY" > > I don't think this does anything. The shell doesn't do whitespace > splitting on the right-hand side of a variable assignment: > > $ foo='lots of spaces and "!'\'' funky chars' > $ bar=$foo > $ echo "$bar" > lots of spaces and "!' funky chars > > Of course we _do_ need quotes when we refer to $remove_trash as an > argument (as with "$bar" above), but it looks like we do so correctly > everywhere. I'm used to that behavior, yes, but: - Is this true for every shell that we support? - Having quotes there, too, is a good reminder to have it also where necessary. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Change in .gitignore handling: intended or bug?
On Fri, Mar 4, 2016 at 6:56 PM, Kevin Daudtwrote: > Verified that it's different in 2.7.0, but 2.7.2 gives expected output. Thanks. 2.7.1 reverts the faulty commit from 2.7.0 that generated two other regression reports before this one. I guess it's all good then (except for the people still on 2.7.0) -- 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] t9700: fix test for perl older than 5.14
On vr, 2016-03-04 at 06:43 -0500, Jeff King wrote: > On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote: > > > On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote: > > > ? Those are just guesses, but if we are tickling a bug in perl's parser, > > > this might avoid them. I also wondered when "/r" appeared. It was in > > > 5.14, so you're presumably good there. The "use" statement at the top of > > > the script says "5.008", so perhaps we should be writing it out longhand > > > anyway (that version is "only" 5 years old, so I suspect there are still > > > systems around with 5.12 or older). > > > > Knowing the system Christian is testing on, I think the problem is that > > the tests are actually being run against perl 5.10, which RHEL 6 ships > > as system perl. As that's still a supported OS, writing tests in a form > > compatible with it would be a good thing :) > > That would make sense. `perl` in t9700-perl-git.sh (and all of our > scripts) is actually a shell function: > > perl () { > command "$PERL_PATH" "$@" > } > > to make sure we respect PERL_PATH everywhere. And that defaults in the > Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH, > but /usr/bin/perl is the system 5.10. Yeah, that's how our systems are set up. > One workaround would therefore be for him to tweak PERL_PATH, but > obviously that does not help anyone else. I think we should do this: Tested against 5.10 and 5.18 and works with both. I also tested the /r variant with 5.18 and that works as expected. -- Dennis Kaarsemaker http://www.kaarsemaker.net -- To unsubscribe from this list: send the line "unsubscribe 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 diff does not precompose unicode file paths (OS X)
On 03/04/2016 10:07 AM, Alexander Rinass wrote: Hallo, It appears that the git diff command does not precompose file path arguments, even if the option core.precomposeunicode is set to true (which is the default on OS X). Passing the decomposed form of a file path to the git diff command will yield no diff for a modified file. In my case, the decomposed form of the file path is sent by the OS X Cocoa framework's NSTask, wich I am using in an application. It can be simulated on OS X by using $(iconv -f utf-8 -t utf-8-mac <<< FILE_PATH) as file path argument on the shell. Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path forms, git diff does not. It can be tested with the following setup on OS X (as iconv's utf-8-mac encoding is only available on OS X): git init . git config core.quotepath true git config core.precomposeunicode true # (default on OS X) touch .gitignore && git add .gitignore && git commit -m "Initial commit" echo "." >> Ä git add Ä git commit -m "Create commit with unicode file path" echo "." >> Ä This gives the following status, showing the precomposed form of "Ä": git status --short M "\303\204" Running git add with both forms does work as expected: git add Ä git status --short M "\303\204" git reset HEAD -- Ä git add $(iconv -f utf-8 -t utf-8-mac <<< Ä) git status --short M "\303\204" git reset HEAD -- Ä However, running git diff only works with the precomposed form: git status --short M "\303\204" git --no-pager diff -- Ä [...shows diff...] git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä) [...shows NO diff...] I took a look at the Git source code, and the builtin/diff*.c do not contain the parse_options call (which does the precompose_argv call) that the other builtins use. But I am not really familiar with either C or the Git project structure, so this may not mean anything. Best regards, Alexander Rinass Good analyzes, and thanks for the report. It should be possible to stick in a precompose_arrgv(argc, argv) into builtin/diff.c Do you you can test that ? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Change in .gitignore handling: intended or bug?
On Fri, Mar 04, 2016 at 01:12:37AM -0500, Charles Strahan wrote: > I'm on 2.7.0. > > Here's a quick sanity check: > > ├── baz > │ ├── quux > │ │ ├── corge > │ │ │ └── wibble.txt > │ │ └── grault.txt > │ └── waldo.txt > └── foo > ├── bar.txt > └── garply.txt > > $ git --version > git version 2.7.0 > > $ git status -sb -uall > ## Initial commit on master > ?? baz/quux/corge/wibble.txt > ?? baz/quux/grault.txt > ?? baz/waldo.txt > ?? foo/bar.txt > ?? foo/garply.txt > > > For the lazy (such as myself), this will set up an identical tree: > > mkdir -p foo > mkdir -p baz/quux/corge > touch foo/bar.txt > touch foo/garply.txt > touch baz/waldo.txt > touch baz/quux/grault.txt > touch baz/quux/corge/wibble.txt > cat <<"EOF" > .gitignore > * > !/foo > !/foo/bar.txt > !/baz > !/baz/quux > !/baz/quux/**/* > EOF > > > I just checked https://git-scm.com/docs/gitignore and the example at the > bottom > suggests that this behavior may be expected: > > $ cat .gitignore > # exclude everything except directory foo/bar > /* > !/foo > /foo/* > !/foo/bar > > Note the /foo/*, explicitly ignoring the entries below /foo. > > This wasn't always the case, though, so I'd love to hear if it was > intentional > (or if I've lost my mind, which is quite possible). > > -Charles > > > > On Fri, Mar 4, 2016, at 12:51 AM, Kevin Daudt wrote: > > On Thu, Mar 03, 2016 at 09:11:56PM -0500, Charles Strahan wrote: > > > Hello, > > > > > > I've found a change in the way .gitignore works, and I'm not sure if > > > it's a bug > > > or intended. > > > > > > Previously, one could use the following .gitignore: > > > > > > * > > > !/foo > > > !/foo/bar.txt > > > !/baz > > > !/baz/quux > > > !/baz/quux/**/* > > > > > > And these files would be seen by git: > > > > > > foo/bar.txt > > > baz/quux/grault.txt > > > baz/quux/corge/wibble.txt > > > > > > And these files would be ignored: > > > > > > foo/garply.txt > > > baz/waldo.txt > > > > > > At some point (between git 2.6.0 and 2.7.0, I think), the behavior > > > changed such > > > that _none_ of the files above would be ignored. Previously, git would > > > treat > > > !/foo as an indication that it should not prune /foo, but that > > > _wouldn't_ be > > > sufficient to un-ignore the contents thereof. Now, it seems the new > > > scheme > > > treats !/foo as functionally equivalent to !/foo followed by !/foo/**/* > > > in the > > > old scheme. > > > > > > I manage my home directory by making it a git repo, and using > > > ~/.gitignore to > > > selectively permit certain files or subdirectories to be seen by git. > > > The recent > > > change in behavior has resulted in sensitive directories like ~/.gpg > > > being > > > un-ignored. For reference, I've appended my .gitignore to the end of > > > this email. > > > > > > So, is this behavior intended, or is this a bug? If the former, is there > > > an > > > announcement explaining this change? > > > > > > -Charles > > > > > > [snip] > > > -- > > > To unsubscribe from this list: send the line "unsubscribe git" in > > > the body of a message to majord...@vger.kernel.org > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > Works as intended for me: > > > > ├── baz > > │ ├── quux > > │ │ ├── corge > > │ │ │ └── wibble.txt > > │ │ └── grault.txt > > │ └── waldo.txt > > └── foo > > ├── bar.txt > > └── garply.txt > > > > $ git status -s -uall > > ?? baz/quux/corge/wibble.txt > > ?? baz/quux/grault.txt > > ?? foo/bar.txt > > > > garply.txt and waldo.txt are ignore, but the rest is still tracked. > > > > I'm on 2.7.2. > -- Verified that it's different in 2.7.0, but 2.7.2 gives expected output. -- To unsubscribe from this list: send the line "unsubscribe 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] t5510: do not leave changed cwd
On Fri, Mar 04, 2016 at 11:53:50AM +0100, Michael J Gruber wrote: > t5510 carefully keeps the cwd at the test root by using either subshells > or explicit cd'ing back to the root. Use a subshell for the last > subtest, too. I doubt this caused the heisenbug you saw, as we should have an absolute path for the trash-dir, and we "cd" to its containing directory before deleting it. But this is definitely a good thing to be doing anyway, to prevent surprises for new tests added to t5510. -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 1/2] test-lib: quote TRASH_DIRECTORY
On Fri, Mar 04, 2016 at 11:53:49AM +0100, Michael J Gruber wrote: > We always quote $TRASH_DIRECTORY to guard against funky path names. Do > so in one more spot > > Signed-off-by: Michael J Gruber> --- > t/test-lib.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 0b47eb6..8957916 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -868,7 +868,7 @@ case "$TRASH_DIRECTORY" in > /*) ;; # absolute path is good > *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;; > esac > -test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY > +test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY" I don't think this does anything. The shell doesn't do whitespace splitting on the right-hand side of a variable assignment: $ foo='lots of spaces and "!'\'' funky chars' $ bar=$foo $ echo "$bar" lots of spaces and "!' funky chars Of course we _do_ need quotes when we refer to $remove_trash as an argument (as with "$bar" above), but it looks like we do so correctly everywhere. -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: t9700-perl-git.sh is broken on some configurations
On Fri, Mar 04, 2016 at 11:30:36AM +0100, Christian Couder wrote: > > Those are just guesses, but if we are tickling a bug in perl's parser, > > this might avoid them. I also wondered when "/r" appeared. It was in > > 5.14, so you're presumably good there. > > If I just remove the "r" at the end of "s/\\/\//gr", I get with both > Perl versions: > > Can't modify non-lvalue subroutine call at t/t9700/test.pl line 36. Right, because the string being operated on is the return value of a function, so we can't do substitution on it (unless with "r", whose purpose is to allow exactly such a thing). > > The "use" statement at the top of > > the script says "5.008", so perhaps we should be writing it out longhand > > anyway (that version is "only" 5 years old, so I suspect there are still > > systems around with 5.12 or older). > > Yeah, it would work. Thanks for confirming the longhand does work; I think the patch I just posted elsewhere in the thread should be good for you, then. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] t9700: fix test for perl older than 5.14
On Fri, Mar 04, 2016 at 11:58:24AM +0100, Dennis Kaarsemaker wrote: > On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote: > > ? Those are just guesses, but if we are tickling a bug in perl's parser, > > this might avoid them. I also wondered when "/r" appeared. It was in > > 5.14, so you're presumably good there. The "use" statement at the top of > > the script says "5.008", so perhaps we should be writing it out longhand > > anyway (that version is "only" 5 years old, so I suspect there are still > > systems around with 5.12 or older). > > Knowing the system Christian is testing on, I think the problem is that > the tests are actually being run against perl 5.10, which RHEL 6 ships > as system perl. As that's still a supported OS, writing tests in a form > compatible with it would be a good thing :) That would make sense. `perl` in t9700-perl-git.sh (and all of our scripts) is actually a shell function: perl () { command "$PERL_PATH" "$@" } to make sure we respect PERL_PATH everywhere. And that defaults in the Makefile to /usr/bin/perl. Christian presumably has 5.14 in his $PATH, but /usr/bin/perl is the system 5.10. One workaround would therefore be for him to tweak PERL_PATH, but obviously that does not help anyone else. I think we should do this: -- >8 -- Subject: t9700: fix test for perl older than 5.14 Commit d53c2c6 (mingw: fix t9700's assumption about directory separators, 2016-01-27) uses perl's "/r" regex modifier to do a non-destructive replacement on a string, leaving the original unmodified and returning the result. This feature was introduced in perl 5.14, but systems with older perl are still common (e.g., CentOS 6.5 still has perl 5.10). Let's work around it by providing a helper function that does the same thing using older syntax. While we're at it, let's switch to using an alternate regex separator, which is slightly more readable. Reported-by: Christian CouderHelped-by: Dennis Kaarsemaker Signed-off-by: Jeff King --- And yes, before anyone checks, the alternate separators are available even in ancient versions of perl. :) t/t9700/test.pl | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/t/t9700/test.pl b/t/t9700/test.pl index 7e8c40b..1b75c91 100755 --- a/t/t9700/test.pl +++ b/t/t9700/test.pl @@ -17,6 +17,12 @@ BEGIN { use Cwd; use File::Basename; +sub adjust_dirsep { + my $path = shift; + $path =~ s{\\}{/}g; + return $path; +} + BEGIN { use_ok('Git') } # set up @@ -33,7 +39,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer"); is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent"); ok($r->config_bool("test.booltrue"), "config_bool: true"); ok(!$r->config_bool("test.boolfalse"), "config_bool: false"); -is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"), +is(adjust_dirsep($r->config_path("test.path")), $r->config("test.pathexpanded"), "config_path: ~/foo expansion"); is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"], "config_path: multiple values"); -- 2.8.0.rc0.309.g6677de9 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t9700-perl-git.sh is broken on some configurations
On vr, 2016-03-04 at 03:56 -0500, Jeff King wrote: > ? Those are just guesses, but if we are tickling a bug in perl's parser, > this might avoid them. I also wondered when "/r" appeared. It was in > 5.14, so you're presumably good there. The "use" statement at the top of > the script says "5.008", so perhaps we should be writing it out longhand > anyway (that version is "only" 5 years old, so I suspect there are still > systems around with 5.12 or older). Knowing the system Christian is testing on, I think the problem is that the tests are actually being run against perl 5.10, which RHEL 6 ships as system perl. As that's still a supported OS, writing tests in a form compatible with it would be a good thing :) -- Dennis Kaarsemaker http://www.kaarsemaker.net -- To unsubscribe from this list: send the line "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/RFC 0/2] Test trash dir sanitizing
I encountered a Heisenbug[*] where t5510 would leave its trash directory without cleanup, though not reproducibly so. This mini series cleans up two spots which may or may not be related, but should be goog cleanup anyways. [*] Running tests with prove -j4. Michael J Gruber (2): test-lib: quote TRASH_DIRECTORY t5510: do not leave changed cwd t/t5510-fetch.sh | 10 ++ t/test-lib.sh| 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) -- 2.8.0.rc0.181.g163d81c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] test-lib: quote TRASH_DIRECTORY
We always quote $TRASH_DIRECTORY to guard against funky path names. Do so in one more spot Signed-off-by: Michael J Gruber--- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 0b47eb6..8957916 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -868,7 +868,7 @@ case "$TRASH_DIRECTORY" in /*) ;; # absolute path is good *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;; esac -test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY +test ! -z "$debug" || remove_trash="$TRASH_DIRECTORY" rm -fr "$TRASH_DIRECTORY" || { GIT_EXIT_OK=t echo >&5 "FATAL: Cannot prepare test area" -- 2.8.0.rc0.181.g163d81c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] t5510: do not leave changed cwd
t5510 carefully keeps the cwd at the test root by using either subshells or explicit cd'ing back to the root. Use a subshell for the last subtest, too. Signed-off-by: Michael J Gruber--- t/t5510-fetch.sh | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 0c10c85..38321d1 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -679,10 +679,12 @@ test_expect_success 'fetching with auto-gc does not lock up' ' EOF git clone "file://$D" auto-gc && test_commit test2 && - cd auto-gc && - git config gc.autoPackLimit 1 && - GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && - ! grep "Should I try again" fetch.out + ( + cd auto-gc && + git config gc.autoPackLimit 1 && + GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && + ! grep "Should I try again" fetch.out + ) ' test_done -- 2.8.0.rc0.181.g163d81c -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t9700-perl-git.sh is broken on some configurations
On Fri, Mar 4, 2016 at 9:56 AM, Jeff Kingwrote: > On Fri, Mar 04, 2016 at 09:13:51AM +0100, Christian Couder wrote: > >> Indeed on the command line I get: >> >> >> $ t/t9700/test.pl >> ok 2 - use Git; >> Bareword found where operator expected at t/t9700/test.pl line 36, >> near "s/\\/\//gr" >> syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr" >> Execution of t/t9700/test.pl aborted due to compilation errors. >> >> >> A quick look at t/t9700/test.pl line 36 was not enough for me to spot >> the problem. >> >> Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for >> x86_64-linux >> >> The machine is running CentOS 6.5. > > I can't reproduce on any of the machines I have handy (perl 5.14, 5.20, > and 5.22). I don't have 5.18 handy. The line in question looks fine to > me, so perhaps it is a temporary regression in 5.18. It is strange because on the same machine there is also v5.10.1 installed and I get the same error with it. > Does it help to wrap it in parentheses, like: > > diff --git a/t/t9700/test.pl b/t/t9700/test.pl > index 7e8c40b..edeeb0e 100755 > --- a/t/t9700/test.pl > +++ b/t/t9700/test.pl > @@ -33,7 +33,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer"); > is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent"); > ok($r->config_bool("test.booltrue"), "config_bool: true"); > ok(!$r->config_bool("test.boolfalse"), "config_bool: false"); > -is($r->config_path("test.path") =~ s/\\/\//gr, > $r->config("test.pathexpanded"), > +is(($r->config_path("test.path") =~ s/\\/\//gr), > $r->config("test.pathexpanded"), > "config_path: ~/foo expansion"); > is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"], > "config_path: multiple values"); No, parentheses don't help. > or even write it out longhand without "/r": > > diff --git a/t/t9700/test.pl b/t/t9700/test.pl > index 7e8c40b..52471cf 100755 > --- a/t/t9700/test.pl > +++ b/t/t9700/test.pl > @@ -33,7 +33,9 @@ is($r->config_int("test.int"), 2048, "config_int: integer"); > is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent"); > ok($r->config_bool("test.booltrue"), "config_bool: true"); > ok(!$r->config_bool("test.boolfalse"), "config_bool: false"); > -is($r->config_path("test.path") =~ s/\\/\//gr, > $r->config("test.pathexpanded"), > +my $test_path = $r->config_path("test.path"); > +$test_path =~ s/\\/\//g; > +is($test_path, $r->config("test.pathexpanded"), > "config_path: ~/foo expansion"); > is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"], > "config_path: multiple values"); > > ? Yeah, it works like the above with both perl versions. > Those are just guesses, but if we are tickling a bug in perl's parser, > this might avoid them. I also wondered when "/r" appeared. It was in > 5.14, so you're presumably good there. If I just remove the "r" at the end of "s/\\/\//gr", I get with both Perl versions: Can't modify non-lvalue subroutine call at t/t9700/test.pl line 36. > The "use" statement at the top of > the script says "5.008", so perhaps we should be writing it out longhand > anyway (that version is "only" 5 years old, so I suspect there are still > systems around with 5.12 or older). Yeah, it would work. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git diff does not precompose unicode file paths (OS X)
Hallo, It appears that the git diff command does not precompose file path arguments, even if the option core.precomposeunicode is set to true (which is the default on OS X). Passing the decomposed form of a file path to the git diff command will yield no diff for a modified file. In my case, the decomposed form of the file path is sent by the OS X Cocoa framework's NSTask, wich I am using in an application. It can be simulated on OS X by using $(iconv -f utf-8 -t utf-8-mac <<< FILE_PATH) as file path argument on the shell. Git commands like add, log, ls-tree, ls-files, mv, ... accept both file path forms, git diff does not. It can be tested with the following setup on OS X (as iconv's utf-8-mac encoding is only available on OS X): git init . git config core.quotepath true git config core.precomposeunicode true # (default on OS X) touch .gitignore && git add .gitignore && git commit -m "Initial commit" echo "." >> Ä git add Ä git commit -m "Create commit with unicode file path" echo "." >> Ä This gives the following status, showing the precomposed form of "Ä": git status --short M "\303\204" Running git add with both forms does work as expected: git add Ä git status --short M "\303\204" git reset HEAD -- Ä git add $(iconv -f utf-8 -t utf-8-mac <<< Ä) git status --short M "\303\204" git reset HEAD -- Ä However, running git diff only works with the precomposed form: git status --short M "\303\204" git --no-pager diff -- Ä [...shows diff...] git --no-pager diff -- $(iconv -f utf-8 -t utf-8-mac <<< Ä) [...shows NO diff...] I took a look at the Git source code, and the builtin/diff*.c do not contain the parse_options call (which does the precompose_argv call) that the other builtins use. But I am not really familiar with either C or the Git project structure, so this may not mean anything. Best regards, Alexander Rinass -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t9700-perl-git.sh is broken on some configurations
On Fri, Mar 04, 2016 at 09:13:51AM +0100, Christian Couder wrote: > Indeed on the command line I get: > > > $ t/t9700/test.pl > ok 2 - use Git; > Bareword found where operator expected at t/t9700/test.pl line 36, > near "s/\\/\//gr" > syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr" > Execution of t/t9700/test.pl aborted due to compilation errors. > > > A quick look at t/t9700/test.pl line 36 was not enough for me to spot > the problem. > > Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for > x86_64-linux > > The machine is running CentOS 6.5. I can't reproduce on any of the machines I have handy (perl 5.14, 5.20, and 5.22). I don't have 5.18 handy. The line in question looks fine to me, so perhaps it is a temporary regression in 5.18. Does it help to wrap it in parentheses, like: diff --git a/t/t9700/test.pl b/t/t9700/test.pl index 7e8c40b..edeeb0e 100755 --- a/t/t9700/test.pl +++ b/t/t9700/test.pl @@ -33,7 +33,7 @@ is($r->config_int("test.int"), 2048, "config_int: integer"); is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent"); ok($r->config_bool("test.booltrue"), "config_bool: true"); ok(!$r->config_bool("test.boolfalse"), "config_bool: false"); -is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"), +is(($r->config_path("test.path") =~ s/\\/\//gr), $r->config("test.pathexpanded"), "config_path: ~/foo expansion"); is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"], "config_path: multiple values"); or even write it out longhand without "/r": diff --git a/t/t9700/test.pl b/t/t9700/test.pl index 7e8c40b..52471cf 100755 --- a/t/t9700/test.pl +++ b/t/t9700/test.pl @@ -33,7 +33,9 @@ is($r->config_int("test.int"), 2048, "config_int: integer"); is($r->config_int("test.nonexistent"), undef, "config_int: nonexistent"); ok($r->config_bool("test.booltrue"), "config_bool: true"); ok(!$r->config_bool("test.boolfalse"), "config_bool: false"); -is($r->config_path("test.path") =~ s/\\/\//gr, $r->config("test.pathexpanded"), +my $test_path = $r->config_path("test.path"); +$test_path =~ s/\\/\//g; +is($test_path, $r->config("test.pathexpanded"), "config_path: ~/foo expansion"); is_deeply([$r->config_path("test.pathmulti")], ["foo", "bar"], "config_path: multiple values"); ? Those are just guesses, but if we are tickling a bug in perl's parser, this might avoid them. I also wondered when "/r" appeared. It was in 5.14, so you're presumably good there. The "use" statement at the top of the script says "5.008", so perhaps we should be writing it out longhand anyway (that version is "only" 5 years old, so I suspect there are still systems around with 5.12 or older). -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
t9700-perl-git.sh is broken on some configurations
Hi, It looks like t9700-perl-git.sh is broken on one machine I use but not on my laptop since commit d53c2c67380f769f91fd45cc8c63a5883245ccca (mingw: fix t9700's assumption about directory separators, Jan 27 17:19:56 2016). I get: Initialized empty Git repository in /home/ccouder/git/git/t/trash directory.t9700-perl-git/.git/ expecting success: echo "test file 1" > file1 && echo "test file 2" > file2 && mkdir directory1 && echo "in directory1" >> directory1/file && mkdir directory2 && echo "in directory2" >> directory2/file && git add . && git commit -m "first commit" && echo "new file in subdir 2" > directory2/file2 && git add . && git commit -m "commit in directory2" && echo "changed file 1" > file1 && git commit -a -m "second commit" && git config --add color.test.slot1 green && git config --add test.string value && git config --add test.dupstring value1 && git config --add test.dupstring value2 && git config --add test.booltrue true && git config --add test.boolfalse no && git config --add test.boolother other && git config --add test.int 2k && git config --add test.path "~/foo" && git config --add test.pathexpanded "$HOME/foo" && git config --add test.pathmulti foo && git config --add test.pathmulti bar [master (root-commit) fc41470] first commit Author: A U Thor4 files changed, 4 insertions(+) create mode 100644 directory1/file create mode 100644 directory2/file create mode 100644 file1 create mode 100644 file2 [master 6a30dee] commit in directory2 Author: A U Thor 1 file changed, 1 insertion(+) create mode 100644 directory2/file2 [master 33414b1] second commit Author: A U Thor 1 file changed, 1 insertion(+), 1 deletion(-) ok 1 - set up test repository # run 1: Perl API (perl /home/ccouder/git/git/t/t9700/test.pl) ok 2 - use Git; # test_external test Perl API failed: perl /home/ccouder/git/git/t/t9700/test.pl # expecting no stderr from previous command # test_external_without_stderr test no stderr: Perl API failed: perl /home/ccouder/git/git/t/t9700/test.pl: # Stderr is: Bareword found where operator expected at /home/ccouder/git/git/t/t9700/test.pl line 36, near "s/\\/\//gr" syntax error at /home/ccouder/git/git/t/t9700/test.pl line 36, near "s/\\/\//gr" Execution of /home/ccouder/git/git/t/t9700/test.pl aborted due to compilation errors. Indeed on the command line I get: $ t/t9700/test.pl ok 2 - use Git; Bareword found where operator expected at t/t9700/test.pl line 36, near "s/\\/\//gr" syntax error at t/t9700/test.pl line 36, near "s/\\/\//gr" Execution of t/t9700/test.pl aborted due to compilation errors. A quick look at t/t9700/test.pl line 36 was not enough for me to spot the problem. Perl version is: perl 5, version 18, subversion 2 (v5.18.2) built for x86_64-linux The machine is running CentOS 6.5. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html