Score in diff-format
Hi, diff-format.txt says this: An output line is formatted this way: (snip) That is, from the left to the right: (snip) . status, followed by optional "score" number. (snip) Status letters C and R are always followed by a score (denoting the percentage of similarity between the source and target of the move or copy), and are the only ones to be so. As I read this last paragraph, the following is not supposed to happen: $ git diff-tree 926b1ec63ee045503f609e88ca445b94c06bd5d7 --abbrev -r -C -B 926b1ec63ee045503f609e88ca445b94c06bd5d7 :100644 100644 81ac702... 7ab0cf4... M087 contrib/subtree/INSTALL It however makes sense that it happens, and it looks like a case of the documentation being outdated or confusing. Or am I interpreting it wrong? Cheers, Mike -- To unsubscribe from this list: send the line "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] http: Add Accept-Language header if possible
On Tue, Jan 27, 2015 at 3:34 PM, Junio C Hamano wrote: > Yi EungJun writes: > >> + >> + sprintf(q_format, ";q=0.%%0%dd", decimal_places); >> + >> + strbuf_addstr(buf, "Accept-Language: "); >> + >> + for(i = 0; i < num_langs; i++) { >> + if (i > 0) >> + strbuf_addstr(buf, ", "); >> + >> + strbuf_addstr(buf, strbuf_detach(&language_tags[i], >> NULL)); > > This is not wrong per-se, but it looks somewhat convoluted to me. > ... Actually, this is wrong, isn't it? strbuf_detach() removes the language_tags[i].buf from the strbuf, and the caller now owns that piece of memory. Then strbuf_addstr() appends a copy of that string to buf, and the piece of memory that was originally held by language_tags[i].buf is now lost forever. This is leaking. >> + /* free language tags */ >> + for(i = 0; i < num_langs; i++) { >> + strbuf_release(&language_tags[i]); >> + } ... because this loop does not free memory for earlier parts of language_tags[]. > I am wondering if using strbuf for each of the language_tags[] is > even necessary. How about doing it this way instead? And I think my counter-proposal does not leak (as it does not us strbuf for language_tags[] anymore). > > http.c | 22 +- > 1 file changed, 9 insertions(+), 13 deletions(-) > > diff --git a/http.c b/http.c > index 6111c6a..db591b3 100644 > --- a/http.c > +++ b/http.c > @@ -1027,7 +1027,7 @@ static void write_accept_language(struct strbuf *buf) > const int MAX_DECIMAL_PLACES = 3; > const int MAX_LANGUAGE_TAGS = 1000; > const int MAX_ACCEPT_LANGUAGE_HEADER_SIZE = 4000; > - struct strbuf *language_tags = NULL; > + char **language_tags = NULL; > int num_langs = 0; > const char *s = get_preferred_languages(); > int i; > @@ -1053,9 +1053,7 @@ static void write_accept_language(struct strbuf *buf) > if (tag.len) { > num_langs++; > REALLOC_ARRAY(language_tags, num_langs); > - strbuf_init(&language_tags[num_langs - 1], 0); > - strbuf_swap(&tag, &language_tags[num_langs - 1]); > - > + language_tags[num_langs - 1] = strbuf_detach(&tag, > NULL); > if (num_langs >= MAX_LANGUAGE_TAGS - 1) /* -1 for '*' > */ > break; > } > @@ -1070,13 +1068,12 @@ static void write_accept_language(struct strbuf *buf) > > /* add '*' */ > REALLOC_ARRAY(language_tags, num_langs + 1); > - strbuf_init(&language_tags[num_langs], 0); > - strbuf_addstr(&language_tags[num_langs++], "*"); > + language_tags[num_langs++] = "*"; /* it's OK; this won't be > freed */ > > /* compute decimal_places */ > for (max_q = 1, decimal_places = 0; > - max_q < num_langs && decimal_places <= > MAX_DECIMAL_PLACES; > - decimal_places++, max_q *= 10) > +max_q < num_langs && decimal_places <= > MAX_DECIMAL_PLACES; > +decimal_places++, max_q *= 10) > ; > > sprintf(q_format, ";q=0.%%0%dd", decimal_places); > @@ -1087,7 +1084,7 @@ static void write_accept_language(struct strbuf *buf) > if (i > 0) > strbuf_addstr(buf, ", "); > > - strbuf_addstr(buf, strbuf_detach(&language_tags[i], > NULL)); > + strbuf_addstr(buf, language_tags[i]); > > if (i > 0) > strbuf_addf(buf, q_format, max_q - i); > @@ -1101,10 +1098,9 @@ static void write_accept_language(struct strbuf *buf) > } > } > > - /* free language tags */ > - for(i = 0; i < num_langs; i++) { > - strbuf_release(&language_tags[i]); > - } > + /* free language tags -- last one is a static '*' */ > + for(i = 0; i < num_langs - 1; i++) > + free(language_tags[i]); > free(language_tags); > } > -- To unsubscribe from this list: send the line "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] Correction to git-p4 "exclude" change
My previous change for adding support for "exclude" to git-p4 "sync" was incorrect, missing out a comma, which stopped git-p4 from working. This change fixes that. I've also noticed that t9814-git-p4-rename.sh has stopped working; I'm going to follow up with a fix for that once I've worked out what's wrong with it. There's a small shell syntax problem (missing "esac") but after fixing that it still fails, so I'm not sure what's happening yet. It was discussed a while back. Luke Diamand (1): git-p4: correct "exclude" change git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.3.0.rc2.188.g4b64765.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-p4: correct "exclude" change
The previous change for excluding paths in the "sync" subcommand was incorrect, missing a comma, preventing git-p4 from working. Signed-off-by: Luke Diamand --- git-p4.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-p4.py b/git-p4.py index 6ff0b76..549022e 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1915,7 +1915,7 @@ class P4Sync(Command, P4UserMap): optparse.make_option("--keep-path", dest="keepRepoPath", action='store_true', help="Keep entire BRANCH/DIR/SUBDIR prefix during import"), optparse.make_option("--use-client-spec", dest="useClientSpec", action='store_true', - help="Only sync files that are included in the Perforce Client Spec") + help="Only sync files that are included in the Perforce Client Spec"), optparse.make_option("-/", dest="cloneExclude", action="append", type="string", help="exclude depot path"), -- 2.3.0.rc2.188.g4b64765.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 21/21] t3080: tests for git-list-files
Test 3 is failing on my mac: expecting success: test_config color.ls.file red && test_config color.ls.directory green && test_config color.ls.submodule yellow && git list-files --color=always >actual && test_cmp "$TEST_DIRECTORY"/t3080/color_ls actual --- /Users/michael.blume/workspace/git/t/t3080/color_ls 2015-01-28 04:40:23.0 + +++ actual 2015-01-28 04:42:59.0 + @@ -1,3 +1,3 @@ -dir -file -gitlink +dir +file +gitlink not ok 3 - color.ls.* # # test_config color.ls.file red && # test_config color.ls.directory green && # test_config color.ls.submodule yellow && # git list-files --color=always >actual && # test_cmp "$TEST_DIRECTORY"/t3080/color_ls actual # On Sun, Jan 25, 2015 at 11:20 AM, Eric Sunshine wrote: > On Sun, Jan 25, 2015 at 7:37 AM, Nguyễn Thái Ngọc Duy > wrote: >> Signed-off-by: Nguyễn Thái Ngọc Duy >> --- >> diff --git a/t/t3080-list-files.sh b/t/t3080-list-files.sh >> new file mode 100755 >> index 000..6313dd9 >> --- /dev/null >> +++ b/t/t3080-list-files.sh >> +test_expect_success 'no dups' ' >> + echo dirty >>file && > > To leave a clean slate for subsequent tests, would it make sense to > restore 'file' to a clean state via test_when_finished()? > >> + git list-files -m file >actual && >> + echo "file" >expected && >> + test_cmp expected actual && >> + git list-files -cm file >actual && >> + echo "C file" >expected && >> + test_cmp expected actual && >> + git list-files -tcm file >actual && >> + test_cmp expected actual >> +' >> + >> +test_expect_success 'diff-cached' ' >> + echo dirty >>file && >> + git add file && > > Ditto here? > >> + git list-files -M >actual && >> + echo "file" >expected && >> + test_cmp expected actual >> +' > -- > To unsubscribe from this list: send the line "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
[ANNOUNCE] Git v2.3.0-rc2
A release candidate Git v2.3.0-rc2 is now available for testing at the usual places. Hopefully this can become the final v2.3.0 next week, almost as-is. There are no regression noticed and/or fixed since -rc1, and the changes are mostly l10n and minor documentation updates. 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.3.0-rc2' 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 = https://code.google.com/p/git-core/ 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 Git v2.3 Release Notes (draft) == Updates since v2.2 -- Ports * Recent gcc toolchain on Cygwin started throwing compilation warning, which has been squelched. * A few updates to build on platforms that lack tv_nsec, clock_gettime, CLOCK_MONOTONIC and HMAC_CTX_cleanup (e.g. older RHEL) have been added. UI, Workflows & Features * It was cumbersome to use "GIT_SSH" mechanism when the user wanted to pass an extra set of arguments to the underlying ssh. A new environment variable GIT_SSH_COMMAND can be used for this. * A request to store an empty note via "git notes" meant to remove note from the object but with --allow-empty we will store a (surprise!) note that is empty. * "git interpret-trailers" learned to properly handle the "Conflicts:" block at the end. * "git am" learned "--message-id" option to copy the message ID of the incoming e-mail to the log message of resulting commit. * "git clone --reference=" learned the "--dissociate" option to go with it; it borrows objects from the reference object store while cloning only to reduce network traffic and then dissociates the resulting clone from the reference by performing local copies of borrowed objects. * "git send-email" learned "--transfer-encoding" option to force a non-fault Content-Transfer-Encoding header (e.g. base64). * "git send-email" normally identifies itself via X-Mailer: header in the message it sends out. A new command line flag --no-xmailer allows the user to squelch the header. * "git push" into a repository with a working tree normally refuses to modify the branch that is checked out. The command learned to optionally do an equivalent of "git reset --hard" only when there is no change to the working tree and the index instead, which would be useful to "deploy" by pushing into a repository. * "git new-workdir" (in contrib/) can be used to populate an empty and existing directory now. * Credential helpers are asked in turn until one of them give positive response, which is cumbersome to turn off when you need to run Git in an automated setting. The credential helper interface learned to allow a helper to say "stop, don't ask other helpers." Also GIT_TERMINAL_PROMPT environment can be set to false to disable our built-in prompt mechanism for passwords. * "git branch -d" (delete) and "git branch -m" (move) learned to honor "-f" (force) flag; unlike many other subcommands, the way to force these have been with separate "-D/-M" options, which was inconsistent. * "diff-highlight" filter (in contrib/) allows its color output to be customized via configuration variables. * "git imap-send" learned to take "-v" (verbose) and "-q" (quiet) command line options. * "git remote add $name $URL" is now allowed when "url.$URL.insteadOf" is already defined. * "git imap-send" now can be built to use cURL library to talk to IMAP servers (if the library is recent enough, of course). This allows you to use authenticate method other than CRAM-MD5, among other things. * "git imap-send" now allows GIT_CURL_VERBOSE environment variable to control the verbosity when talking via the cURL library. * The prompt script (in contrib/) learned to optionally hide prompt when in an ignored directory by setting GIT_PS1_HIDE_IF_PWD_IGNORED shell variable. Performance, Internal Implementation, Development Support etc. * Earlier we made "rev-list --object-edge" more aggressively list the objects at the edge commits, in order to reduce number of objects fetched into a shallow repository, but the change affected cases other than "fetching into a shallow repository" and made it unusably slow (e.g. fetching into a normal repository should not have to suffer the overhead from extra processing). Limit it to a more specific case by introducing --objects-edge-aggressive, a new option to rev-list. * Squelched useless compiler warnings on Mac OS X regarding the crypto API. * The procedure to generate unicode table has been simplified. * Some filesystems assign filemodes in a st
What's cooking in git.git (Jan 2015, #05; Tue, 27)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Hopefully final release candidate 2.3-rc2 has been tagged. Please spend some time to find and fix regressions, instead of spending all time having fun with new and shiny toys. The final hopefully will happen sometime next week. 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"] * ak/cat-file-clean-up (2015-01-13) 1 commit (merged to 'next' on 2015-01-15 at bb1a4b3) + cat-file: use "type" and "size" from outer scope * js/t1050 (2015-01-14) 1 commit (merged to 'next' on 2015-01-15 at f010b00) + t1050-large: generate large files without dd -- [New Topics] * jc/apply-ws-fix-expands-report (2015-01-22) 1 commit - apply: detect and mark whitespace errors in context lines when fixing (this branch uses jc/apply-ws-fix-expands.) "git apply --whitespace=fix" fixed whitespace errors in the common context lines but did so without reporting. Will merge to 'next'. * ks/rebase-i-abbrev (2015-01-22) 1 commit - rebase -i: use full object name internally throughout the script The insn sheet "git rebase -i" creates did not fully honor core.abbrev settings. Will merge to 'next'. * mh/deref-symref-over-helper-transport (2015-01-21) 1 commit - transport-helper: do not request symbolic refs to remote helpers "git fetch" over a remote-helper that cannot respond to "list" command could not fetch from a symbolic reference e.g. HEAD. Will merge to 'next'. * ak/add-i-empty-candidates (2015-01-22) 1 commit - add -i: return from list_and_choose if there is no candidate The interactive "show a list and let the user choose from it" interface "add -i" used showed and prompted to the user even when the candidate list was empty, against which the only "choice" the user could have made was to choose nothing. Will merge to 'next'. * av/wincred-with-at-in-username-fix (2015-01-25) 1 commit - wincred: fix get credential if username has "@" Will merge to 'next'. * jc/conf-var-doc (2015-01-27) 3 commits - CodingGuidelines: describe naming rules for configuration variables - config.txt: mark deprecated variables more prominently - config.txt: clarify that add.ignore-errors as deprecated Need to send this out for review before doing anything to it. * jc/doc-log-rev-list-options (2015-01-23) 1 commit - Documentation: what does "git log --indexed-objects" even mean? Will merge to 'next'. * jk/dumb-http-idx-fetch-fix (2015-01-27) 1 commit - dumb-http: do not pass NULL path to parse_pack_index Will merge to 'next'. * ld/p4-submit-hint (2015-01-23) 1 commit (merged to 'next' on 2015-01-23 at ed972d3) + git-p4: correct --prepare-p4-only instructions Will merge to 'master' after 2.3 final. * mg/commit-author-no-match-malformed-message (2015-01-26) 1 commit - commit: reword --author error message Will merge to 'next'. * mg/push-repo-option-doc (2015-01-27) 1 commit - git-push.txt: document the behavior of --repo Will merge to 'next'. -- [Stalled] * jn/doc-api-errors (2014-12-04) 1 commit - doc: document error handling functions and conventions For discussion. * pw/remote-set-url-fetch (2014-11-26) 1 commit - remote: add --fetch and --both options to set-url Expecting a reroll. * ms/submodule-update-config-doc (2014-11-03) 1 commit - submodule: clarify documentation for update subcommand Needs a reroll ($gmane/259037). * je/quiltimport-no-fuzz (2014-10-21) 2 commits - git-quiltimport: flip the default not to allow fuzz - git-quiltimport.sh: allow declining fuzz with --exact option "quiltimport" drove "git apply" always with -C1 option to reduce context of the patch in order to give more chance to somewhat stale patches to apply. Add an "--exact" option to disable, and also "-C$n" option to customize this behaviour. The top patch optionally flips the default to "--exact". Tired of waiting for an Ack; will discard. * tr/remerge-diff (2014-11-10) 9 commits - t4213: avoid "|" in sed regexp - log --remerge-diff: show what the conflict resolution changed - name-hash: allow dir hashing even when !ignore_case - merge-recursive: allow storing conflict hunks in index - merge_diff_mode: fold all merge diff variants into an enum - combine-diff: do not pass revs->dense_combined_merges redundantly - merge-recursive: -Xindex-only to leave worktree unchanged - merge-recursive: internal flag to avoid touching the worktree - merge-recursive: remove dead conditional in update_stages() "log -p" output learns a new way to let users inspect a merge commit by showing the differences between the automerged result with co
Re: [PATCH] http: Add Accept-Language header if possible
Yi EungJun writes: > +static void write_accept_language(struct strbuf *buf) > +{ > + /* > + * MAX_DECIMAL_PLACES must not be larger than 3. If it is larger than > + * that, q-value will be smaller than 0.001, the minimum q-value the > + * HTTP specification allows. See > + * http://tools.ietf.org/html/rfc7231#section-5.3.1 for q-value. > + */ > + const int MAX_DECIMAL_PLACES = 3; > + const int MAX_LANGUAGE_TAGS = 1000; > + const int MAX_ACCEPT_LANGUAGE_HEADER_SIZE = 4000; > + struct strbuf *language_tags = NULL; > + int num_langs = 0; > + const char *s = get_preferred_languages(); > + int i; > + struct strbuf tag = STRBUF_INIT; > + > + /* Don't add Accept-Language header if no language is preferred. */ > + if (!s) > + return; > + > + /* > + * Split the colon-separated string of preferred languages into > + * language_tags array. > + */ > + do { > + /* collect language tag */ > + for (; *s && (isalnum(*s) || *s == '_'); s++) > + strbuf_addch(&tag, *s == '_' ? '-' : *s); > + > + /* skip .codeset, @modifier and any other unnecessary parts */ > + while (*s && *s != ':') > + s++; > + > + if (tag.len) { > + num_langs++; > + REALLOC_ARRAY(language_tags, num_langs); > + strbuf_init(&language_tags[num_langs - 1], 0); > + strbuf_swap(&tag, &language_tags[num_langs - 1]); > + > + if (num_langs >= MAX_LANGUAGE_TAGS - 1) /* -1 for '*' */ > + break; > + } > + } while (*s++); The structure of this function is much easier to understand than any of the previous rounds. You collect up to the max you are going to support, and then you format up to the max you are going to send. Very straight-forward and simple. > + /* write Accept-Language header into buf */ > + if (num_langs >= 1) { micronit: should be OK to just say "if (num_langs)". > + int last_buf_len = 0; > + int max_q; > + int decimal_places; > + char q_format[32]; > + > + /* add '*' */ > + REALLOC_ARRAY(language_tags, num_langs + 1); > + strbuf_init(&language_tags[num_langs], 0); > + strbuf_addstr(&language_tags[num_langs++], "*"); > + > + /* compute decimal_places */ > + for (max_q = 1, decimal_places = 0; > + max_q < num_langs && decimal_places <= > MAX_DECIMAL_PLACES; > + decimal_places++, max_q *= 10) > + ; micronit: the second and the third line are indented too deeply and made me wonder if this has an overlong first line (i.e. the set-up part to enter the for-loop) split into multiple lines. > + > + sprintf(q_format, ";q=0.%%0%dd", decimal_places); > + > + strbuf_addstr(buf, "Accept-Language: "); > + > + for(i = 0; i < num_langs; i++) { > + if (i > 0) > + strbuf_addstr(buf, ", "); > + > + strbuf_addstr(buf, strbuf_detach(&language_tags[i], > NULL)); This is not wrong per-se, but it looks somewhat convoluted to me. You can just peek language_tags[i].buf here without detaching, as you will free the strbufs after you exit the loop anyway. Your version is not wrong and it does not result in double-freeing, because detach clears the strbuf, but at the same time, makes the responsibility to free language_tags[] strbuf split into here (for elements up to the ones that are used to fill buf) and the cleanup loop (for elements that are not used in this loop). > + if (i > 0) > + strbuf_addf(buf, q_format, max_q - i); > + > + if (buf->len > MAX_ACCEPT_LANGUAGE_HEADER_SIZE) { > + strbuf_remove(buf, last_buf_len, buf->len - > last_buf_len); > + break; > + } > + > + last_buf_len = buf->len; > + } > + } > + > + /* free language tags */ > + for(i = 0; i < num_langs; i++) { > + strbuf_release(&language_tags[i]); > + } > + free(language_tags); > +} I am wondering if using strbuf for each of the language_tags[] is even necessary. How about doing it this way instead? http.c | 22 +- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/http.c b/http.c index 6111c6a..db591b3 100644 --- a/http.c +++ b/http.c @@ -1027,7 +1027,7 @@ static void write_accept_language(struct strbuf *buf) const int MAX_DECIMAL_PLACES = 3; const int MAX_LANGUAGE_TAGS = 1000; const int MAX_ACCEPT_LANGUAGE_HEADER_SIZE = 4000; - struct strbuf *language_tags = NULL; + char **lang
Re: implement a stable 'Last updated' in Documentation
Olaf Hering writes: > Several files in Documentation have an unstable 'Last updated' timestamp. The > reason is that their mtime changes every time, which prevents reproducible > builds. > > 341 technical/api-index.txt: technical/api-index-skel.txt \ > 342 technical/api-index.sh $(patsubst %,%.txt,$(API_DOCS)) > 343 $(QUIET_GEN)cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh > > 388 howto-index.txt: howto-index.sh $(wildcard howto/*.txt) > 389 $(QUIET_GEN)$(RM) $@+ $@ && \ > 390 '$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(wildcard > howto/*.txt)) >$@+ && \ > 391 mv $@+ $@ > > 399 $(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt > 400 $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ > 401 sed -e '1,/^$$/d' $< | \ > 402 $(TXT_TO_HTML) - >$@+ && \ > 403 mv $@+ $@ All of them seem to have dependencies so it seems to me that two builds back to back without actually changing their input would not re-build anything. What am I missing??? > What file timestamp should be used for them? Likely "../version"? I tend to think the "Last updated" timestamp taken from the filesystem timestamp is a bad practice inherited by these tools from the days back when nobody used any revision control systems. If I check out v1.8.5 and build documentation now, it does not matter if the generated documentation had the timestamp of the checkout of individual files or that of a single file generated during the build process, i.e. ../version. Neither has much relevance to the time the actual contents of the documentation was prepared or what vintage of the software the documentation is for. I am fine with branding generated documentation with the version number (i.e. "You are reading the documentation for Git version 2.2.0"). Replacing that statement with "You are reading the documentation for a version of Git that was committed on such and such time" is also fine. But using file timestamp would not help either. And that practice of using file timestamp is doubly bad by leading misguided people to want to set timestamps of all the files by copying commit timestamp. That is backwards, I would think. -- To unsubscribe from this list: send the line "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] test-lib.sh: Dynamic test for the prerequisite SANITY
Torsten Bögershausen writes: > # When the tests are run as root, permission tests will report that > # things are writable when they shouldn't be. This no longer is relevant, I think. > +# Special check for CYGWIN (or Windows in general): Misleading comment in the end result, as your new test drops SANITY correctly on POSIX for the root user, too. In a commit log message it is correct to say "This adds special check for Cygwin", but the resulting code is sensible with or without Cygwin, I would think, with the justification to "test by checking what we really want, not by inferring from the result of indirectly testing something else". > +# A file can be deleted, even if the containing directory does'nt > +# have write permissions We also rely on SANITY to make sure that "chmod -rx directory" makes "directory/file" undiscoverable. How about extending it like this (not tested)? -- >8 -- From: Torsten Bögershausen Date: Tue, 27 Jan 2015 16:39:01 +0100 Subject: [PATCH] test-lib.sh: set prerequisite SANITY by testing what we really need What we wanted out of the SANITY precondition is that the filesystem behaves sensibly with permission bits settings. - You should not be able to remove a file in a read-only directory, - You should not be able to tell if a file in a directory exists if the directory lacks read or execute permission bits. We used to cheat by approximating that condition with "is the / writable?" test and/or "are we running as root?" test. Neither test is sufficient or appropriate in more exotic environments like Cygwin. Signed-off-by: Torsten Bögershausen Signed-off-by: Junio C Hamano --- t/test-lib.sh | 25 ++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index b2b2ec7..446d8d5 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -997,9 +997,28 @@ test_lazy_prereq NOT_ROOT ' test "$uid" != 0 ' -# When the tests are run as root, permission tests will report that -# things are writable when they shouldn't be. -test -w / || test_set_prereq SANITY +# On a filesystem that lacks SANITY, a file can be deleted even if +# the containing directory doesn't have write permissions, or a file +# can be accessed even if the containing directory doesn't have read +# or execute permissions, causing our tests that validate that Git +# works sensibly in such situations. +test_lazy_prereq SANITY ' + mkdir SANETESTD.1 SANETESTD.2 && + + chmod +w SANETESTD.1 SANETESTD.2 && + >SANETESTD.1/x 2>SANETESTD.2/x && + chmod -w SANETESTD.1 && + chmod -rx SANETESTD.2 || + error "bug in test sript: cannot prepare SANETESTD" + + ! rm SANETESTD.1/x && ! test -f SANETESTD.2/x + status=$? + + chmod +rwx SANETESTD.1 SANETESTD.2 && + rm -rf SANETESTD.1 SANETESTD.2 || + error "bug in test sript: cannot clean SANETESTD" + return $status +' GIT_UNZIP=${GIT_UNZIP:-unzip} test_lazy_prereq UNZIP ' -- 2.3.0-rc1-180-g1a69fe5 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-push.txt: document the behavior of --repo
On Tue, Jan 27, 2015 at 7:35 AM, Michael J Gruber wrote: > As per the code, the --repo option is equivalent to the > argument to 'git push'. [It exists for historical reasons, back from the time > when options had to come before arguments.] > > Say so. [But not that.] > > Signed-off-by: Michael J Gruber > --- > diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt > index ea97576..0ad31c4 100644 > --- a/Documentation/git-push.txt > +++ b/Documentation/git-push.txt > @@ -219,22 +219,8 @@ origin +master` to force a push to the `master` branch). > See the > `...` section above for details. > > --repo=:: > - This option is only relevant if no argument is > - passed in the invocation. In this case, 'git push' derives the > - remote name from the current branch: If it tracks a remote > - branch, then that remote repository is pushed to. Otherwise, > - the name "origin" is used. For this latter case, this option > - can be used to override the name "origin". In other words, > - the difference between these two commands > -+ > --- > -git push public #1 > -git push --repo=public #2 > --- > -+ > -is that #1 always pushes to "public" whereas #2 pushes to "public" > -only if the current branch does not track a remote branch. This is > -useful if you write an alias or script around 'git push'. > + This option is equivalent to the argument; the latter > + wins if both are specified. To what does "latter" refer in this case? (I presume it means the standalone argument, though the text feels ambiguous.) Also, both the standalone argument and the right-hand-side of --repo= are spelled "", so there may be potential for confusion when talking about (despite the subsequent "argument"). Perhaps qualifying it as "_standalone_ argument" might help. > -u:: > --set-upstream:: > -- > 2.3.0.rc1.222.gae238f2 -- To unsubscribe from this list: send the line "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 17/21] list-files: show directories as well as files
Nguyễn Thái Ngọc Duy writes: > @@ -194,16 +225,31 @@ static void write_ce_name(struct strbuf *sb, const > struct cache_entry *ce) > static void show_ce_entry(const char *tag, const struct cache_entry *ce) > { > static struct strbuf sb = STRBUF_INIT; > - int len = max_prefix_len; > + int len = max_prefix_len, saved_max_depth; > > if (len >= ce_namelen(ce)) > die("git ls-files: internal error - cache entry not superset of > prefix"); > > + if (show_dirs) { > + /* ignore depth to catch dirs that contain matched entries */ > + saved_max_depth = pathspec.max_depth; > + pathspec.max_depth = -1; > + } > + > if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce), > len, ps_matched, > S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) > return; > > + if (show_dirs) { > + pathspec.max_depth = saved_max_depth; > + if (strchr(ce->name, '/') && > + !match_pathspec(&pathspec, ce->name, ce_namelen(ce), > + prefix_len, NULL, 1) && > + show_as_directory(ce)) > + return; > + } > + My compiler seems to be too stupid to notice that saved_max_depth is always set before it is used, if it gets used and complains. Sigh. For now I am tempted to squash this in. Note that the original does not seem to restore saved_max_depath when the pathspec does not match and function returns in the call to match_pathspec() we have in the code before your patch, which smells like a bug, and the attached would fix it. builtin/ls-files.c | 33 ++--- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 29b5c2e..f28b7e9 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -222,27 +222,38 @@ static void write_ce_name(struct strbuf *sb, const struct cache_entry *ce) strbuf_release("ed); } +static int match_pathspec_with_depth(struct pathspec *ps, +const char *name, int namelen, +int prefix, char *seen, int is_dir, +const int *custom_depth) +{ + int saved_depth = ps->max_depth; + int result; + + if (custom_depth) + ps->max_depth = *custom_depth; + result = match_pathspec(ps, name, namelen, prefix, seen, is_dir); + if (custom_depth) + ps->max_depth = saved_depth; + return result; +} + static void show_ce_entry(const char *tag, const struct cache_entry *ce) { static struct strbuf sb = STRBUF_INIT; - int len = max_prefix_len, saved_max_depth; + int len = max_prefix_len; + static const int infinite_depth = -1; if (len >= ce_namelen(ce)) die("git ls-files: internal error - cache entry not superset of prefix"); - if (show_dirs) { - /* ignore depth to catch dirs that contain matched entries */ - saved_max_depth = pathspec.max_depth; - pathspec.max_depth = -1; - } - - if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce), - len, ps_matched, - S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode))) + if (!match_pathspec_with_depth(&pathspec, ce->name, ce_namelen(ce), + len, ps_matched, + S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode), + show_dirs ? &infinite_depth : NULL)) return; if (show_dirs) { - pathspec.max_depth = saved_max_depth; if (strchr(ce->name, '/') && !match_pathspec(&pathspec, ce->name, ce_namelen(ce), prefix_len, NULL, 1) && -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dumb-http: do not pass NULL path to parse_pack_index
Charles Bailey writes: > On Tue, Jan 27, 2015 at 03:02:27PM -0500, Jeff King wrote: >> Discovery and tests by Charles Bailey . >> >> Signed-off-by: Jeff King >> --- >> I'm happy to flip the authorship on this. You have more lines in it than >> I do. :) > > No, I'm happy with you taking the blame/praise for this, it's definitely > your fix. Looks good (rather, makes the original look obviously broken and makes me wonder why our review process did not catch that bogus NULL in the first place). Thanks, both. -- To unsubscribe from this list: send the line "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.7.3 no longer applies relative symbolic link patches
Junio C Hamano writes: > Linus Torvalds writes: > >> Ugh. I don't see anything we can do about this on the git side, and I >> do kind of understand why 'patch' would be worried about '..' files. >> In a perfect world, patch would parse the filename and see that it >> stays within the directory structure of the project, but that is a >> rather harder thing to do than just say "no dot-dot files". > > It is unclear to me why "limit to the current directory and below" > is such a big deal in the first place. > > If the user wants to apply a patch that touches ../etc/shadow, is > the tool in the place to complain?" Let me take this part back. I think "git apply" should behave closely to "git apply --index" (which is used by "git am" unless there is a very good reason not to (and "'git apply --index' behaves differently from GNU patch, and we should match what the latter does" is not a very good reason). When the index guards the working tree, we do not follow any symlink, whether the destination is inside the current directory or not. I however do not think the current "git apply" notices that it will overwrite a path beyond a symlink---we may need to fix that if that is the case. I'll see what I can find (but I'll be doing 2.3-rc2 today so it may be later this week). 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 09/21] list-files: a user friendly version of ls-files and more
Nguyễn Thái Ngọc Duy writes: > +color.list-files:: > + A boolean to enable/disable color in the output of > + linkgit:git-list-files[1]. May be set to `always`, `false` (or > + `never`) or `auto` (or `true`), in which case colors are used > + only when the output is to a terminal. Defaults to false. This violates the configuration variable naming rules; perhaps rename it to color.listFiles or something? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dumb-http: do not pass NULL path to parse_pack_index
On Tue, Jan 27, 2015 at 03:02:27PM -0500, Jeff King wrote: > Discovery and tests by Charles Bailey . > > Signed-off-by: Jeff King > --- > I'm happy to flip the authorship on this. You have more lines in it than > I do. :) No, I'm happy with you taking the blame/praise for this, it's definitely your fix. -- To unsubscribe from this list: send the line "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] Documentation/git-add.txt: add `add.ginore-errors` configuration variable
Eric Sunshine writes: > On Mon, Jan 26, 2015 at 11:55 AM, Alexander Kuleshov > wrote: >> 'git add' supports not only `add.ignoreErrors`, but also `add.ignore-errors` >> configuration variable. > > See 6b3020a2 (add: introduce add.ignoreerrors synonym for > add.ignore-errors, 2010-12-01) for why this patch is undesirable. We might want to do this instead, as the correction to the misspelled configuration variable name was from the 1.7.0-era that is sufficiently old (v1.7.0.8 was done in December 2010). -- >8 -- Subject: doc: clarify naming rules for configuration variables Also mark deprecated variables in the documentation more clearly. The text for the rules are partly taken from the log message of Jonathan's 6b3020a2 (add: introduce add.ignoreerrors synonym for add.ignore-errors, 2010-12-01). Signed-off-by: Junio C Hamano --- Documentation/CodingGuidelines | 25 + Documentation/config.txt | 15 +++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 894546d..8fbac15 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -413,6 +413,31 @@ Error Messages - Say what the error is first ("cannot open %s", not "%s: cannot open") +Externally Visible Names + + - For configuration variable names, follow the existing convention: + + . The section name indicates the affected subsystem. + + . The subsection name, if any, indicates which of an unbounded set + of things to set the value for. + + . The variable name describes the effect of tweaking this knob. + + The section and variable names that consist of multiple words are + formed by concatenating the words without punctuations (e.g. `-`), + and are broken using bumpyCaps in documentation as a hint to the + reader. + + When choosing the variable namespace, do not use variable name for + specifying possibly unbounded set of things, most notably anything + an end user can freely come up with (e.g. branch names), but also + large fixed set defined by the system that can grow over time + (e.g. what kind of common whitespace problems to notice). Use + subsection names or variable values, like existing variables + branch..description and core.whitespace do, instead. + + Writing Documentation: Most (if not all) of the documentation pages are written in the diff --git a/Documentation/config.txt b/Documentation/config.txt index 0a4d22a..8a76d1d 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -664,14 +664,13 @@ core.abbrev:: for abbreviated object names to stay unique for sufficiently long time. -add.ignore-errors:: add.ignoreErrors:: +add.ignore-errors (deprecated):: Tells 'git add' to continue adding files when some files cannot be added due to indexing errors. Equivalent to the '--ignore-errors' - option of linkgit:git-add[1]. Older versions of Git accept only - `add.ignore-errors`, which does not follow the usual naming - convention for configuration variables. Newer versions of Git - honor `add.ignoreErrors` as well. + option of linkgit:git-add[1]. `add.ignore-errors` is deprecated, + as it does not follow the usual naming convention for configuration + variables. alias.*:: Command aliases for the linkgit:git[1] command wrapper - e.g. @@ -1924,7 +1923,7 @@ pack.useBitmaps:: true. You should not generally need to turn this off unless you are debugging pack bitmaps. -pack.writebitmaps:: +pack.writebitmaps (deprecated):: This is a deprecated synonym for `repack.writeBitmaps`. pack.writeBitmapHashCache:: @@ -2235,7 +2234,7 @@ sendemail.smtpencryption:: See linkgit:git-send-email[1] for description. Note that this setting is not subject to the 'identity' mechanism. -sendemail.smtpssl:: +sendemail.smtpssl (deprecated):: Deprecated alias for 'sendemail.smtpencryption = ssl'. sendemail.smtpsslcertpath:: @@ -2273,7 +2272,7 @@ sendemail.thread:: sendemail.validate:: See linkgit:git-send-email[1] for description. -sendemail.signedoffcc:: +sendemail.signedoffcc (deprecated):: Deprecated alias for 'sendemail.signedoffbycc'. showbranch.default:: -- To unsubscribe from this list: send the line "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] dumb-http: do not pass NULL path to parse_pack_index
On Tue, Jan 27, 2015 at 01:12:20PM -0500, Jeff King wrote: > It looks like the culprit is 7b64469 (Allow parse_pack_index on > temporary files, 2010-04-19). It added a new "idx_path" parameter to > parse_pack_index, which we pass as NULL. That causes its call to > check_packed_git_idx to fail (because it has no idea what file we are > talking about!). This is not quite right. That refactoring commit correctly passes the result of sha1_pack_index_name() for the newly-added parameter. It's the _next_ commit which then erroneously passes NULL. :) Here's my fix with a proper commit message. I tweaked the title on your test, as I didn't find the original very descriptive. I also bumped the call to sha1_pack_index_name() into the caller, as that makes more sense to me after reading the older commits (and there are literally no other callers outside of this function). -- >8 -- Once upon a time, dumb http always fetched .idx files directly into their final location, and then checked their validity with parse_pack_index. This was refactored in commit 750ef42 (http-fetch: Use temporary files for pack-*.idx until verified, 2010-04-19), which uses the following logic: 1. If we have the idx already in place, see if it's valid (using parse_pack_index). If so, use it. 2. Otherwise, fetch the .idx to a tempfile, check that, and if so move it into place. 3. Either way, fetch the pack itself if necessary. However, it got step 1 wrong. We pass a NULL path parameter to parse_pack_index, so an existing .idx file always looks broken. Worse, we do not treat this broken .idx as an opportunity to re-fetch, but instead return an error, ignoring the pack entirely. This can lead to a dumb-http fetch failing to retrieve the necessary objects. This doesn't come up much in practice, because it must be a packfile that we found out about (and whose .idx we stored) during an earlier dumb-http fetch, but whose packfile we _didn't_ fetch. I.e., we did a partial clone of a repository, didn't need some packfiles, and now a followup fetch needs them. Discovery and tests by Charles Bailey . Signed-off-by: Jeff King --- I'm happy to flip the authorship on this. You have more lines in it than I do. :) As I mentioned before, I think we could treat a broken .idx file differently (by deleting and refetching), but I doubt it matters in practice. The only reason this came up at all is that our test for "broken" was bogus, and it may be better to let a user diagnose and deal with breakage. http.c | 2 +- t/t5550-http-fetch-dumb.sh | 18 ++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index 040f362..f2373c0 100644 --- a/http.c +++ b/http.c @@ -1240,7 +1240,7 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head, int ret; if (has_pack_index(sha1)) { - new_pack = parse_pack_index(sha1, NULL); + new_pack = parse_pack_index(sha1, sha1_pack_index_name(sha1)); if (!new_pack) return -1; /* parse_pack_index() already issued error message */ goto add_pack; diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index ac71418..6da9422 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -165,6 +165,24 @@ test_expect_success 'fetch notices corrupt idx' ' ) ' +test_expect_success 'fetch can handle previously-fetched .idx files' ' + git checkout --orphan branch1 && + echo base >file && + git add file && + git commit -m base && + git --bare init "$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git && + git push "$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git branch1 && + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git repack -d && + git checkout -b branch2 branch1 && + echo b2 >>file && + git commit -a -m b2 && + git push "$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git branch2 && + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git repack -d && + git --bare init clone_packed_branches.git && + git --git-dir=clone_packed_branches.git fetch "$HTTPD_URL"/dumb/repo_packed_branches.git branch1:branch1 && + git --git-dir=clone_packed_branches.git fetch "$HTTPD_URL"/dumb/repo_packed_branches.git branch2:branch2 +' + test_expect_success 'did not use upload-pack service' ' grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act : >exp -- 2.3.0.rc1.287.g761fd19 -- To unsubscribe from this list: send the line "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] test-lib.sh: Dynamic test for the prerequisite SANITY
Minor typo in a comment. On Wed, Jan 28, 2015 at 4:39 AM, Torsten Bögershausen wrote: > The SANITY precondition was not set when running as root, > but this is not 100% reliable for CYGWIN: > > A file may be allowed to be deleted when the containing > directory does not have write permissions. > > See > https://technet.microsoft.com/en-us/library/bb463216.aspx > "...In UNIX, the write permission bit on a directory permits both > the creation and removal of new files or sub-directories in the directory. > On Windows, the Write_Data access right controls the creation of new > sub-files and the Delete_Child access right controls the deletion. " > > We may argue that the translation of the POSIX write permission bit > into "Windows access rights" can be improved in CYGWIN. > > A dynamic test handles all cases: when the sequence > $ mkdir SANETESTD && > $ chmod +w SANETESTD && > $ >SANETESTD/x && > $ ! rm SANETESTD/x > succeeds the prerequisite SANITY is true. > > Helped-by: Junio C Hamano > Signed-off-by: Torsten Bögershausen > --- > t/test-lib.sh | 17 - > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 93f7cad..887e986 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1038,8 +1038,23 @@ test_lazy_prereq NOT_ROOT ' > > # When the tests are run as root, permission tests will report that > # things are writable when they shouldn't be. > +# Special check for CYGWIN (or Windows in general): > +# A file can be deleted, even if the containing directory does'nt s/does'nt/doesn't/ > +# have write permissions > test_lazy_prereq SANITY ' > - test_have_prereq POSIXPERM,NOT_ROOT > + mkdir SANETESTD && > + chmod +w SANETESTD && > + >SANETESTD/x && > + chmod -w SANETESTD || > + error "bug in test sript: cannot prepare SANETESTD" > + > + ! rm SANETESTD/x > + status=$? > + > + chmod +w SANETESTD && > + rm -rf SANETESTD || > + error "bug in test sript: cannot clean SANETESTD" > + return $status > ' > > GIT_UNZIP=${GIT_UNZIP:-unzip} > -- > 2.2.0.rc1.26.g3e3a70d > > -- > To unsubscribe from this list: send the line "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: [PATCH] git-push.txt: document the behavior of --repo
Michael J Gruber writes: > As per the code, the --repo option is equivalent to the > argument to 'git push'. [It exists for historical reasons, back from the time > when options had to come before arguments.] > > Say so. [But not that.] > > Signed-off-by: Michael J Gruber > --- > Thanks for digging up the thread, Junio. I never would have thought that > I had been with the Git community for that long already... ;-) I think this update will do for now, but in the medium term (read: by the end of this year, or earlier if somebody is motivated enough), we might want to: * deprecate --repo= as it is very much no-op these days (that is, strike "But not that" part above); * dig deeper what Prem wanted out of their imagined semantics of the --repo= option. I suspect that it has something to do with support of triangular workflow, and - it might turn out that there is a better way to do what Prem wanted to do without that option but using other existing mechanisms [*1*], in which case we can stop there on the code side, and clarify how to use those other existing mechanisms in the tutorial. - or it may be that we do not have a good way to achieve what Prem wanted to do, and that a *new* option to specify the target URL from the command line, like Prem used the --repo option may turn out to be the best way forward [*2*], in which case a code update may become necessary. Thanks. [Footnotes] *1* For example, in 1.8.3 we saw some changes around triangular "pull from one place, push to another place" workflow with remote.pushdefault configuration, and branch.*.pushremote lets the users control this even at a branch level. *2* I say "may turn out to be" because we cannot tell if that is the best solution until we know what was really what Prem wanted to do---we may be looking at an XY problem after all. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add failing test for fetching from multiple packs over dumb httpd
On Tue, Jan 27, 2015 at 01:12:21PM -0500, Jeff King wrote: > On Tue, Jan 27, 2015 at 03:20:41PM +, Charles Bailey wrote: > > > From: Charles Bailey > > > > When objects are spread across multiple packs, if an initial fetch does > > require all pack files, a subsequent fetch for objects in packs not > > retrieved in the initial fetch will fail. > > s/does/does not/, I think? Yes, that's definitely what I meant to write. [...] > It looks like the culprit is 7b64469 (Allow parse_pack_index on > temporary files, 2010-04-19). It added a new "idx_path" parameter to > parse_pack_index, which we pass as NULL. That causes its call to > check_packed_git_idx to fail (because it has no idea what file we are > talking about!). That change looks like it went into 1.7.1.1. I cannot confirm this working before then but we've definitely seen the bug in 1.7.12.3 and more recent versions. > This seems to fix it: > > diff --git a/sha1_file.c b/sha1_file.c > index 30995e6..eda4d90 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -1149,6 +1149,9 @@ struct packed_git *parse_pack_index(unsigned char > *sha1, const char *idx_path) > const char *path = sha1_pack_name(sha1); > struct packed_git *p = alloc_packed_git(strlen(path) + 1); > > + if (!idx_path) > + idx_path = sha1_pack_index_name(sha1); > + > strcpy(p->pack_name, path); > hashcpy(p->sha1, sha1); > if (check_packed_git_idx(idx_path, p)) { It certainly fixes my test script and I can give this patch a test in the 'real' world. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add failing test for fetching from multiple packs over dumb httpd
On Tue, Jan 27, 2015 at 03:20:41PM +, Charles Bailey wrote: > From: Charles Bailey > > When objects are spread across multiple packs, if an initial fetch does > require all pack files, a subsequent fetch for objects in packs not > retrieved in the initial fetch will fail. s/does/does not/, I think? > I'm not very familiar with the http client code so this analysis is based > purely on observed behaviour. Debugging the http code is a royal pain because all the work happens in a separate helper. I use a git-remote-debug script like this: #!/bin/sh host=localhost:5001 proto=$(echo "${2:-$1}" | sed 's/:.*//') prog=git-remote-$proto echo >&2 "gdb -ex 'target remote $host' $prog" gdbserver localhost:5001 "$prog" "$@" and then you can use: git fetch debug::http://... in the test script, cut-and-paste the gdb command printed to stderr, and you're dropped into the appropriate debugger without worrying about all of the stdio mess. > When fetching only some refs from a repository served over dumb httpd Git > appears to download all of the index files for the available packs but then > only chooses the pack files that help it resolve the objects which we need. Right. And it looks like we have special code in sha1_file.c to make sure we do not trust an index which does not have a matching packfile. So that's good. The http-walker code does its own check, in fetch_and_setup_pack_index, that checks for an existing valid copy of the index. If we don't have it, we download the index and proceed. If we do, we skip straight to grabbing the pack. But if we have it and it doesn't appear valid, we return an error. And there seems to be a bug with checking the validity. It looks like the culprit is 7b64469 (Allow parse_pack_index on temporary files, 2010-04-19). It added a new "idx_path" parameter to parse_pack_index, which we pass as NULL. That causes its call to check_packed_git_idx to fail (because it has no idea what file we are talking about!). This seems to fix it: diff --git a/sha1_file.c b/sha1_file.c index 30995e6..eda4d90 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1149,6 +1149,9 @@ struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path) const char *path = sha1_pack_name(sha1); struct packed_git *p = alloc_packed_git(strlen(path) + 1); + if (!idx_path) + idx_path = sha1_pack_index_name(sha1); + strcpy(p->pack_name, path); hashcpy(p->sha1, sha1); if (check_packed_git_idx(idx_path, p)) { (Alternatively, we could pass in sha1_pack_index_name instead of NULL in the first place, but I think it is reasonable for parse_pack_index to take care of this). I think it may also make sense for fetch_and_setup_pack_index to delete and re-download a broken .idx file (rather than aborting), but I don't think that's a big deal. It should only happen in the face of on-disk data corruption, and the user can remove the broken .idx themselves. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-new-workdir: support submodules
Am 26.01.2015 um 06:39 schrieb Junio C Hamano: Craig Silverstein writes: This patch, in adding submodule support, I expect would be similarly useful to people even though it, also, imposes those same constraints to the submodule's config files. I would expect that you would see exactly the same issue with Duy's multiple work tree series. This is not limited to new-workdir. The right way to look at this is to fix what "git submodule" does; its use of "config" that is shared across branches is the root cause of the trouble. No other part of Git keeps data that needs to be per-branch (or more specifically "tied to the working tree state") in .git/config in such a way that leaving it stale when the working tree state changes breaks the system. I'm not sure what submodule config shared across branches you are referring to here, the only two things that submodules by default store in .git/config are the url (to mark the submodule initialized) and the update setting (which I believe should be copied there in the first place, but changing that in a correct and backwards compatible would take some effort). This means you'll init (or deinit) a submodule in all multiple worktrees at the same time, but having the url setting in .git/config is a feature, not a bug. And if you are talking about the core.worktree setting in .git/modules//config, I proposed way to get rid of that when we moved the submodule .git directory there (but that wasn't accepted): http://permalink.gmane.org/gmane.comp.version-control.git/188007 Additionally I suspect submodules are just the first to suffer, other worktree specific config (like e.g. EOL settings) affecting all multiple worktrees at the same time when changed in a single worktree might easily confuse other users too. One way to do so might be to move the bits it stores in the config file to somewhere else that is more closely tied to the checkout state and handle that similar to .git/index and .git/HEAD when it comes to multiple work-trees. Yup, the idea of separating config entries into worktree and repo specific files sounds like the solution for these problems. -- To unsubscribe from this list: send the line "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] INSTALL: minor typo fix
On Wed, Jan 28, 2015 at 06:09:52AM +1300, Chris Packham wrote: > On Wed, Jan 28, 2015 at 5:15 AM, Alexander Kuleshov > wrote: > > Signed-off-by: Alexander Kuleshov > > --- > > INSTALL | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/INSTALL b/INSTALL > > index ffb071e..6f1c3d5 100644 > > --- a/INSTALL > > +++ b/INSTALL > > @@ -53,7 +53,7 @@ or > > > > As a caveat: a profile-optimized build takes a *lot* longer since the > > git tree must be built twice, and in order for the profiling > > -measurements to work properly, ccache must be disabled and the test > > +measurements to work properly, cache must be disabled and the test > > suite has to be run using only a single CPU. In addition, the profile > > feedback build stage currently generates a lot of additional compiler > > warnings. > > -- > > That's not a typo ccache[1] is a compiler cache tool that can be used > to speed up rebuilding object files. I don't know anything about the > profile enabled builds but I imagine it has something to do with > needing the 2nd build to produce difference object files from the > first (with ccache enabled I imagine the 2nd build would result in > identical output since on source files are changed between builds). Yes, ccache creates a cache key for each file based on the command line flags to the compiler, and the output of the pre-processor when run on the file. The profiling data generated by the first run is outside of what ccache knows about, so it can't include that information in its cache key (and consequently, we get false cache hits). So yeah. Not a typo. -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] INSTALL: minor typo fix
On Wed, Jan 28, 2015 at 6:09 AM, Chris Packham wrote: > Hi Alexander > > On Wed, Jan 28, 2015 at 5:15 AM, Alexander Kuleshov > wrote: >> Signed-off-by: Alexander Kuleshov >> --- >> INSTALL | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/INSTALL b/INSTALL >> index ffb071e..6f1c3d5 100644 >> --- a/INSTALL >> +++ b/INSTALL >> @@ -53,7 +53,7 @@ or >> >> As a caveat: a profile-optimized build takes a *lot* longer since the >> git tree must be built twice, and in order for the profiling >> -measurements to work properly, ccache must be disabled and the test >> +measurements to work properly, cache must be disabled and the test >> suite has to be run using only a single CPU. In addition, the profile >> feedback build stage currently generates a lot of additional compiler >> warnings. >> -- > > That's not a typo ccache[1] is a compiler cache tool that can be used > to speed up rebuilding object files. I don't know anything about the > profile enabled builds but I imagine it has something to do with > needing the 2nd build to produce difference object files from the s/difference/different/ > first (with ccache enabled I imagine the 2nd build would result in > identical output since on source files are changed between builds). > > -- > [1] - https://ccache.samba.org/ The commit that added that note f2d713fc[1] has a bit more of an explanation of why profile enabled builds and ccache are incompatible. [1] - https://github.com/git/git/commit/f2d713fc3e8e0b7be89584f04b421808aa99c403 -- To unsubscribe from this list: send the line "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] INSTALL: minor typo fix
Hi Alexander On Wed, Jan 28, 2015 at 5:15 AM, Alexander Kuleshov wrote: > Signed-off-by: Alexander Kuleshov > --- > INSTALL | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/INSTALL b/INSTALL > index ffb071e..6f1c3d5 100644 > --- a/INSTALL > +++ b/INSTALL > @@ -53,7 +53,7 @@ or > > As a caveat: a profile-optimized build takes a *lot* longer since the > git tree must be built twice, and in order for the profiling > -measurements to work properly, ccache must be disabled and the test > +measurements to work properly, cache must be disabled and the test > suite has to be run using only a single CPU. In addition, the profile > feedback build stage currently generates a lot of additional compiler > warnings. > -- That's not a typo ccache[1] is a compiler cache tool that can be used to speed up rebuilding object files. I don't know anything about the profile enabled builds but I imagine it has something to do with needing the 2nd build to produce difference object files from the first (with ccache enabled I imagine the 2nd build would result in identical output since on source files are changed between builds). -- [1] - https://ccache.samba.org/ -- To unsubscribe from this list: send the line "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: t5539 broken under Mac OS X
On Tue, Jan 27, 2015 at 3:51 AM, Junio C Hamano wrote: > Erik Faye-Lund writes: > >> On Fri, Jan 16, 2015 at 1:04 AM, Junio C Hamano wrote: >>> Jeff King writes: >>> Exactly. I am happy to submit a patch, but I cannot think of any mechanisms besides: 1. Calling `id`, which I suspect is very not portable. 2. Writing a C program to check getuid(). That's portable for most Unixes. It looks like we already have a hacky wrapper on mingw that will always return "1". Is (2) too gross? >>> >>> Not overly gross compared to some existing test-*.c files, I would >>> say. >>> >>> I wondered what 'perl -e 'print $>' would say in mingw, and if that >>> is portable enough, though. >> >> $ perl -e 'print $>' >> 500 > > Thanks for a follow-up. > > Is "id -u" not useful over there? I ask because that is what is > used in the version tentatively queued on 'pu' for NOT_ROOT > prerequisite (the jk/sanity topic). It's pretty much the same thing: $ id -u 500 > The SANITY prerequisite in that topic needs to be replaced with the > one from Torsten that attempts to check what we want to know in a > more direct way; i.e. "after making a directory or a file read-only, > does the filesystem really honours that, or lets us clobber?" is > what we need to know to skip some tests, and we should check that, > instead of "is / writable by us?" or "are we root?". $ test -w / && echo yes yes $ mkdir foo && chmod a= foo $ test -w w && echo yes $ rm -r foo rm: directory `foo' is write protected; descend into it anyway? n $ rm -r foo < /dev/null $ ls -la foo ls: foo: No such file or directory $ So, Windows does only kind-of respect read-only flags. Dunno if this tells you something useful, though. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] INSTALL: minor typo fix
Signed-off-by: Alexander Kuleshov --- INSTALL | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/INSTALL b/INSTALL index ffb071e..6f1c3d5 100644 --- a/INSTALL +++ b/INSTALL @@ -53,7 +53,7 @@ or As a caveat: a profile-optimized build takes a *lot* longer since the git tree must be built twice, and in order for the profiling -measurements to work properly, ccache must be disabled and the test +measurements to work properly, cache must be disabled and the test suite has to be run using only a single CPU. In addition, the profile feedback build stage currently generates a lot of additional compiler warnings. -- 2.3.0-rc1 -- To unsubscribe from this list: send the line "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] http: Add Accept-Language header if possible
From: Yi EungJun Add an Accept-Language header which indicates the user's preferred languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG. Examples: LANGUAGE= -> "" LANGUAGE=ko:en -> "Accept-Language: ko, en;q=0.9, *;q=0.1" LANGUAGE=ko LANG=en_US.UTF-8 -> "Accept-Language: ko, *;q=0.1" LANGUAGE= LANG=en_US.UTF-8 -> "Accept-Language: en-US, *;q=0.1" This gives git servers a chance to display remote error messages in the user's preferred language. Limit the number of languages to 1,000 because q-value must not be smaller than 0.001, and limit the length of Accept-Language header to 4,000 bytes for some HTTP servers which cannot accept such long header. Signed-off-by: Yi EungJun --- http.c | 151 + remote-curl.c | 2 + t/t5550-http-fetch-dumb.sh | 42 + 3 files changed, 195 insertions(+) diff --git a/http.c b/http.c index 040f362..6111c6a 100644 --- a/http.c +++ b/http.c @@ -68,6 +68,8 @@ static struct curl_slist *no_pragma_header; static struct active_request_slot *active_queue_head; +static char *cached_accept_language; + size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_) { size_t size = eltsize * nmemb; @@ -515,6 +517,9 @@ void http_cleanup(void) cert_auth.password = NULL; } ssl_cert_password_required = 0; + + free(cached_accept_language); + cached_accept_language = NULL; } struct active_request_slot *get_active_slot(void) @@ -986,6 +991,146 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type, strbuf_addstr(charset, "ISO-8859-1"); } +/* + * Guess the user's preferred languages from the value in LANGUAGE environment + * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined. + * + * The result can be a colon-separated list like "ko:ja:en". + */ +static const char *get_preferred_languages(void) +{ + const char *retval; + + retval = getenv("LANGUAGE"); + if (retval && *retval) + return retval; + +#ifndef NO_GETTEXT + retval = setlocale(LC_MESSAGES, NULL); + if (retval && *retval && + strcmp(retval, "C") && + strcmp(retval, "POSIX")) + return retval; +#endif + + return NULL; +} + +static void write_accept_language(struct strbuf *buf) +{ + /* +* MAX_DECIMAL_PLACES must not be larger than 3. If it is larger than +* that, q-value will be smaller than 0.001, the minimum q-value the +* HTTP specification allows. See +* http://tools.ietf.org/html/rfc7231#section-5.3.1 for q-value. +*/ + const int MAX_DECIMAL_PLACES = 3; + const int MAX_LANGUAGE_TAGS = 1000; + const int MAX_ACCEPT_LANGUAGE_HEADER_SIZE = 4000; + struct strbuf *language_tags = NULL; + int num_langs = 0; + const char *s = get_preferred_languages(); + int i; + struct strbuf tag = STRBUF_INIT; + + /* Don't add Accept-Language header if no language is preferred. */ + if (!s) + return; + + /* +* Split the colon-separated string of preferred languages into +* language_tags array. +*/ + do { + /* collect language tag */ + for (; *s && (isalnum(*s) || *s == '_'); s++) + strbuf_addch(&tag, *s == '_' ? '-' : *s); + + /* skip .codeset, @modifier and any other unnecessary parts */ + while (*s && *s != ':') + s++; + + if (tag.len) { + num_langs++; + REALLOC_ARRAY(language_tags, num_langs); + strbuf_init(&language_tags[num_langs - 1], 0); + strbuf_swap(&tag, &language_tags[num_langs - 1]); + + if (num_langs >= MAX_LANGUAGE_TAGS - 1) /* -1 for '*' */ + break; + } + } while (*s++); + + /* write Accept-Language header into buf */ + if (num_langs >= 1) { + int last_buf_len = 0; + int max_q; + int decimal_places; + char q_format[32]; + + /* add '*' */ + REALLOC_ARRAY(language_tags, num_langs + 1); + strbuf_init(&language_tags[num_langs], 0); + strbuf_addstr(&language_tags[num_langs++], "*"); + + /* compute decimal_places */ + for (max_q = 1, decimal_places = 0; + max_q < num_langs && decimal_places <= MAX_DECIMAL_PLACES; + decimal_places++, max_q *= 10) + ; + + sprintf(q_format, ";q=0.%%0%dd", decimal_places); + + strbuf_addstr(buf, "Accept-Language: "); + + for(i = 0; i < num_langs; i++) { + if (i > 0) +
[PATCH v8 0/1] http: Add Accept-Language header if possible
From: Yi EungJun Change since v7 >From Torsten Bögershausen's review: * remove unnecessary if-statement >From Eric Sunshine's review: * fix memory leaks and uninitialized variables * remove unnecessary if-statement >From Junio C Hamano's review: * fix uninitialized variables Yi EungJun (1): http: Add Accept-Language header if possible http.c | 151 + remote-curl.c | 2 + t/t5550-http-fetch-dumb.sh | 42 + 3 files changed, 195 insertions(+) -- 2.3.0.rc1.32.ga3df1c7 -- To unsubscribe from this list: send the line "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] test-lib.sh: Dynamic test for the prerequisite SANITY
The SANITY precondition was not set when running as root, but this is not 100% reliable for CYGWIN: A file may be allowed to be deleted when the containing directory does not have write permissions. See https://technet.microsoft.com/en-us/library/bb463216.aspx "...In UNIX, the write permission bit on a directory permits both the creation and removal of new files or sub-directories in the directory. On Windows, the Write_Data access right controls the creation of new sub-files and the Delete_Child access right controls the deletion. " We may argue that the translation of the POSIX write permission bit into "Windows access rights" can be improved in CYGWIN. A dynamic test handles all cases: when the sequence $ mkdir SANETESTD && $ chmod +w SANETESTD && $ >SANETESTD/x && $ ! rm SANETESTD/x succeeds the prerequisite SANITY is true. Helped-by: Junio C Hamano Signed-off-by: Torsten Bögershausen --- t/test-lib.sh | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 93f7cad..887e986 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1038,8 +1038,23 @@ test_lazy_prereq NOT_ROOT ' # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. +# Special check for CYGWIN (or Windows in general): +# A file can be deleted, even if the containing directory does'nt +# have write permissions test_lazy_prereq SANITY ' - test_have_prereq POSIXPERM,NOT_ROOT + mkdir SANETESTD && + chmod +w SANETESTD && + >SANETESTD/x && + chmod -w SANETESTD || + error "bug in test sript: cannot prepare SANETESTD" + + ! rm SANETESTD/x + status=$? + + chmod +w SANETESTD && + rm -rf SANETESTD || + error "bug in test sript: cannot clean SANETESTD" + return $status ' GIT_UNZIP=${GIT_UNZIP:-unzip} -- 2.2.0.rc1.26.g3e3a70d -- To unsubscribe from this list: send the line "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] t2026 needs procondition SANITY
When running t0026 as root 'prune directories with unreadable gitdir' fails. Skip this test if SANITY is not set (the use of POSIXPERM is wrong here) Signed-off-by: Torsten Bögershausen --- Here is the promised patch, on top of pu: The fix of t0026 is unrelated to all other patches, I think. The advantage of the series is that it has been tested on CYGWIN/MINGW/Mac/Linux/root@Mac, root@Linux, the disadvantage may be that the commit message may need improvements. t/t2026-prune-linked-checkouts.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh index 170aefe..2936d52 100755 --- a/t/t2026-prune-linked-checkouts.sh +++ b/t/t2026-prune-linked-checkouts.sh @@ -33,7 +33,7 @@ EOF ! test -d .git/worktrees ' -test_expect_success POSIXPERM 'prune directories with unreadable gitdir' ' +test_expect_success SANITY 'prune directories with unreadable gitdir' ' mkdir -p .git/worktrees/def/abc && : >.git/worktrees/def/def && : >.git/worktrees/def/gitdir && -- 2.2.0.rc1.26.g3e3a70d -- To unsubscribe from this list: send the line "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.7.3 no longer applies relative symbolic link patches
On Mon, 26 Jan 2015 12:44:33 -0800, Linus Torvalds wrote: > I've considered that for a while already, because "patch" _does_ kind of > understand them these days, although I think it gets the cross-rename > case wrong because it fundamentally works on a file-by-file basis. Patch handles cross-renames correctly nowadays; if not, it's a bug. That's not enough to solve the symlink problem unfortunately. Andreas -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Add failing test for fetching from multiple packs over dumb httpd
From: Charles Bailey When objects are spread across multiple packs, if an initial fetch does require all pack files, a subsequent fetch for objects in packs not retrieved in the initial fetch will fail. --- I'm not very familiar with the http client code so this analysis is based purely on observed behaviour. When fetching only some refs from a repository served over dumb httpd Git appears to download all of the index files for the available packs but then only chooses the pack files that help it resolve the objects which we need. If we then later try to fetch an object which is in a pack file for which Git has previously downloaded an index file, it seems to trip because it believes it already has the object locally due to the presence of the index file but doesn't actually have it because it never retrieved the corresponding pack file. It reports an error of the form "Cannot obtain needed object ...". Manually deleting index files which have no corresponding local pack file will allow a repeat of the failed fetch to succeed. t/t5550-http-fetch-dumb.sh | 18 ++ 1 file changed, 18 insertions(+) diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index ac71418..cf2362a 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -165,6 +165,24 @@ test_expect_success 'fetch notices corrupt idx' ' ) ' +test_expect_failure 'fetch packed branches' ' + git checkout --orphan branch1 && + echo base >file && + git add file && + git commit -m base && + git --bare init "$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git && + git push "$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git branch1 && + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git repack -d && + git checkout -b branch2 branch1 && + echo b2 >>file && + git commit -a -m b2 && + git push "$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git branch2 && + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH"/repo_packed_branches.git repack -d && + git --bare init clone_packed_branches.git && + git --git-dir=clone_packed_branches.git fetch "$HTTPD_URL"/dumb/repo_packed_branches.git branch1:branch1 && + git --git-dir=clone_packed_branches.git fetch "$HTTPD_URL"/dumb/repo_packed_branches.git branch2:branch2 +' + test_expect_success 'did not use upload-pack service' ' grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act : >exp -- 2.0.2.611.g8c85416 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: bug report
On Tue, Jan 27, 2015 at 02:43:31PM +, Albert Akhriev wrote: > My system: RedHat 6.5/64-bit (gcc 4.4.7) > My configuration options: ./configure --prefix=/home/albert/soft > > Compilation of git 2.2.2 itself was fine, but some tests failed. > I presume, there might be some network communication restrictions here in > the lab > (gitweb had failed as can be seen in the listing below): The gitweb tests should run locally and not need to touch the network. It looks like gitweb cannot run at all. Are you sure you have a working perl (with CGI and other base modules)? Try running: cd t ./t9500-gitweb-standaloneno-errors.sh -v -i which should give more output. Also check the contents of gitweb.log in "t/trash directory/t9500-...". -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
bug report
Hi All, My system: RedHat 6.5/64-bit (gcc 4.4.7) My configuration options: ./configure --prefix=/home/albert/soft Compilation of git 2.2.2 itself was fine, but some tests failed. I presume, there might be some network communication restrictions here in the lab (gitweb had failed as can be seen in the listing below): 1..14 *** t9400-git-cvsserver-server.sh *** 1..0 # SKIP skipping git-cvsserver tests, Perl SQLite interface unavailable *** t9401-git-cvsserver-crlf.sh *** 1..0 # SKIP skipping git-cvsserver tests, Perl SQLite interface unavailable *** t9402-git-cvsserver-refs.sh *** 1..0 # SKIP skipping git-cvsserver tests, Perl SQLite interface unavailable *** t9500-gitweb-standalone-no-errors.sh *** not ok 1 - no commits: projects_list (implicit) # gitweb_run not ok 2 - no commits: projects_index # gitweb_run "a=project_index" not ok 3 - no commits: .git summary (implicit) # gitweb_run "p=.git" not ok 4 - no commits: .git commit (implicit HEAD) # gitweb_run "p=.git;a=commit" not ok 5 - no commits: .git commitdiff (implicit HEAD) # gitweb_run "p=.git;a=commitdiff" not ok 6 - no commits: .git tree (implicit HEAD) # gitweb_run "p=.git;a=tree" not ok 7 - no commits: .git heads # gitweb_run "p=.git;a=heads" not ok 8 - no commits: .git tags # gitweb_run "p=.git;a=tags" ok 9 - Make initial commit not ok 10 - projects_list (implicit) # gitweb_run not ok 11 - projects_index # gitweb_run "a=project_index" not ok 12 - .git summary (implicit) # gitweb_run "p=.git" not ok 13 - .git commit (implicit HEAD) # gitweb_run "p=.git;a=commit" not ok 14 - .git commitdiff (implicit HEAD, root commit) # gitweb_run "p=.git;a=commitdiff" not ok 15 - .git commitdiff_plain (implicit HEAD, root commit) # gitweb_run "p=.git;a=commitdiff_plain" not ok 16 - .git commit (HEAD) # gitweb_run "p=.git;a=commit;h=HEAD" not ok 17 - .git tree (implicit HEAD) # gitweb_run "p=.git;a=tree" not ok 18 - .git blob (file) # gitweb_run "p=.git;a=blob;f=file" not ok 19 - .git blob_plain (file) # gitweb_run "p=.git;a=blob_plain;f=file" not ok 20 - .git commit (non-existent) # gitweb_run "p=.git;a=commit;h=non-existent" not ok 21 - .git commitdiff (non-existent) # gitweb_run "p=.git;a=commitdiff;h=non-existent" not ok 22 - .git commitdiff (non-existent vs HEAD) # gitweb_run "p=.git;a=commitdiff;hp=non-existent;h=HEAD" not ok 23 - .git tree () # gitweb_run "p=.git;a=tree;h=" not ok 24 - .git tag () # gitweb_run "p=.git;a=tag;h=" not ok 25 - .git blob (non-existent) # gitweb_run "p=.git;a=blob;f=non-existent" not ok 26 - .git blob_plain (non-existent) # gitweb_run "p=.git;a=blob_plain;f=non-existent" not ok 27 - commitdiff(0): root # gitweb_run "p=.git;a=commitdiff" not ok 28 - commitdiff(0): file added # echo "New file" > new_file && #git add new_file && #git commit -a -m "File added." && #gitweb_run "p=.git;a=commitdiff" not ok 29 - commitdiff(0): mode change # test_chmod +x new_file && #git commit -a -m "Mode changed." && #gitweb_run "p=.git;a=commitdiff" not ok 30 - commitdiff(0): file renamed # git mv new_file renamed_file && #git commit -a -m "File renamed." && #gitweb_run "p=.git;a=commitdiff" not ok 31 - commitdiff(0): file to symlink # rm renamed_file && #test_ln_s_add file renamed_file && #git commit -a -m "File to symlink." && #gitweb_run "p=.git;a=commitdiff" not ok 32 - commitdiff(0): file deleted # git rm renamed_file && #rm -f renamed_file && #git commit -a -m "File removed." && #gitweb_run "p=.git;a=commitdiff" not ok 33 - commitdiff(0): file copied / new file # cp file file2 && #git add file2 && #git commit -a -m "File copied." && #gitweb_run "p=.git;a=commitdiff" not ok 34 - commitdiff(0): mode change and modified # echo "New line" >> file2 && #test_chmod +x file2 && #git commit -a -m "Mode change and modification." && #gitweb_run "p=.git;a=commitdiff" not ok 35 - commitdiff(0): renamed and modified # cat >file2<> file3 && #git commit -a -m "File rename and modification." && #gitweb_run "p=.git;a=commitdiff" not ok 36 - commitdiff(0): renamed, mode change and modified # git mv file3 file2 && #echo "Propter nomen suum." >> file2 && #test_chmod +x file2 && #git commit -a -m "File rename, mode change and modification." && #gitweb_run "p=.git;a=commitdiff" ok 37 - s
[PATCH] git-push.txt: document the behavior of --repo
As per the code, the --repo option is equivalent to the argument to 'git push'. [It exists for historical reasons, back from the time when options had to come before arguments.] Say so. [But not that.] Signed-off-by: Michael J Gruber --- Thanks for digging up the thread, Junio. I never would have thought that I had been with the Git community for that long already... Documentation/git-push.txt | 18 ++ 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index ea97576..0ad31c4 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -219,22 +219,8 @@ origin +master` to force a push to the `master` branch). See the `...` section above for details. --repo=:: - This option is only relevant if no argument is - passed in the invocation. In this case, 'git push' derives the - remote name from the current branch: If it tracks a remote - branch, then that remote repository is pushed to. Otherwise, - the name "origin" is used. For this latter case, this option - can be used to override the name "origin". In other words, - the difference between these two commands -+ --- -git push public #1 -git push --repo=public #2 --- -+ -is that #1 always pushes to "public" whereas #2 pushes to "public" -only if the current branch does not track a remote branch. This is -useful if you write an alias or script around 'git push'. + This option is equivalent to the argument; the latter + wins if both are specified. -u:: --set-upstream:: -- 2.3.0.rc1.222.gae238f2 -- To unsubscribe from this list: send the line "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: [PATCHv2] commit: reword --author error message
On 26/01/15 19:07, Jeff King wrote: > On Mon, Jan 26, 2015 at 04:48:33PM +0100, Michael J Gruber wrote: > >> -die(_("No existing author found with '%s'"), name); >> +die(_("--author '%s': neither 'Name ' nor a match for an >> existing author"), name); > > I had to add to the bikeshed, but I had to read this several times to > make sense of it. It is grammatically: > > X [is] neither Y nor Z > > except that by eliding the verb ("is"), I somehow had trouble making > sense of Z ("a match...") as a noun. > > I came up with: > > --author '%s': neither 'Name ' nor matches an existing author > > only to see that it was suggested earlier in the thread as a predecessor > to this. ;) > > I wonder if adding back in the missing verb, rather than a colon, would > also make more sense: > > --author '%s' is neither 'Name ' nor a match for an existing author > I usually don't like to add anything to the bikeshed, but ... This sounds odd to me, so maybe: --author '%s' is neither in the form of 'Name ' nor a match for an existing author although that is getting a bit long! :-D 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: implement a stable 'Last updated' in Documentation
On Mon, Jan 26, Olaf Hering wrote: > Several files in Documentation have an unstable 'Last updated' timestamp. The > reason is that their mtime changes every time, which prevents reproducible > builds. > > 341 technical/api-index.txt: technical/api-index-skel.txt \ > 342 technical/api-index.sh $(patsubst %,%.txt,$(API_DOCS)) > 343 $(QUIET_GEN)cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh > > 388 howto-index.txt: howto-index.sh $(wildcard howto/*.txt) > 389 $(QUIET_GEN)$(RM) $@+ $@ && \ > 390 '$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(wildcard > howto/*.txt)) >$@+ && \ > 391 mv $@+ $@ > > 399 $(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt > 400 $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ > 401 sed -e '1,/^$$/d' $< | \ > 402 $(TXT_TO_HTML) - >$@+ && \ > 403 mv $@+ $@ > > What file timestamp should be used for them? Likely "../version"? > The final file, before passing it to asciidoc, should get a fixed timestamp > with 'touch -r $reference_file $file'. I came up with this change. If anything happens to depend on howto-index.txt and technical/api-index.txt then this part needs rework. diff --git a/Documentation/Makefile b/Documentation/Makefile index 3e39e28..25ad0bd 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -6,6 +6,7 @@ TECH_DOCS = ARTICLES = SP_ARTICLES = OBSOLETE_HTML = +TIMESTAMP_FILE = ../version MAN1_TXT += $(filter-out \ $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \ @@ -341,6 +342,7 @@ user-manual.xml: user-manual.txt user-manual.conf technical/api-index.txt: technical/api-index-skel.txt \ technical/api-index.sh $(patsubst %,%.txt,$(API_DOCS)) $(QUIET_GEN)cd technical && '$(SHELL_PATH_SQ)' ./api-index.sh + touch -r '$(TIMESTAMP_FILE)' $@ technical/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../ $(patsubst %,%.html,$(API_DOCS) technical/api-index $(TECH_DOCS)): %.html : %.txt asciidoc.conf @@ -388,6 +390,7 @@ $(patsubst %.txt,%.texi,$(MAN_TXT)): %.texi : %.xml howto-index.txt: howto-index.sh $(wildcard howto/*.txt) $(QUIET_GEN)$(RM) $@+ $@ && \ '$(SHELL_PATH_SQ)' ./howto-index.sh $(sort $(wildcard howto/*.txt)) >$@+ && \ + touch -r '$(TIMESTAMP_FILE)' $@+ && \ mv $@+ $@ $(patsubst %,%.html,$(ARTICLES)) : %.html : %.txt @@ -397,9 +400,12 @@ WEBDOC_DEST = /pub/software/scm/git/docs howto/%.html: ASCIIDOC_EXTRA += -a git-relative-html-prefix=../ $(patsubst %.txt,%.html,$(wildcard howto/*.txt)): %.html : %.txt - $(QUIET_ASCIIDOC)$(RM) $@+ $@ && \ - sed -e '1,/^$$/d' $< | \ - $(TXT_TO_HTML) - >$@+ && \ + $(QUIET_ASCIIDOC)$(RM) $@++ $@+ $@ && \ + sed -e '1,/^$$/d' $< >$@++ && \ + touch -r '$(TIMESTAMP_FILE)' $@++ && \ + $(TXT_TO_HTML) -o $@+ $@++ && \ + $(RM) $@++ && \ + touch $@+ && \ mv $@+ $@ install-webdoc : html Olaf -- To unsubscribe from this list: send the line "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] Makefile: do not compile git with debugging symbols by default
On Fri, Jan 23, 2015 at 07:55:17AM +0900, Mike Hommey wrote: > On Thu, Jan 22, 2015 at 01:35:38PM -0500, Jeff King wrote: > > On Thu, Jan 22, 2015 at 06:36:41PM +0100, Matthieu Moy wrote: > > > > > > Yes, main point is size of executable. > > > > > > The Git executable is a few megabytes, i.e. 0.001% the size of a really > > > small hard disk. The benefit seems really negligible to me. > > > > I don't know the layout of the symbols with respect to the code, or > > whether the stripped version might reduce memory pressure. So in theory > > it could have a performance impact. > > It doesn't. Debugging info is in a part of the file that is not mapped > in memory, and in a part that can be removed without affecting the rest > of the file, so it's more or less at the end. It goes even further. These days Fedora systems strip debug info out into separate files and packages while creating rpms debuginfo packages are created automatically and provide debuginfo files under /usr/lib/debug, where gdb knows to look by default. Alexander, one nice thing about the Makefile is that it supports you creating a file in your Git worktree called "config.mak" with the following content: CFLAGS = -O2 -Wall If you do that then git will build without debug info and you won't have to specify CFLAGS when invoking "make". Hopefully that's easy and convenient enough. cheers, -- David -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] commit: reword --author error message
From: "Jeff King" On Mon, Jan 26, 2015 at 06:43:46PM -0800, Junio C Hamano wrote: Jeff King writes: > ... I somehow had trouble making > sense of Z ("a match...") as a noun. > I wonder if adding back in the missing verb, rather than a colon, > would > also make more sense: > > --author '%s' is neither 'Name ' nor a match for an > existing author Then > --author '%s' is not 'Name ' and matches no existing > author would be the shortest, sweetest and hopefully grammatical, perhaps? Yes, that one make perfect sense to me. Agreed. Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html