Re: [ANNOUNCE] Git for Windows 2.14.2(3)
> On 16 Oct 2017, at 20:59, Steve Hoelzerwrote: > > Johannes, > > On Mon, Oct 16, 2017 at 5:57 AM, Johannes Schindelin > wrote: >> Hi Steve, >> >> On Sun, 15 Oct 2017, Johannes Schindelin wrote: >> >>> On Fri, 13 Oct 2017, Steve Hoelzer wrote: >>> On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin wrote: > > It is my pleasure to announce that Git for Windows 2.14.2(3) is > available from: > >https://git-for-windows.github.io/ > > Changes since Git for Windows v2.14.2(2) (October 5th 2017) > > New Features > > * Comes with Git LFS v2.3.3. I just ran "git update" and afterward "git version" reported 2.14.2(3), but "git lfs version" still said 2.3.2. I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2. >>> >>> Ah bummer. I forgot to actually update it in the VM where I build the >>> releases :-( >>> >>> Will work on it tomorrow. >> >> I'll actually use this opportunity to revamp a part of Git for Windows' >> release engineering process to try to prevent similar things from >> happening in the future. >> >> Also, cURL v7.56.1 is slated to be released in exactly one week, and I >> have some important installer work to do this week, so I'll just defer the >> new Git for Windows version tentatively to Monday, 23rd. >> >> Git LFS users can always install Git LFS v2.3.3 specifically in the >> meantime, or use Git for Windows' snapshot versions >> (https://wingit.blob.core.windows.net/files/index.html). > > Sounds like a good plan. > > I think I have successfully updated to LFS 2.3.3 by copying the new > git-lfs.exe into C:\Program Files\Git\mingw64\bin. Is that right way > to do it? That's how I do it and that's how the Git LFS installer will (hopefully) do it in the future, too: https://github.com/git-lfs/git-lfs/issues/2587 - Lars
Re: [PATCH] check-ref-format: require a repository for --branch
Junio C Hamanowrites: >> I don't think there is any need to prepare it upon my 4d03f955, >> though. I'd think it could simply replace it. > > Yeah, it ended up that way, it seems. Still it needs a bit of doc > updates to balance the description. So here is what I have now on 'pu'. Clearly not a 2.15 material yet. -- >8 -- Subject: [PATCH] check-ref-format: --branch cannot grok @{-1} outside a repository "git check-ref-format --branch $name" feature was originally introduced (and was advertised) as a way for scripts to take any end-user supplied string (like "master", "@{-1}" etc.) and see if that is usable when Git expects to see a branch name, and also obtain the concrete branch name that the at-mark magic expands to. When the user asks to interpret a branch name like "@{-1}", we have to dig the answer out of the HEAD reflog. We can obviously only do that if we have a repository, and indeed, running it outside a repository causes us to hit a BUG(). Let's disable the "expand @{-n}" half of the feature when it is run outside a repository, but keep the feature to check the syntax of a proposed branch name, as "git check-ref-format --branch $name" can be stricter than "git check-ref-format refs/heads/$name", and Porcelain scripts need to have a way to check a given name against the stricter rule. Helped-by: Jeff King Signed-off-by: Junio C Hamano --- Documentation/git-check-ref-format.txt | 9 - builtin/check-ref-format.c | 15 --- cache.h| 14 ++ sha1_name.c| 22 +++--- strbuf.h | 6 ++ t/t1402-check-ref-format.sh| 12 6 files changed, 71 insertions(+), 7 deletions(-) diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt index 92777cef25..cf0a0b7df2 100644 --- a/Documentation/git-check-ref-format.txt +++ b/Documentation/git-check-ref-format.txt @@ -77,7 +77,14 @@ reference name expressions (see linkgit:gitrevisions[7]): . at-open-brace `@{` is used as a notation to access a reflog entry. -With the `--branch` option, it expands the ``previous branch syntax'' +With the `--branch` option, the command takes a name and checks if +it can be used as a valid branch name (e.g. when creating a new +branch). The rule `git check-ref-format --branch $name` implements +may be stricter than what `git check-ref-format refs/heads/$name` +says (e.g. a dash may appear at the beginning of a ref component, +but it is explicitly forbidden at the beginning of a branch name). +When run with `--branch` option in a repository, the input is first +expanded for the ``previous branch syntax'' `@{-n}`. For example, `@{-1}` is a way to refer the last branch you were on. This option should be used by porcelains to accept this syntax anywhere a branch name is expected, so they can act as if you diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c index eac499450f..4e62852089 100644 --- a/builtin/check-ref-format.c +++ b/builtin/check-ref-format.c @@ -38,13 +38,22 @@ static char *collapse_slashes(const char *refname) static int check_ref_format_branch(const char *arg) { + int nongit, malformed; struct strbuf sb = STRBUF_INIT; - int nongit; + const char *name = arg; setup_git_directory_gently(); - if (strbuf_check_branch_ref(, arg)) + + if (!nongit) + malformed = (strbuf_check_branch_ref(, arg) || +!skip_prefix(sb.buf, "refs/heads/", )); + else + malformed = check_branch_ref_format(arg); + + if (malformed) die("'%s' is not a valid branch name", arg); - printf("%s\n", sb.buf + 11); + printf("%s\n", name); + strbuf_release(); return 0; } diff --git a/cache.h b/cache.h index 52b91f5b64..3815925122 100644 --- a/cache.h +++ b/cache.h @@ -1444,6 +1444,20 @@ extern int parse_oid_hex(const char *hex, struct object_id *oid, const char **en #define INTERPRET_BRANCH_HEAD (1<<2) extern int interpret_branch_name(const char *str, int len, struct strbuf *, unsigned allowed); + +/* + * NEEDSWORK: declare strbuf_branchname() and strbuf_check_branch_ref() + * here, not in strbuf.h + */ + +/* + * Check if a 'name' is suitable to be used as a branch name; this can + * be and is stricter than what check_refname_format() returns for a + * string that is a concatenation of "name" after "refs/heads/"; a + * name that begins with "-" is not allowed, for example. + */ +extern int check_branch_ref_format(const char *name); + extern int get_oid_mb(const char *str, struct object_id *oid); extern int validate_headref(const char *ref); diff --git a/sha1_name.c b/sha1_name.c index 5e2ec37b65..c95080258f 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1319,15
Re: [PATCH] check-ref-format: require a repository for --branch
Hi, Junio C Hamano wrote: > Things like @{-1} would not make any sense when the command is run > outside a repository, and the documentation is quite clear that it > is the primary reason why we added "--branch" option to the command, > i.e. > > With the `--branch` option, it expands the ``previous branch syntax'' > `@{-n}`. For example, `@{-1}` is a way to refer the last branch you > were on. This option should be used by porcelains to accept this > syntax anywhere a branch name is expected, so they can act as if you > typed the branch name. > > So I am tempted to take this patch to make sure that we won't gain > more people who abuse the command outside a repository. That seems very sensible on its face. My only worry is that a script that can be run both inside and outside a repository and does branch=$(git check-ref-format --branch "$user_supplied_branch_arg") currently works with user_supplied_branch_arg='master' and would stop working. If we have reason to believe that no such scripts exist, then this would be a good way to go, but I don't believe we can count on that. And in that spirit, I think the patch you replied with aims to go in the right direction, by providing the core functionality when in a repository while avoiding breaking such a script outside of one (though I do not understand it fully yet). > Having said that, there may still be a use case where a Porcelain > script wants a way to see if a $name it has is appropriate as a > branch name before it has a repository This seems like a different goal than "git check-ref-format --branch" was originally designed to fulfill (even though it fits well with the check-ref-format name and coincides with --branch behavior when in a repository). I think it's fine for us not to fulfill it. >(e.g. a wrapper to "git > clone" that wants to verify the name it is going to give to the "-b" > option), and a check desired in such a context is different from > (and is stricter than) feeding refs/heads/$name to the same command > without the "--branch" option. Can you say more about this example? E.g. why is this hypothetical wrapper unable to rely on "git clone -b"'s own error handling? > So I think the right endgame in the longer term is: > > - Find (or add if it doesn't exist) a way to recommend to Porcelain >scripts to use to expand an end-user generated string, and to map >it to a branch name (it may be "rev-parse --symbolic-full-name >$name"; I dunno). --symbolic-full-name seems like a good fit. Do you remember why check-ref-format was introduced instead? Was it just a matter of implementation simplicity, since --symbolic-full-name can handle a broader class of revision specifications like --remotes? The commit message to v1.6.3-rc0~29^2~4 (give Porcelain a way to grok branch shorthand, 2009-03-21) is appropriately apologetic but doesn't give more clues. > - Keep check-ref-format as (or revert it to be) a tool to "check". >This would involve split strbuf_check_branch_ref() into two: Without an example of where this tool would be used, if we consider "check-ref-format --branch" to be a mistake then I'd rather deprecate it with a goal of removing it completely. Ok, time to look in more detail. Thanks for your thoughtfulness, Jonathan
Re: [PATCH] check-ref-format: require a repository for --branch
On Tue, Oct 17, 2017 at 11:40:17AM +0900, Junio C Hamano wrote: > Kevin Daudtwrites: > > >> + setup_git_directory_gently(); > >> + > >> + if (!nongit) > >> + malformed = (strbuf_check_branch_ref(, arg) || > >> + !skip_prefix(sb.buf, "refs/heads/", )); > >> + else > >> + malformed = check_branch_ref_format(arg); > >> + > > > > Would it make sense to swap the logic and get rid of the double > > negative (!nongit)? > > I am trying to follow the pattern "handle the normal case that have > been supported forever first, and then handle new exception next", > so that it is easier to see that there is no behaviour change in the > normal case, so I do not think it makes it easier to see to swap the > if/else cases. Ok, thanks for your reasoning, makes sense. > > > >> + if (malformed) > >>die("'%s' is not a valid branch name", arg); > >> - printf("%s\n", sb.buf + 11); > >> + printf("%s\n", name); > >> + strbuf_release(); > >>return 0; > >> } > >>
Re: [PATCH 2/1] mention git stash push first in the man page
On Thu, Oct 05, 2017 at 09:10:29PM +0100, Thomas Gummerer wrote: > Because 'stash push' and 'stash save' are so closely related they share one > section in the man page. Currently 'stash save' comes first, as that > was the command that people were historically using. However this makes > the newer, more feature rich git stash push very easy to overlook. > Change the order to give the newer interface for creating a stash the > more prominent position. Seems reasonable, though if we are deprecating "save" should we demote it from being in the synopsis entirely? -Peff
Re: [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path)
On Thu, Oct 05, 2017 at 09:00:49PM +0100, Thomas Gummerer wrote: > Since you were asking :) With the introduction of 'git stash push', > my hope was always that we could eventually get rid of 'git stash > save' and only keep one interface around. > > As there still many references to it around on the internet, it > probably requires a bit of a longer deprecation plan. What do you > think about the following: > > - Change docs/man pages to use 'git stash push' everywhere instead of > 'git stash save'. > - Mention the deprecation in the release notes and in the man page. > - Print a warning when 'git stash save' is used. > - Eventually get rid of it (maybe something for 3.0?) > > The steps above would all occur sequentially with a few releases > between each of them. That sounds like a pretty good plan. > A patch for the first step below. I think that even makes sense if we > don't want to follow through with the deprecation. Agreed. The patch mostly looks good, except: > diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt > index b4d88af133..1c4e44892d 100644 > --- a/Documentation/user-manual.txt > +++ b/Documentation/user-manual.txt > @@ -1556,7 +1556,7 @@ so on a different branch and then coming back), unstash > the > work-in-progress changes. > > > -$ git stash save "work in progress for foo feature" > +$ git stash push "work in progress for foo feature" > This needs "-m", doesn't it? > > This command will save your changes away to the `stash`, and > diff --git a/git-stash.sh b/git-stash.sh > index 8b2ce9afda..8ce6929d7f 100755 > --- a/git-stash.sh > +++ b/git-stash.sh > @@ -267,11 +267,11 @@ push_stash () { > # translation of "error: " takes in your language. E.g. > in > # English this is: > # > - #$ git stash save --blah-blah 2>&1 | head -n 2 > - #error: unknown option for 'stash save': --blah-blah > - # To provide a message, use git stash save -- > '--blah-blah' > - eval_gettextln "error: unknown option for 'stash save': > \$option > - To provide a message, use git stash save -- '\$option'" > + #$ git stash push --blah-blah 2>&1 | head -n 2 > + #error: unknown option for 'stash push': --blah-blah > + # To provide a message, use git stash push -- > '--blah-blah' > + eval_gettextln "error: unknown option for 'stash push': > \$option > + To provide a message, use git stash push -- '\$option'" And here, too? -Peff
Re: [PATCH 0/4] pre-merge hook
Michael J Gruberwrites: > This series revives an old suggestion of mine to make merge honor > pre-commit hook or a separate pre-merge hook This seems to have become an abandoned loose end, so I'll drop the topic from my tree for now; revival of the discussion is _not_ unwelcome (aka "dropping without prejudice").
Re: [PATCH] patch reply
Thais Dinizwrites: > +Just to clarify I did not see Marius patch. > +Did see Marius' comment saying he would look it in the leftoverbits list, > +but since i didn't see any patch i thought i could work on it and did so > based on Stephan's comment > +(which i suppose Mario also did and that is why the code resulted to be > similar). In any case, both versions share exactly the same "don't call get_multi() to grab the same configuration values from inside the callback of git_config()" issue, so whoever works on it to complete the topic, it needs further work.
Re: [PATCH][Outreachy] New git config variable to specify string that will be automatically passed as --push-option
Just to clarify I did not see Marius patch. Did see Marius' comment saying he would look it in the leftoverbits list, but since i didn't see any patch i thought i could work on it and did so based on Stephan's comment (which i suppose Mario also did and that is why the code resulted to be similar). And sorry send this email as patch. Didn't know how to use git send-email just as reply On Thu, Oct 12, 2017 at 12:26 AM, Junio C Hamanowrote: > Christian Couder writes: > >>> Can somebody explain what is going on? >>> >>> I am guessing that Thais and marius are different people (judging by >>> the fact that one CC's a message to the other). Are you two >>> collaborating on this change, or something? >> >> I guess that Thais decided to work on this, because we ask Outreachy >> applicants to search for #leftoverbits mentions in the mailing list >> archive to find small tasks they could work on. >> >> In this case it looks like Marius sent a patch a few hours before >> Thais also sent one. > > ... after seeing Marius's already working on it, I think. > >> Thais, I am sorry, but as Marius sent a patch first, I think it is >> better if you search for another different small task to work on. > > In general, I do not mind seeing people working together well, and > it is one of the more important skills necessary in the open source > community. I however tend to agree with you that this is a bit too > small a topic for multiple people to be working on. > >> Also please keep Peff and me in cc. > > Yup, that is always a good idea. > -- Atenciosamente Thais Diniz Braz
[PATCH] fetch doc: src side of refspec could be full SHA-1
Since a9d34933 ("Merge branch 'fm/fetch-raw-sha1'", 2015-06-01) we allow to fetch by an object name when the other side accepts such a request, but we never updated the documentation to match. Signed-off-by: Junio C Hamano--- Documentation/pull-fetch-param.txt | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt index 1ebbf1d738..733f932479 100644 --- a/Documentation/pull-fetch-param.txt +++ b/Documentation/pull-fetch-param.txt @@ -23,9 +23,11 @@ ifdef::git-pull[] endif::git-pull[] + The format of a parameter is an optional plus -`+`, followed by the source ref , followed +`+`, followed by the source , followed by a colon `:`, followed by the destination ref . -The colon can be omitted when is empty. +The colon can be omitted when is empty. is most +typically a ref, but it can also be an fully spelled hex object +name. + `tag ` means the same as `refs/tags/:refs/tags/`; it requests fetching everything up to the given tag. -- 2.15.0-rc1-168-g2b9456ab46
[PATCH] patch reply
From: Thais Diniz Braz--- emailReply | 4 1 file changed, 4 insertions(+) create mode 100644 emailReply diff --git a/emailReply b/emailReply new file mode 100644 index 0..2d591b55b --- /dev/null +++ b/emailReply @@ -0,0 +1,4 @@ +Just to clarify I did not see Marius patch. +Did see Marius' comment saying he would look it in the leftoverbits list, +but since i didn't see any patch i thought i could work on it and did so based on Stephan's comment +(which i suppose Mario also did and that is why the code resulted to be similar). -- 2.15.0.rc0.39.g2f0e14e.dirty
Re: [PATCH] check-ref-format: require a repository for --branch
Jeff Kingwrites: > On Tue, Oct 17, 2017 at 10:22:31AM +0900, Junio C Hamano wrote: > >> > I like the state this puts us in, but there's one catch: we're >> > completely changing the meaning of "check-ref-format --branch", aren't >> > we? >> > >> > It is going from "this is how you resolve @{-1}" to "this is how you >> > check the validity of a potential branch name". Do we need to pick a >> > different name, and/or have a deprecation period? >> ... >> At least that is what I wanted to happen in the patch. > > Ah, OK, I did not read carefully enough then. I think that would be OK, > and probably close to what Jonathan was asking for. > > It leaves unresolved the fact that the resolving feature does not belong > in check-ref-format in the first place, but we can just accept that as a > historical wart. Yup, I actually was in favor of removing that and making it a "purely checking validity" feature, but given that it has been advertised in the documentation since 604e0cb5 ("Documentation: describe check-ref-format --branch", 2009-10-12), it is a bit too late to tell users that rev-parse is the right/kosher thing to do. > I don't think there is any need to prepare it upon my 4d03f955, > though. I'd think it could simply replace it. Yeah, it ended up that way, it seems. Still it needs a bit of doc updates to balance the description. Right now we stress on @{-n} resolution too much. Perhaps something like this? Documentation/git-check-ref-format.txt | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt index 92777cef25..cf0a0b7df2 100644 --- a/Documentation/git-check-ref-format.txt +++ b/Documentation/git-check-ref-format.txt @@ -77,7 +77,14 @@ reference name expressions (see linkgit:gitrevisions[7]): . at-open-brace `@{` is used as a notation to access a reflog entry. -With the `--branch` option, it expands the ``previous branch syntax'' +With the `--branch` option, the command takes a name and checks if +it can be used as a valid branch name (e.g. when creating a new +branch). The rule `git check-ref-format --branch $name` implements +may be stricter than what `git check-ref-format refs/heads/$name` +says (e.g. a dash may appear at the beginning of a ref component, +but it is explicitly forbidden at the beginning of a branch name). +When run with `--branch` option in a repository, the input is first +expanded for the ``previous branch syntax'' `@{-n}`. For example, `@{-1}` is a way to refer the last branch you were on. This option should be used by porcelains to accept this syntax anywhere a branch name is expected, so they can act as if you
Re: [PATCH v2] column: show auto columns when pager is active
Kevin Daudtwrites: > When columns are set to automatic for git tag and the output is > paginated by git, the output is a single column instead of multiple > columns. > > Standard behaviour in git is to honor auto values when the pager is > active, which happens for example with commands like git log showing > colors when being paged. > > Since ff1e72483 (tag: change default of `pager.tag` to "on", > 2017-08-02), the pager has been enabled by default, exposing this > problem to more people. > > finalize_colopts in column.c only checks whether the output is a TTY to > determine if columns should be enabled with columns set to auto. Also > check if the pager is active. > > Adding a test for git column is possible but requires some care to work > around a race on stdin. See commit 18d8c2693 (test_terminal: redirect > child process' stdin to a pty, 2015-08-04). Test git tag instead, since > that does not involve stdin, and since that was the original motivation > for this patch. Nicely done. > +test_expect_success TTY 'git tag with auto-columns ' ' > + test_commit one && > + test_commit two && > + test_commit three && > + test_commit four && > + test_commit five && > + cat >expected <<\EOF && > +initial one two threefour five > +EOF > + test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \ > + git -c column.ui=auto tag --sort=authordate && > + test_cmp expected actual.tag > +' I'd use <<-\EOF so that here document can be intended like other tests, and also use expect vs actual that are used in the other tests in the same script, instead of suddenly becoming creative in only this single test. I can do these clean-ups locally when queuing so no need to resend only to collect these. Thanks, will queue.
Re: [PATCH] doc: list filter-branch subdirectory-filter first
David Glasserwrites: > From: David Glasser > > The docs claim that filters are applied in the listed order, so > subdirectory-filter should come first. > --- > Documentation/git-filter-branch.txt | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) Good. Could you sign it off? Somewhat related tangent is that we may want to also reorder the output from "git filter-branch -h" to the order of filter application. For that matter, the order in which the SYNOPSIS section lists these command line arguments may want to match, both for consistency and as an extra reminder to the users.
Re: Consider escaping special characters like 'less' does
On Tue, Oct 17, 2017 at 10:13:34AM +0900, Junio C Hamano wrote: > Jeff Kingwrites: > > > Alternatively, I suppose we could just always escape in > > add--interactive. I'm trying to think of a case where somebody would > > really want their diffFilter to see the raw bytes (or vice versa, to > > show raw bytes produced by their filter), but I'm having trouble coming > > up with one. > > Your patch below only implements the "tame the raw bytes that come > out of their filter", which is quite agreeable. Yes. I think that is probably OK, especially given that we continue to allow terminal escapes (certainly some filters would want to use colors; I don't know if any would want to use more exotic control codes). > > I can't help but feel this is the tip of a larger iceberg, though. E.g., > > what about characters outside of the terminal's correct encoding? Or > > broken UTF-8 characters? > > Hmph. If you use it as a "built-in" that is a fallback for > diffFilter, i.e. use it only when the end user does not have one, > then users can override whatever wrong thing the built-in logic does > so... ;-) Yes, and maybe that is the best way to do it. It just seems like it is opening a can of worms about exactly which things should be filtered and how. I also wondered if people would be annoyed that by using a filter, they don't get the benefit of the escaping, unless their filter implements it separately on top (and the original purpose of the filter option was for things like diff-highlight and diff-so-fancy, which do not do such escaping). -Peff
Re: [PATCH] check-ref-format: require a repository for --branch
On Tue, Oct 17, 2017 at 10:22:31AM +0900, Junio C Hamano wrote: > > I like the state this puts us in, but there's one catch: we're > > completely changing the meaning of "check-ref-format --branch", aren't > > we? > > > > It is going from "this is how you resolve @{-1}" to "this is how you > > check the validity of a potential branch name". Do we need to pick a > > different name, and/or have a deprecation period? > > That was not my intention. When used in a repository, it behaves > exactly the same as before, including @{-1} resolution part. And by > using strbuf_check_branch_ref(), it has always been checking the > validity of a potential branch name, even though it wasn't > advertised as such. The documentation needs to be updated, I would > think. > > When used outside a repository, @{-1} would not have worked anyway, > and @{-1} continues not to work, but the part that checks the > validity should continue to work. > > At least that is what I wanted to happen in the patch. Ah, OK, I did not read carefully enough then. I think that would be OK, and probably close to what Jonathan was asking for. It leaves unresolved the fact that the resolving feature does not belong in check-ref-format in the first place, but we can just accept that as a historical wart. I don't think there is any need to prepare it upon my 4d03f955, though. I'd think it could simply replace it. -Peff
Re: [PATCH] check-ref-format: require a repository for --branch
Kevin Daudtwrites: >> +setup_git_directory_gently(); >> + >> +if (!nongit) >> +malformed = (strbuf_check_branch_ref(, arg) || >> + !skip_prefix(sb.buf, "refs/heads/", )); >> +else >> +malformed = check_branch_ref_format(arg); >> + > > Would it make sense to swap the logic and get rid of the double > negative (!nongit)? I am trying to follow the pattern "handle the normal case that have been supported forever first, and then handle new exception next", so that it is easier to see that there is no behaviour change in the normal case, so I do not think it makes it easier to see to swap the if/else cases. > >> +if (malformed) >> die("'%s' is not a valid branch name", arg); >> -printf("%s\n", sb.buf + 11); >> +printf("%s\n", name); >> +strbuf_release(); >> return 0; >> } >>
[L10N] Kickoff of translation for Git 2.15.0 round 2
Hi, Git v2.15.0-rc1 released with a typo fix from commit dfab1eac23 ("i18n: add a missing space in message", Sun Oct 8 14:18:39 2017 +0200). This time there are 2 updated messages need to be translated since last update. Let's start new round of translation for Git 2.15.0. You can get it from the usual place: https://github.com/git-l10n/git-po/ As how to update your XX.po and help to translate Git, please see "Updating a XX.po file" and other sections in “po/README" file. -- Jiang Xin
Re: [PATCH v4 0/3] implement fetching of moved submodules
Heiko Voigtwrites: > The previous RFC iteration can be found here: > > https://public-inbox.org/git/20171006222544.GA26642@sandbox/ > > This should now be in a state ready for review for inclusion. > > The main difference from last iteration is that we now also support > unconfigured gitlinks for push and fetch for backwards compatibility. > > To implement this compatibility we construct a default name for gitlinks > if there is a repository found at their location in the worktree. I do not remember the details of the patch in the previous round that corresponds to PATCH 2/3 here, so I cannot comment on the incremental improvement between the two, but the fallback in 2/3 looks like a sensible thing to do. Let's see what others, especially those who are interested in the "--recurse-submodules" area, say. Thanks.
Re: What's cooking in git.git (Oct 2017, #03; Mon, 16)
On Mon, Oct 16, 2017 at 03:54:56PM +0900, Junio C Hamano wrote: > * bc/hash-algo (2017-10-04) 6 commits > - fixup! hash-algo: integrate hash algorithm support with repo setup > - hash-algo: switch empty tree and blob lookups to use hash abstraction > - hash-algo: integrate hash algorithm support with repo setup > - hash-algo: add structure representing hash algorithm > - setup: expose enumerated repo info > - Merge branch 'bc/vcs-svn-cleanup' into bc/hash-algo > > RFC > cf. <2017082122.26729-1-sand...@crustytoothpaste.net> I do plan to reroll this, but might not get to it for a while. Since it breaks compilation with ASan and friends, feel free to drop it if that is convenient for you or others, and I'll resubmit with some fixes. -- brian m. carlson / brian with sandals: Houston, Texas, US https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [PATCH] check-ref-format: require a repository for --branch
On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: > Junio C Hamanowrites: > > [..] > > diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c > index 1e5f9835f0..4e62852089 100644 > --- a/builtin/check-ref-format.c > +++ b/builtin/check-ref-format.c > @@ -38,12 +38,22 @@ static char *collapse_slashes(const char *refname) > > static int check_ref_format_branch(const char *arg) > { > + int nongit, malformed; > struct strbuf sb = STRBUF_INIT; > + const char *name = arg; > > - setup_git_directory(); > - if (strbuf_check_branch_ref(, arg)) > + setup_git_directory_gently(); > + > + if (!nongit) > + malformed = (strbuf_check_branch_ref(, arg) || > + !skip_prefix(sb.buf, "refs/heads/", )); > + else > + malformed = check_branch_ref_format(arg); > + Would it make sense to swap the logic and get rid of the double negative (!nongit)? > + if (malformed) > die("'%s' is not a valid branch name", arg); > - printf("%s\n", sb.buf + 11); > + printf("%s\n", name); > + strbuf_release(); > return 0; > } >
Re: [PATCH] check-ref-format: require a repository for --branch
Jeff Kingwrites: > On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: > >> > So I think the right endgame in the longer term is: >> > ... >> >> Here is to illustrate what I mean in a patch form. It resurrects >> the gentle setup, and uses a purely textual format check when we are >> outside the repository, while bypassing the @{magic} interpolation >> codepath that requires us to be in a repository. When we are in a >> repository, we operate the same way as before. > > I like the state this puts us in, but there's one catch: we're > completely changing the meaning of "check-ref-format --branch", aren't > we? > > It is going from "this is how you resolve @{-1}" to "this is how you > check the validity of a potential branch name". Do we need to pick a > different name, and/or have a deprecation period? That was not my intention. When used in a repository, it behaves exactly the same as before, including @{-1} resolution part. And by using strbuf_check_branch_ref(), it has always been checking the validity of a potential branch name, even though it wasn't advertised as such. The documentation needs to be updated, I would think. When used outside a repository, @{-1} would not have worked anyway, and @{-1} continues not to work, but the part that checks the validity should continue to work. At least that is what I wanted to happen in the patch.
Re: [PATCH v3 00/25] object_id part 10
"brian m. carlson"writes: > On Mon, Oct 16, 2017 at 11:15:34AM +0900, Junio C Hamano wrote: >> With a hope that this might help other reviewers, here is the >> interdiff between "the previous one merged with v2.15-rc1" and "this >> round applied on v2.15-rc1 directly". >> >> The changes all looked sensible to me. Thanks. > > Is there a reasonably straightforward tool or workflow to generate > interdiffs? If so, I can include them in the future. To me, it was straightforward to do: $ git checkout master^0 $ git merge bc/object-id ... free conflict resolution thanks to rerere ... $ git commit -a -m 'old one' $ OLD=$(git describe) $ git checkout master^0 $ git am bc-object-id.mbox $ git diff $OLD If you had a copy of my 'pu' branch, then you would have had a merge commit that merges your previous version of the topic to it, and you can feed that to contrib/rerere-train.sh to tell your rerere database how I resolved the conflicts there, which may apply to the reproduction of the above procedure yourself.
Re: Consider escaping special characters like 'less' does
Jeff Kingwrites: > Alternatively, I suppose we could just always escape in > add--interactive. I'm trying to think of a case where somebody would > really want their diffFilter to see the raw bytes (or vice versa, to > show raw bytes produced by their filter), but I'm having trouble coming > up with one. Your patch below only implements the "tame the raw bytes that come out of their filter", which is quite agreeable. > I.e., something like this would probably help your case without hurting > anybody: > ... > > I can't help but feel this is the tip of a larger iceberg, though. E.g., > what about characters outside of the terminal's correct encoding? Or > broken UTF-8 characters? Hmph. If you use it as a "built-in" that is a fallback for diffFilter, i.e. use it only when the end user does not have one, then users can override whatever wrong thing the built-in logic does so... ;-)
Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration
Jeff Kingwrites: > On Sat, Oct 14, 2017 at 12:01:46PM +0900, Junio C Hamano wrote: > >> > That takes us back to the pre-regression state. The ancient bug from >> > 4c7f1819 still exists, but that would be OK for v2.15. We'd probably >> > want to bump the -rc cycle a bit to give more confidence that (2) caught >> > everything. >> >> Yes, I think that is the approach I was pushing initially with the >> jc/ref-filter-colors-fix topic that was later retracted; the result >> of your 4-patch series more or less matches that one, modulo that I >> didn't treat for-each-ref as a plumbing. > > Ah, right, I forgot about that one while I was putting it together. So > many alternatives floating around. > >> I do share the worry that >> it is hard to make sure that these post-revert adjustment caught >> everything; after all, that was a major part of the reason why my >> earlier attempt was retracted. I still think this is the _right_ >> direction to go in, even though it is harder to get right. > > To be honest, I'm not actually very worried. I think missing a > post-revert adjustment is the main risk, but my general sense is that > there hasn't been a lot going on with color fixes outside of my recent > work. Famous last words and all that, I'm sure. :) > >> True. Let's see what others think. I know Jonathan is running >> the fork at $work with "downgrade always to auto" patches, and while >> I think both approaches would probably work well in practice, I have >> preference for this "harder but right" approach, so I'd want to see >> different views discussed on the list before we decide. > > After pondering over it, I have a slight preference for that, too. But > I'm also happy to hear more input. OK, so it seems we both have slight preference for the "peel back" approach. Adding Jonathan to Cc:
Re: [PATCH v3 00/25] object_id part 10
On Mon, Oct 16, 2017 at 11:49:13PM +, brian m. carlson wrote: > On Mon, Oct 16, 2017 at 11:15:34AM +0900, Junio C Hamano wrote: > > With a hope that this might help other reviewers, here is the > > interdiff between "the previous one merged with v2.15-rc1" and "this > > round applied on v2.15-rc1 directly". > > > > The changes all looked sensible to me. Thanks. > > Is there a reasonably straightforward tool or workflow to generate > interdiffs? If so, I can include them in the future. > -- > brian m. carlson / brian with sandals: Houston, Texas, US > https://www.crustytoothpaste.net/~bmc | My opinion only > OpenPGP: https://keybase.io/bk2204 tbdiff: https://github.com/trast/tbdiff
Re: [PATCH v3 00/25] object_id part 10
On Mon, Oct 16, 2017 at 11:15:34AM +0900, Junio C Hamano wrote: > With a hope that this might help other reviewers, here is the > interdiff between "the previous one merged with v2.15-rc1" and "this > round applied on v2.15-rc1 directly". > > The changes all looked sensible to me. Thanks. Is there a reasonably straightforward tool or workflow to generate interdiffs? If so, I can include them in the future. -- brian m. carlson / brian with sandals: Houston, Texas, US https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Adding a target prefix to git filter-branch --subdirectory-filter
git filter-branch --subdirectory-filter is really useful and easy to use. It's a commonly used step as part of moving a directory from one repo to another. It lets you move a subdirectory to the root of the repo. I've found that, when moving directories between repos, I often want to do a task that is very similar to --subdirectory-filter but not quite the same — I want to put the subdirectory under a prefix (and maybe in this case the "subdirectory" should just be the entire repo). Searching the web for shows that wanting to do something like this is pretty common. It's certainly possible to do this with --index-filter or --tree-filter, and the man page even has an example: git filter-branch --index-filter \ 'git ls-files -s | sed "s-\t\"*-/-" | GIT_INDEX_FILE=$GIT_INDEX_FILE.new \ git update-index --index-info && mv "$GIT_INDEX_FILE.new" "$GIT_INDEX_FILE"' HEAD But gosh, this is just a pain to write. I'd like to add direct support to git filter-branch for having a non-root target for subdirectory-filter. I have a couple questions: (1) What interface should this have? I'd lean towards having this just be part of --subdirectory-filter as a separate option --subdirectory-target-prefix. For the "move root to subdir" you maybe would have to type --subdirectory-filter=/, or maybe empty --subdirectory-filter combined with --subdirectory-target-prefix does the trick. Alternatively, it could be a new filter type specific to moving subdirectories around, but I don't know that that makes sense. (2) The way I'd imagine I would implement this would be to replace the current `git read-tree -i -m $commit:"$filter_subdir"` with `git read-tree --empty && git read-tree --prefix=PREFIX/ $commit:"$filter_subdir"`. --prefix is incompatible with -m, and I don't really understand the importance of the stat reuse that is done by `-i -m` single-tree merge. Is it OK to just drop the -i -m? --dave
[PATCH] doc: list filter-branch subdirectory-filter first
From: David GlasserThe docs claim that filters are applied in the listed order, so subdirectory-filter should come first. --- Documentation/git-filter-branch.txt | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt index 9e5169aa64f4f..605583c0ad2b5 100644 --- a/Documentation/git-filter-branch.txt +++ b/Documentation/git-filter-branch.txt @@ -89,6 +89,11 @@ OPTIONS can be used or modified in the following filter steps except the commit filter, for technical reasons. +--subdirectory-filter :: + Only look at the history which touches the given subdirectory. + The result will contain that directory (and only that) as its + project root. Implies <>. + --env-filter :: This filter may be used if you only need to modify the environment in which the commit will be performed. Specifically, you might @@ -167,11 +172,6 @@ be removed, buyer beware. There is also no support for changing the author or timestamp (or the tag message for that matter). Tags which point to other tags will be rewritten to point to the underlying commit. ---subdirectory-filter :: - Only look at the history which touches the given subdirectory. - The result will contain that directory (and only that) as its - project root. Implies <>. - --prune-empty:: Some filters will generate empty commits that leave the tree untouched. This option instructs git-filter-branch to remove such commits if they -- https://github.com/git/git/pull/415
Re: [PATCH/RFC] git-post: the opposite of git-cherry-pick
> This is worth discussing, though not my preference. The picture to "pick > cherries" has become quite common, and now that we use it for the name of > the command, "cherry-pick", the direction of flow is quite obvious and > strongly implied: from somewhere else to me (and not to somebody else). What if we borrow '--onto' from rebase and make it cherry-pick --onto ? This would keep the "pick cherries" analogy, but add the "they're not for me" intention. It also carries a bit of the "transplant" meaning of rebase. -Rafael Ascensão
Re: [PATCH] check-ref-format: require a repository for --branch
Jeff King wrote: > On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: >> Here is to illustrate what I mean in a patch form. It resurrects >> the gentle setup, and uses a purely textual format check when we are >> outside the repository, while bypassing the @{magic} interpolation >> codepath that requires us to be in a repository. When we are in a >> repository, we operate the same way as before. > > I like the state this puts us in, but there's one catch: we're > completely changing the meaning of "check-ref-format --branch", aren't > we? > > It is going from "this is how you resolve @{-1}" to "this is how you > check the validity of a potential branch name". Do we need to pick a > different name, and/or have a deprecation period? Sorry to take so long on picking this up. I'll try to make an alternate patch today. For what it's worth, I don't agree with this repurposing of "check-ref-format --branch" at all. The old command already existed. No one asked for the new command. At most, we could get rid of the old command after a deprecation period. I don't understand at all why it's worth the confusion of changing its meaning. Thanks, Jonathan
Re: Consider escaping special characters like 'less' does
On Tue, Oct 17, 2017 at 12:47:01AM +0200, Andreas Schwab wrote: > On Okt 16 2017, Jeff Kingwrote: > > > I can't help but feel this is the tip of a larger iceberg, though. E.g., > > what about characters outside of the terminal's correct encoding? Or > > broken UTF-8 characters? > > Or correctly encoded UTF-8 characters that look confusing? Or blobs > with embedded escape sequences? Yes, leaving ESC unfiltered here made me hesitate, too. We must allow it to show colors, but showing diffs with raw terminal codes can be quite confusing. My general advice on committing unprintable characters is: don't. -Peff
Re: Minor man page weirdness?
On Mon, Oct 16, 2017 at 07:16:49AM -0700, Lars Schneider wrote: > Hi, > > I just noticed that a space between "-f" and "git" is missing in `man > git-branch`. > The space is present in "Documentation/git-branch.txt", though. I am using > `man` > version 1.6c on macOS. > > -f, --force >Reset to if exists already. > Without >-fgit branch refuses to change an existing branch. In combination > with -d (or > ^^ > > Can you reproduce the "problem"? I don't see it on my copy (Debian man-db 2.7.6.1-2) . What does: cd Documentation make git-branch.1 grep Without git-branch.xml show? I see: ... -f git branch ... If there's no space there, then the problem is in asciidoc. If not, then we can further check: grep -A3 Without git-branch.1 I get: Reset to if exists already\&. Without \fB\-f\fR \fIgit branch\fR refuses to change an existing branch\&. In combination with Since there's no space there, I think we're relying on roff to insert one between lines. I'm not familiar enough with roff to say if that's a reasonable expectation or not. But if the problem is at this level, it's actually an issue between docbook and roff, and there's probably not a lot we can do on the Git side. We do have some hacks/workarounds for broken versions of the toolchain. You can try tweaking various knobs you find in Documentation/Makefile). DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the opposite (removes extra spaces). -Peff
Re: Consider escaping special characters like 'less' does
On Okt 16 2017, Jeff Kingwrote: > I can't help but feel this is the tip of a larger iceberg, though. E.g., > what about characters outside of the terminal's correct encoding? Or > broken UTF-8 characters? Or correctly encoded UTF-8 characters that look confusing? Or blobs with embedded escape sequences? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] check-ref-format: require a repository for --branch
On Mon, Oct 16, 2017 at 07:45:46PM +0900, Junio C Hamano wrote: > > So I think the right endgame in the longer term is: > > ... > > Here is to illustrate what I mean in a patch form. It resurrects > the gentle setup, and uses a purely textual format check when we are > outside the repository, while bypassing the @{magic} interpolation > codepath that requires us to be in a repository. When we are in a > repository, we operate the same way as before. I like the state this puts us in, but there's one catch: we're completely changing the meaning of "check-ref-format --branch", aren't we? It is going from "this is how you resolve @{-1}" to "this is how you check the validity of a potential branch name". Do we need to pick a different name, and/or have a deprecation period? -Peff
Re: [PATCH] check-ref-format: require a repository for --branch
On Mon, Oct 16, 2017 at 03:44:08PM +0900, Junio C Hamano wrote: > I threw this topic in stalled category, hoping that one or the other > opinion eventually turns out to be more prevalent, but it didn't > seem to have happened X-<. I think it's sufficiently obscure that nobody really cares. I admit that _I_ don't actually care myself. We should fix the BUG(), obviously, but between the two I could live with it either way. Mostly I didn't want to go to the work to write the patch for the direction that I didn't think was right, and was hoping Jonathan would if he felt strongly about it. > So I think the right endgame in the longer term is: I won't quote the rest of your message because I agree with it completely, in terms of the endgame we'd like to see. I'll address a few specific comments on your followup patch. -Peff
Re: Consider escaping special characters like 'less' does
On Sun, Oct 15, 2017 at 11:37:04PM +0200, Joris Valette wrote: > 2017-10-15 22:06 GMT+02:00 Jeff King: > > Git's diff generation will never do such escaping by default, because it > > means creating a patch that cannot be applied to get back the original > > content. > > Indeed this would be a problem. That's where my knowledge of git's > source code ends, but in that case, can't the output be discriminated > against the command that was executed? > Command that outputs an applicable patch -> don't escape > Command that outputs a diff to see changes -> escape Speaking in a general sense, people use "git diff" for both purposes, and we can't necessarily tell which[1]. As a matter of fact, that's a potential problem with textconv filters as well (which are enabled by default for git-diff, but not for plumbing like diff-tree, format-patch, etc). I think the feature isn't widely used enough for people to run into problems (and they also use it only on things that they don't _expect_ to be able to make patches for, since they're binaries). [1] Of course we can come up with heuristics, like "is stdout going to a a terminal"? But in such a case we already kick in the pager, and it does the exact escaping you're asking for. :) For a command like add--interactive, it knows which invocations are for showing to the user and which are for applying (and it already uses "--color" selectively, for instance). So if there were an "escape this" option in git's diff generation, we could selectively pass it. But again, I think the right solution there is not building the escaping into Git, but just passing it through another filter. > > It doesn't seem out of the question to me to have an out-of-the-box > > default for interactive.diffFilter which does some basic escaping (we > > could even implement it inside the perl script for efficiency). > > Yes I read this thread, but I was left unsatisfied because I would > like something out-of-the-box. > Your suggestion might be the best solution then: implement some > default escaping for interactive.diffFilter. Alternatively, I suppose we could just always escape in add--interactive. I'm trying to think of a case where somebody would really want their diffFilter to see the raw bytes (or vice versa, to show raw bytes produced by their filter), but I'm having trouble coming up with one. I.e., something like this would probably help your case without hurting anybody: diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 28b325d754..d44e5ea459 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -714,6 +714,16 @@ sub parse_diff { push @{$hunk[-1]{DISPLAY}}, (@colored ? $colored[$i] : $diff[$i]); } + + foreach my $hunk (@hunk) { + foreach my $line (@{$hunk->{DISPLAY}}) { + # all control chars minus newline and ESC (for color) + if ($line =~ s/[\000-\011\013-\032\034-\037]/?/g) { + $hunk->{CONTROLCHARS} = 1; + } + } + } + return @hunk; } @@ -1407,6 +1417,9 @@ sub patch_update_file { if ($hunk[$ix]{TYPE} eq 'hunk') { $other .= ',e'; } + if ($hunk[$ix]->{CONTROLCHARS}) { + print "warning: control characters in hunk have been replaced by '?'\n"; + } for (@{$hunk[$ix]{DISPLAY}}) { print; } I can't help but feel this is the tip of a larger iceberg, though. E.g., what about characters outside of the terminal's correct encoding? Or broken UTF-8 characters? -Peff
Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration
On Sat, Oct 14, 2017 at 12:01:46PM +0900, Junio C Hamano wrote: > > That takes us back to the pre-regression state. The ancient bug from > > 4c7f1819 still exists, but that would be OK for v2.15. We'd probably > > want to bump the -rc cycle a bit to give more confidence that (2) caught > > everything. > > Yes, I think that is the approach I was pushing initially with the > jc/ref-filter-colors-fix topic that was later retracted; the result > of your 4-patch series more or less matches that one, modulo that I > didn't treat for-each-ref as a plumbing. Ah, right, I forgot about that one while I was putting it together. So many alternatives floating around. > I do share the worry that > it is hard to make sure that these post-revert adjustment caught > everything; after all, that was a major part of the reason why my > earlier attempt was retracted. I still think this is the _right_ > direction to go in, even though it is harder to get right. To be honest, I'm not actually very worried. I think missing a post-revert adjustment is the main risk, but my general sense is that there hasn't been a lot going on with color fixes outside of my recent work. Famous last words and all that, I'm sure. :) > True. Let's see what others think. I know Jonathan is running > the fork at $work with "downgrade always to auto" patches, and while > I think both approaches would probably work well in practice, I have > preference for this "harder but right" approach, so I'd want to see > different views discussed on the list before we decide. After pondering over it, I have a slight preference for that, too. But I'm also happy to hear more input. -Peff
Re: [PATCH] diff: alias -q to --quiet
On Sat, Oct 14, 2017 at 11:37:28AM +0900, Junio C Hamano wrote: > Jeff Kingwrites: > > > So there are two separate questions/tasks: > > > > 1. Should we remove the special handling of "-q" leftover from this > > deprecation? I think the answer is yes. > > > > 2. Should we teach the diff machinery as a whole to treat "-q" as a > > synonym for "--quiet". > > Good questions. And thanks for archaeology. > > The topic #1 above is something that should have happened when "-q" stopped > working > as "--diff-filter=d", and we probably should have started to error > out then, so that scripts that relied on the original behaviour > would have been forced to update. That did not happen which was a > grave mistake. > > By doing so, we would have made sure any script that uses "-q" died > out, and after a while, we can talk about reusing it for other > purposes, like the topic #2 above. > > Is it worth making "-q" error out while doing #1 and keep it error > out for a few years? I have a feeling that the answer might be > unfortunately yes _if_ we want to also do #2. Even though we broke > "-q" for the scripts who wanted to see it ignore only the removals 4 > years ago and left it broken since then. Removals are much rarer > than modifications and additions, so it wouldn't be surprising if > the users of these scripts simply did not notice the old breakage, > but if we made "-q" to mean "--quiet" without doing #1, they will > break, as all diffs these scripts work on will suddenly give an > empty output. Yeah, after thinking about it, I do think we'd want to restart the deprecation period. For some features it would be fine, but this one is sufficiently subtle that I agree there's a good chance scripts might have been broken without anybody noticing them. > If we aren't doing #2, then I do not think we need to make "-q" > error out when we do #1, though. I don't think we'd add an explicit error-out. But by removing the leftover code, we would naturally say "no such option: -q", which amounts to the same thing. > In any case, if we were to do both of the above two, they must > happen in that order, not the other way around. Yep, agreed. -Peff
Re: [PATCH 3/3] branch: forbid refs/heads/HEAD
On Sat, Oct 14, 2017 at 11:20:11AM +0900, Junio C Hamano wrote: > Junio C Hamanowrites: > > >> Should we test that: > >> > >> git update-ref refs/heads/HEAD HEAD^ > >> > >> continues to work? > > > > Perhaps. Also we may want to make sure that "git branch -D HEAD" > > still works as a way to recover from historical mistakes. > > The only difference is improved tests where we use show-ref to make > sure refs/heads/HEAD does not exist when it shouldn't, exercise > update-ref to create and delete refs/heads/HEAD, and also make sure > it can be deleted with "git branch -d". Thanks, this looks good to me. -Peff
Re: [PATCH v4 03/11] protocol: introduce protocol extension mechanisms
On Mon, Oct 16, 2017 at 10:55:24AM -0700, Brandon Williams wrote: > Create protocol.{c,h} and provide functions which future servers and > clients can use to determine which protocol to use or is being used. > > Also introduce the 'GIT_PROTOCOL' environment variable which will be > used to communicate a colon separated list of keys with optional values > to a server. Unknown keys and values must be tolerated. This mechanism > is used to communicate which version of the wire protocol a client would > like to use with a server. > > Signed-off-by: Brandon Williams> --- > Documentation/config.txt | 17 +++ > Documentation/git.txt| 6 > Makefile | 1 + > cache.h | 8 + > protocol.c | 79 > > protocol.h | 33 > 6 files changed, 144 insertions(+) > create mode 100644 protocol.c > create mode 100644 protocol.h > > [...] > > diff --git a/protocol.h b/protocol.h > new file mode 100644 > index 0..1b2bc94a8 > --- /dev/null > +++ b/protocol.h > @@ -0,0 +1,33 @@ > +#ifndef PROTOCOL_H > +#define PROTOCOL_H > + > +enum protocol_version { > + protocol_unknown_version = -1, > + protocol_v0 = 0, > + protocol_v1 = 1, > +}; > + > +/* > + * Used by a client to determine which protocol version to request be used > when > + * communicating with a server, reflecting the configured value of the > + * 'protocol.version' config. If unconfigured, a value of 'protocol_v0' is > + * returned. > + */ The first sentence reads a little weird to me around 'which version to request be used'.
Re: [ANNOUNCE] Git for Windows 2.14.2(3)
Johannes, On Mon, Oct 16, 2017 at 5:57 AM, Johannes Schindelinwrote: > Hi Steve, > > On Sun, 15 Oct 2017, Johannes Schindelin wrote: > >> On Fri, 13 Oct 2017, Steve Hoelzer wrote: >> >> > On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin >> > wrote: >> > > >> > > It is my pleasure to announce that Git for Windows 2.14.2(3) is >> > > available from: >> > > >> > > https://git-for-windows.github.io/ >> > > >> > > Changes since Git for Windows v2.14.2(2) (October 5th 2017) >> > > >> > > New Features >> > > >> > > * Comes with Git LFS v2.3.3. >> > >> > I just ran "git update" and afterward "git version" reported >> > 2.14.2(3), but "git lfs version" still said 2.3.2. >> > >> > I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2. >> >> Ah bummer. I forgot to actually update it in the VM where I build the >> releases :-( >> >> Will work on it tomorrow. > > I'll actually use this opportunity to revamp a part of Git for Windows' > release engineering process to try to prevent similar things from > happening in the future. > > Also, cURL v7.56.1 is slated to be released in exactly one week, and I > have some important installer work to do this week, so I'll just defer the > new Git for Windows version tentatively to Monday, 23rd. > > Git LFS users can always install Git LFS v2.3.3 specifically in the > meantime, or use Git for Windows' snapshot versions > (https://wingit.blob.core.windows.net/files/index.html). Sounds like a good plan. I think I have successfully updated to LFS 2.3.3 by copying the new git-lfs.exe into C:\Program Files\Git\mingw64\bin. Is that right way to do it? Thanks, Steve
[PATCH v2] column: show auto columns when pager is active
When columns are set to automatic for git tag and the output is paginated by git, the output is a single column instead of multiple columns. Standard behaviour in git is to honor auto values when the pager is active, which happens for example with commands like git log showing colors when being paged. Since ff1e72483 (tag: change default of `pager.tag` to "on", 2017-08-02), the pager has been enabled by default, exposing this problem to more people. finalize_colopts in column.c only checks whether the output is a TTY to determine if columns should be enabled with columns set to auto. Also check if the pager is active. Adding a test for git column is possible but requires some care to work around a race on stdin. See commit 18d8c2693 (test_terminal: redirect child process' stdin to a pty, 2015-08-04). Test git tag instead, since that does not involve stdin, and since that was the original motivation for this patch. Helped-by: Rafael AscensãoSigned-off-by: Kevin Daudt --- Changes since v1: - Remove redundant -p flag in tests - Explain why git tag is being tested instead of the more obvious git column column.c | 3 ++- t/t7006-pager.sh | 14 ++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/column.c b/column.c index ff7bdab1a..ded50337f 100644 --- a/column.c +++ b/column.c @@ -5,6 +5,7 @@ #include "parse-options.h" #include "run-command.h" #include "utf8.h" +#include "pager.c" #define XY2LINEAR(d, x, y) (COL_LAYOUT((d)->colopts) == COL_COLUMN ? \ (x) * (d)->rows + (y) : \ @@ -224,7 +225,7 @@ int finalize_colopts(unsigned int *colopts, int stdout_is_tty) if (stdout_is_tty < 0) stdout_is_tty = isatty(1); *colopts &= ~COL_ENABLE_MASK; - if (stdout_is_tty) + if (stdout_is_tty || pager_in_use()) *colopts |= COL_ENABLED; } return 0; diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh index f0f1abd1c..e985b6873 100755 --- a/t/t7006-pager.sh +++ b/t/t7006-pager.sh @@ -570,4 +570,18 @@ test_expect_success 'command with underscores does not complain' ' test_cmp expect actual ' +test_expect_success TTY 'git tag with auto-columns ' ' + test_commit one && + test_commit two && + test_commit three && + test_commit four && + test_commit five && + cat >expected <<\EOF && +initial one two threefour five +EOF + test_terminal env PAGER="cat >actual.tag" COLUMNS=80 \ + git -c column.ui=auto tag --sort=authordate && + test_cmp expected actual.tag +' + test_done -- 2.14.2
Re: slight addition to t.gummerer's proposed "git stash" patch
On 10/11, Robert P. J. Day wrote: > On Wed, 11 Oct 2017, Thomas Gummerer wrote: > > > On 10/11, Robert P. J. Day wrote: > > > > > > was perusing thomas gummerer's proposed "git stash" patch here: > > > > > > https://www.spinics.net/lists/git/msg313993.html > > > > > > and i'd make one more change -- i'd separate the OPTIONS entries for > > > "git stash push" and "git stash save" so they don't end up being > > > rendered all crushed together when displaying the man page: > > > > I for one would like that. I sent a patch recently [1] that would > > show git stash push first on the man page, which didn't seem to get > > much traction. This goes a bit further than that, which I'd be happy > > with. > > > > [1]: > > https://public-inbox.org/git/20171005201029.4173-1-t.gumme...@gmail.com/ > > ... snip ... > > if you want, just crush my suggestion into your earlier patch and > resubmit it. Thanks, before doing that let me see where that discussion goes. My plan was to be a bit more careful and first get rid of mentions of 'git stash save', and mark it deprecated as a next step. In which case I'd submit a patch with your suggestions in a few cycles. > rday > > -- > > > Robert P. J. Day Ottawa, Ontario, CANADA > http://crashcourse.ca > > Twitter: http://twitter.com/rpjday > LinkedIn: http://ca.linkedin.com/in/rpjday >
Minor man page weirdness?
Hi, I just noticed that a space between "-f" and "git" is missing in `man git-branch`. The space is present in "Documentation/git-branch.txt", though. I am using `man` version 1.6c on macOS. -f, --force Reset to if exists already. Without -fgit branch refuses to change an existing branch. In combination with -d (or ^^ Can you reproduce the "problem"? Cheers, Lars
Re: [RFC] deprecate git stash save?
On 10/12, Junio C Hamano wrote: > Junio C Hamanowrites: > > > Thomas Gummerer writes: > > > >> git stash push is the newer interface for creating a stash. While we > >> are still keeping git stash save around for the time being, it's better > >> to point new users of git stash to the more modern (and more feature > >> rich) interface, instead of teaching them the older version that we > >> might want to phase out in the future. > > > > With devil's advocate hat on, because the primary point of "stash" > > being "clear the desk quickly", I do not necessarily agree that > > "more feature rich" is a plus and something we should nudge newbies > > towards. > > I do not particulary view "feature richness" is the primary benefit > of 'push' that makes it shine. 'save' has a quirk in the command > line option syntax, but 'push' corrects it, and that is why we want > to move users towards the latter. I agree it's not the primary benefit (hence why it's in braces :)), but I at least some users will eventually want the features 'git stash push' has to offer, and it's better if they don't have to re-train their fingers at that point. But yeah, fixing the quirky command line interface is definitely the stronger reason for deprecating it. > IOW, I do not object to the agenda; it is just I found the > justification used to push the agenda forward was flawed. I'm happy as long as we agree on the agenda here. Any opinions about the patches themselves? Would you prefer me to resend with an updated description? > Thanks.
[PATCH v4 11/11] Documentation: document Extra Parameters
From: Jonathan TanDocument the server support for Extra Parameters, additional information that the client can send in its first message to the server during a Git client-server interaction. Signed-off-by: Jonathan Tan Signed-off-by: Brandon Williams --- Documentation/technical/http-protocol.txt | 8 ++ Documentation/technical/pack-protocol.txt | 43 ++- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt index 1c561bdd9..a0e45f288 100644 --- a/Documentation/technical/http-protocol.txt +++ b/Documentation/technical/http-protocol.txt @@ -219,6 +219,10 @@ smart server reply: S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n +The client may send Extra Parameters (see +Documentation/technical/pack-protocol.txt) as a colon-separated string +in the Git-Protocol HTTP header. + Dumb Server Response Dumb servers MUST respond with the dumb server reply format. @@ -269,7 +273,11 @@ the C locale ordering. The stream SHOULD include the default ref named `HEAD` as the first ref. The stream MUST include capability declarations behind a NUL on the first ref. +The returned response contains "version 1" if "version=1" was sent as an +Extra Parameter. + smart_reply = PKT-LINE("# service=$servicename" LF) +*1("version 1") ref_list "" ref_list= empty_list / non_empty_list diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index ed1eae8b8..cd31edc91 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -39,6 +39,19 @@ communicates with that invoked process over the SSH connection. The file:// transport runs the 'upload-pack' or 'receive-pack' process locally and communicates with it over a pipe. +Extra Parameters + + +The protocol provides a mechanism in which clients can send additional +information in its first message to the server. These are called "Extra +Parameters", and are supported by the Git, SSH, and HTTP protocols. + +Each Extra Parameter takes the form of `=` or ``. + +Servers that receive any such Extra Parameters MUST ignore all +unrecognized keys. Currently, the only Extra Parameter recognized is +"version=1". + Git Transport - @@ -46,18 +59,25 @@ The Git transport starts off by sending the command and repository on the wire using the pkt-line format, followed by a NUL byte and a hostname parameter, terminated by a NUL byte. - 0032git-upload-pack /project.git\0host=myserver.com\0 + 0033git-upload-pack /project.git\0host=myserver.com\0 + +The transport may send Extra Parameters by adding an additional NUL +byte, and then adding one or more NUL-terminated strings: + + 003egit-upload-pack /project.git\0host=myserver.com\0\0version=1\0 -- - git-proto-request = request-command SP pathname NUL [ host-parameter NUL ] + git-proto-request = request-command SP pathname NUL + [ host-parameter NUL ] [ NUL extra-parameters ] request-command = "git-upload-pack" / "git-receive-pack" / "git-upload-archive" ; case sensitive pathname = *( %x01-ff ) ; exclude NUL host-parameter= "host=" hostname [ ":" port ] + extra-parameters = 1*extra-parameter + extra-parameter = 1*( %x01-ff ) NUL -- -Only host-parameter is allowed in the git-proto-request. Clients -MUST NOT attempt to send additional parameters. It is used for the +host-parameter is used for the git-daemon name based virtual hosting. See --interpolated-path option to git daemon, with the %H/%CH format characters. @@ -117,6 +137,12 @@ we execute it without the leading '/'. v ssh u...@example.com "git-upload-pack '~alice/project.git'" +Depending on the value of the `protocol.version` configuration variable, +Git may attempt to send Extra Parameters as a colon-separated string in +the GIT_PROTOCOL environment variable. This is done only if +the `ssh.variant` configuration variable indicates that the ssh command +supports passing environment variables as an argument. + A few things to remember here: - The "command name" is spelled with dash (e.g. git-upload-pack), but @@ -137,11 +163,13 @@ Reference Discovery --- When the client initially connects the server will immediately respond -with a listing of each reference it has (all branches and tags) along +with a version number (if "version=1" is sent as an Extra Parameter), +and a listing of each reference it has (all branches and tags) along with the object name that each reference currently points to. - $ echo -e -n
[PATCH v4 10/11] ssh: introduce a 'simple' ssh variant
When using the 'ssh' transport, the '-o' option is used to specify an environment variable which should be set on the remote end. This allows git to send additional information when contacting the server, requesting the use of a different protocol version via the 'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL". Unfortunately not all ssh variants support the sending of environment variables to the remote end. To account for this, only use the '-o' option for ssh variants which are OpenSSH compliant. This is done by checking that the basename of the ssh command is 'ssh' or the ssh variant is overridden to be 'ssh' (via the ssh.variant config). Other options like '-p' and '-P', which are used to specify a specific port to use, or '-4' and '-6', which are used to indicate that IPV4 or IPV6 addresses should be used, may also not be supported by all ssh variants. Currently if an ssh command's basename wasn't 'plink' or 'tortoiseplink' git assumes that the command is an OpenSSH variant. Since user configured ssh commands may not be OpenSSH compliant, tighten this constraint and assume a variant of 'simple' if the basename of the command doesn't match the variants known to git. The new ssh variant 'simple' will only have the host and command to execute ([username@]host command) passed as parameters to the ssh command. Update the Documentation to better reflect the command-line options sent to ssh commands based on their variant. Reported-by: Jeffrey YasskinSigned-off-by: Brandon Williams --- Documentation/config.txt | 27 ++-- Documentation/git.txt| 9 ++-- connect.c| 108 ++- t/t5601-clone.sh | 26 +--- t/t5700-protocol-v1.sh | 2 + 5 files changed, 111 insertions(+), 61 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index b78747abc..0460af37e 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2084,12 +2084,31 @@ ssh.variant:: Depending on the value of the environment variables `GIT_SSH` or `GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git auto-detects whether to adjust its command-line parameters for use - with plink or tortoiseplink, as opposed to the default (OpenSSH). + with ssh (OpenSSH), plink or tortoiseplink, as opposed to the default + (simple). + The config variable `ssh.variant` can be set to override this auto-detection; -valid values are `ssh`, `plink`, `putty` or `tortoiseplink`. Any other value -will be treated as normal ssh. This setting can be overridden via the -environment variable `GIT_SSH_VARIANT`. +valid values are `ssh`, `simple`, `plink`, `putty` or `tortoiseplink`. Any +other value will be treated as normal ssh. This setting can be overridden via +the environment variable `GIT_SSH_VARIANT`. ++ +The current command-line parameters used for each variant are as +follows: ++ +-- + +* `ssh` - [-p port] [-4] [-6] [-o option] [username@]host command + +* `simple` - [username@]host command + +* `plink` or `putty` - [-P port] [-4] [-6] [username@]host command + +* `tortoiseplink` - [-P port] [-4] [-6] -batch [username@]host command + +-- ++ +Except for the `simple` variant, command-line parameters are likely to +change as git gains new features. i18n.commitEncoding:: Character encoding the commit messages are stored in; Git itself diff --git a/Documentation/git.txt b/Documentation/git.txt index 7518ea3af..8bc3f2147 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -518,11 +518,10 @@ other If either of these environment variables is set then 'git fetch' and 'git push' will use the specified command instead of 'ssh' when they need to connect to a remote system. - The command will be given exactly two or four arguments: the - 'username@host' (or just 'host') from the URL and the shell - command to execute on that remote system, optionally preceded by - `-p` (literally) and the 'port' from the URL when it specifies - something other than the default SSH port. + The command-line parameters passed to the configured command are + determined by the ssh variant. See `ssh.variant` option in + linkgit:git-config[1] for details. + + `$GIT_SSH_COMMAND` takes precedence over `$GIT_SSH`, and is interpreted by the shell, which allows additional arguments to be included. diff --git a/connect.c b/connect.c index b8695a2fa..7fbd396b3 100644 --- a/connect.c +++ b/connect.c @@ -776,37 +776,44 @@ static const char *get_ssh_command(void) return NULL; } -static int override_ssh_variant(int *port_option, int *needs_batch) +enum ssh_variant { + VARIANT_SIMPLE, + VARIANT_SSH, + VARIANT_PLINK, + VARIANT_PUTTY, + VARIANT_TORTOISEPLINK, +}; + +static int override_ssh_variant(enum ssh_variant *ssh_variant)
[PATCH v4 00/11] protocol transition
Changes in v4: * Added more tests for the new handeling of ssh variants. * Removed the 'default' case in upload_pack and receive_pack and instead ensured that all enum values were accounted for. This way when a new protocol version is introduced the compiler will throw an error if the new protocol version isn't accounted for in these switch statements. * Added Jonathan's Documentation patch ontop of the series (with the small change I pointed out in reply to the patch itself) * A few other small changes due to reviewer comments. Brandon Williams (9): pkt-line: add packet_write function protocol: introduce protocol extension mechanisms daemon: recognize hidden request arguments upload-pack, receive-pack: introduce protocol version 1 connect: teach client to recognize v1 server response connect: tell server that the client understands v1 http: tell server that the client understands v1 i5700: add interop test for protocol transition ssh: introduce a 'simple' ssh variant Jonathan Tan (2): connect: in ref advertisement, shallows are last Documentation: document Extra Parameters Documentation/config.txt | 44 +++- Documentation/git.txt | 15 +- Documentation/technical/http-protocol.txt | 8 + Documentation/technical/pack-protocol.txt | 43 +++- Makefile | 1 + builtin/receive-pack.c| 17 ++ cache.h | 10 + connect.c | 354 -- daemon.c | 71 +- http.c| 18 ++ pkt-line.c| 6 + pkt-line.h| 1 + protocol.c| 79 +++ protocol.h| 33 +++ t/interop/i5700-protocol-transition.sh| 68 ++ t/lib-httpd/apache.conf | 7 + t/t5601-clone.sh | 26 ++- t/t5700-protocol-v1.sh| 294 + upload-pack.c | 20 +- 19 files changed, 967 insertions(+), 148 deletions(-) create mode 100644 protocol.c create mode 100644 protocol.h create mode 100755 t/interop/i5700-protocol-transition.sh create mode 100755 t/t5700-protocol-v1.sh --- interdiff with 'origin/bw/protocol-v1' diff --git a/Documentation/technical/http-protocol.txt b/Documentation/technical/http-protocol.txt index 1c561bdd9..a0e45f288 100644 --- a/Documentation/technical/http-protocol.txt +++ b/Documentation/technical/http-protocol.txt @@ -219,6 +219,10 @@ smart server reply: S: 003c2cb58b79488a98d2721cea644875a8dd0026b115 refs/tags/v1.0\n S: 003fa3c2e2402b99163d1d59756e5f207ae21cccba4c refs/tags/v1.0^{}\n +The client may send Extra Parameters (see +Documentation/technical/pack-protocol.txt) as a colon-separated string +in the Git-Protocol HTTP header. + Dumb Server Response Dumb servers MUST respond with the dumb server reply format. @@ -269,7 +273,11 @@ the C locale ordering. The stream SHOULD include the default ref named `HEAD` as the first ref. The stream MUST include capability declarations behind a NUL on the first ref. +The returned response contains "version 1" if "version=1" was sent as an +Extra Parameter. + smart_reply = PKT-LINE("# service=$servicename" LF) +*1("version 1") ref_list "" ref_list= empty_list / non_empty_list diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt index ed1eae8b8..cd31edc91 100644 --- a/Documentation/technical/pack-protocol.txt +++ b/Documentation/technical/pack-protocol.txt @@ -39,6 +39,19 @@ communicates with that invoked process over the SSH connection. The file:// transport runs the 'upload-pack' or 'receive-pack' process locally and communicates with it over a pipe. +Extra Parameters + + +The protocol provides a mechanism in which clients can send additional +information in its first message to the server. These are called "Extra +Parameters", and are supported by the Git, SSH, and HTTP protocols. + +Each Extra Parameter takes the form of `=` or ``. + +Servers that receive any such Extra Parameters MUST ignore all +unrecognized keys. Currently, the only Extra Parameter recognized is +"version=1". + Git Transport - @@ -46,18 +59,25 @@ The Git transport starts off by sending the command and repository on the wire using the pkt-line format, followed by a NUL byte and a hostname parameter, terminated by a NUL byte. - 0032git-upload-pack /project.git\0host=myserver.com\0 + 0033git-upload-pack /project.git\0host=myserver.com\0 + +The transport may send Extra Parameters by adding an additional NUL +byte, and then adding one or more NUL-terminated
[PATCH v4 08/11] http: tell server that the client understands v1
Tell a server that protocol v1 can be used by sending the http header 'Git-Protocol' with 'version=1' indicating this. Also teach the apache http server to pass through the 'Git-Protocol' header as an environment variable 'GIT_PROTOCOL'. Signed-off-by: Brandon Williams--- cache.h | 2 ++ http.c | 18 + t/lib-httpd/apache.conf | 7 + t/t5700-protocol-v1.sh | 69 + 4 files changed, 96 insertions(+) diff --git a/cache.h b/cache.h index c74b73671..3a6b869c2 100644 --- a/cache.h +++ b/cache.h @@ -452,6 +452,8 @@ static inline enum object_type object_type(unsigned int mode) * ignored. */ #define GIT_PROTOCOL_ENVIRONMENT "GIT_PROTOCOL" +/* HTTP header used to handshake the wire protocol */ +#define GIT_PROTOCOL_HEADER "Git-Protocol" /* * This environment variable is expected to contain a boolean indicating diff --git a/http.c b/http.c index 9e40a465f..ffb719216 100644 --- a/http.c +++ b/http.c @@ -12,6 +12,7 @@ #include "gettext.h" #include "transport.h" #include "packfile.h" +#include "protocol.h" static struct trace_key trace_curl = TRACE_KEY_INIT(CURL); #if LIBCURL_VERSION_NUM >= 0x070a08 @@ -897,6 +898,21 @@ static void set_from_env(const char **var, const char *envname) *var = val; } +static void protocol_http_header(void) +{ + if (get_protocol_version_config() > 0) { + struct strbuf protocol_header = STRBUF_INIT; + + strbuf_addf(_header, GIT_PROTOCOL_HEADER ": version=%d", + get_protocol_version_config()); + + + extra_http_headers = curl_slist_append(extra_http_headers, + protocol_header.buf); + strbuf_release(_header); + } +} + void http_init(struct remote *remote, const char *url, int proactive_auth) { char *low_speed_limit; @@ -927,6 +943,8 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) if (remote) var_override(_proxy_authmethod, remote->http_proxy_authmethod); + protocol_http_header(); + pragma_header = curl_slist_append(http_copy_default_headers(), "Pragma: no-cache"); no_pragma_header = curl_slist_append(http_copy_default_headers(), diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index 0642ae7e6..df1943631 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -67,6 +67,9 @@ LockFile accept.lock LoadModule unixd_module modules/mod_unixd.so + + LoadModule setenvif_module modules/mod_setenvif.so + PassEnv GIT_VALGRIND @@ -76,6 +79,10 @@ PassEnv ASAN_OPTIONS PassEnv GIT_TRACE PassEnv GIT_CONFIG_NOSYSTEM += 2.4> + SetEnvIf Git-Protocol ".*" GIT_PROTOCOL=$0 + + Alias /dumb/ www/ Alias /auth/dumb/ www/auth/dumb/ diff --git a/t/t5700-protocol-v1.sh b/t/t5700-protocol-v1.sh index 6551932da..b0779d362 100755 --- a/t/t5700-protocol-v1.sh +++ b/t/t5700-protocol-v1.sh @@ -220,4 +220,73 @@ test_expect_success 'push with ssh:// using protocol v1' ' grep "push< version 1" log ' +# Test protocol v1 with 'http://' transport +# +. "$TEST_DIRECTORY"/lib-httpd.sh +start_httpd + +test_expect_success 'create repo to be served by http:// transport' ' + git init "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" config http.receivepack true && + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" one +' + +test_expect_success 'clone with http:// using protocol v1' ' + GIT_TRACE_PACKET=1 GIT_TRACE_CURL=1 git -c protocol.version=1 \ + clone "$HTTPD_URL/smart/http_parent" http_child 2>log && + + git -C http_child log -1 --format=%s >actual && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" log -1 --format=%s >expect && + test_cmp expect actual && + + # Client requested to use protocol v1 + grep "Git-Protocol: version=1" log && + # Server responded using protocol v1 + grep "git< version 1" log +' + +test_expect_success 'fetch with http:// using protocol v1' ' + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" two && + + GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \ + fetch 2>log && + + git -C http_child log -1 --format=%s origin/master >actual && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" log -1 --format=%s >expect && + test_cmp expect actual && + + # Server responded using protocol v1 + grep "git< version 1" log +' + +test_expect_success 'pull with http:// using protocol v1' ' + GIT_TRACE_PACKET=1 git -C http_child -c protocol.version=1 \ + pull 2>log && + + git -C http_child log -1 --format=%s >actual && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/http_parent" log -1 --format=%s >expect && + test_cmp
[PATCH v4 01/11] connect: in ref advertisement, shallows are last
From: Jonathan TanCurrently, get_remote_heads() parses the ref advertisement in one loop, allowing refs and shallow lines to intersperse, despite this not being allowed by the specification. Refactor get_remote_heads() to use two loops instead, enforcing that refs come first, and then shallows. This also makes it easier to teach get_remote_heads() to interpret other lines in the ref advertisement, which will be done in a subsequent patch. As part of this change, this patch interprets capabilities only on the first line in the ref advertisement, printing a warning message when encountering capabilities on other lines. Signed-off-by: Jonathan Tan Signed-off-by: Brandon Williams --- connect.c | 189 -- 1 file changed, 123 insertions(+), 66 deletions(-) diff --git a/connect.c b/connect.c index df56c0cbf..8e2e276b6 100644 --- a/connect.c +++ b/connect.c @@ -11,6 +11,7 @@ #include "string-list.h" #include "sha1-array.h" #include "transport.h" +#include "strbuf.h" static char *server_capabilities; static const char *parse_feature_value(const char *, const char *, int *); @@ -107,6 +108,104 @@ static void annotate_refs_with_symref_info(struct ref *ref) string_list_clear(, 0); } +/* + * Read one line of a server's ref advertisement into packet_buffer. + */ +static int read_remote_ref(int in, char **src_buf, size_t *src_len, + int *responded) +{ + int len = packet_read(in, src_buf, src_len, + packet_buffer, sizeof(packet_buffer), + PACKET_READ_GENTLE_ON_EOF | + PACKET_READ_CHOMP_NEWLINE); + const char *arg; + if (len < 0) + die_initial_contact(*responded); + if (len > 4 && skip_prefix(packet_buffer, "ERR ", )) + die("remote error: %s", arg); + + *responded = 1; + + return len; +} + +#define EXPECTING_FIRST_REF 0 +#define EXPECTING_REF 1 +#define EXPECTING_SHALLOW 2 + +static void process_capabilities(int *len) +{ + int nul_location = strlen(packet_buffer); + if (nul_location == *len) + return; + server_capabilities = xstrdup(packet_buffer + nul_location + 1); + *len = nul_location; +} + +static int process_dummy_ref(void) +{ + struct object_id oid; + const char *name; + + if (parse_oid_hex(packet_buffer, , )) + return 0; + if (*name != ' ') + return 0; + name++; + + return !oidcmp(_oid, ) && !strcmp(name, "capabilities^{}"); +} + +static void check_no_capabilities(int len) +{ + if (strlen(packet_buffer) != len) + warning("Ignoring capabilities after first line '%s'", + packet_buffer + strlen(packet_buffer)); +} + +static int process_ref(int len, struct ref ***list, unsigned int flags, + struct oid_array *extra_have) +{ + struct object_id old_oid; + const char *name; + + if (parse_oid_hex(packet_buffer, _oid, )) + return 0; + if (*name != ' ') + return 0; + name++; + + if (extra_have && !strcmp(name, ".have")) { + oid_array_append(extra_have, _oid); + } else if (!strcmp(name, "capabilities^{}")) { + die("protocol error: unexpected capabilities^{}"); + } else if (check_ref(name, flags)) { + struct ref *ref = alloc_ref(name); + oidcpy(>old_oid, _oid); + **list = ref; + *list = >next; + } + check_no_capabilities(len); + return 1; +} + +static int process_shallow(int len, struct oid_array *shallow_points) +{ + const char *arg; + struct object_id old_oid; + + if (!skip_prefix(packet_buffer, "shallow ", )) + return 0; + + if (get_oid_hex(arg, _oid)) + die("protocol error: expected shallow sha-1, got '%s'", arg); + if (!shallow_points) + die("repository on the other end cannot be shallow"); + oid_array_append(shallow_points, _oid); + check_no_capabilities(len); + return 1; +} + /* * Read all the refs from the other end */ @@ -123,76 +222,34 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, * willing to talk to us. A hang-up before seeing any * response does not necessarily mean an ACL problem, though. */ - int saw_response; - int got_dummy_ref_with_capabilities_declaration = 0; + int responded = 0; + int len; + int state = EXPECTING_FIRST_REF; *list = NULL; - for (saw_response = 0; ; saw_response = 1) { - struct ref *ref; - struct object_id old_oid; - char *name; - int len, name_len; - char *buffer =
[PATCH v4 06/11] connect: teach client to recognize v1 server response
Teach a client to recognize that a server understands protocol v1 by looking at the first pkt-line the server sends in response. This is done by looking for the response "version 1" send by upload-pack or receive-pack. Signed-off-by: Brandon Williams--- connect.c | 30 ++ 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/connect.c b/connect.c index 8e2e276b6..a5e708a61 100644 --- a/connect.c +++ b/connect.c @@ -12,6 +12,7 @@ #include "sha1-array.h" #include "transport.h" #include "strbuf.h" +#include "protocol.h" static char *server_capabilities; static const char *parse_feature_value(const char *, const char *, int *); @@ -129,9 +130,23 @@ static int read_remote_ref(int in, char **src_buf, size_t *src_len, return len; } -#define EXPECTING_FIRST_REF 0 -#define EXPECTING_REF 1 -#define EXPECTING_SHALLOW 2 +#define EXPECTING_PROTOCOL_VERSION 0 +#define EXPECTING_FIRST_REF 1 +#define EXPECTING_REF 2 +#define EXPECTING_SHALLOW 3 + +/* Returns 1 if packet_buffer is a protocol version pkt-line, 0 otherwise. */ +static int process_protocol_version(void) +{ + switch (determine_protocol_version_client(packet_buffer)) { + case protocol_v1: + return 1; + case protocol_v0: + return 0; + default: + die("server is speaking an unknown protocol"); + } +} static void process_capabilities(int *len) { @@ -224,12 +239,19 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len, */ int responded = 0; int len; - int state = EXPECTING_FIRST_REF; + int state = EXPECTING_PROTOCOL_VERSION; *list = NULL; while ((len = read_remote_ref(in, _buf, _len, ))) { switch (state) { + case EXPECTING_PROTOCOL_VERSION: + if (process_protocol_version()) { + state = EXPECTING_FIRST_REF; + break; + } + state = EXPECTING_FIRST_REF; + /* fallthrough */ case EXPECTING_FIRST_REF: process_capabilities(); if (process_dummy_ref()) { -- 2.15.0.rc0.271.g36b669edcc-goog
[PATCH v4 02/11] pkt-line: add packet_write function
Add a function which can be used to write the contents of an arbitrary buffer. This makes it easy to build up data in a buffer before writing the packet instead of formatting the entire contents of the packet using 'packet_write_fmt()'. Signed-off-by: Brandon Williams--- pkt-line.c | 6 ++ pkt-line.h | 1 + 2 files changed, 7 insertions(+) diff --git a/pkt-line.c b/pkt-line.c index 647bbd3bc..7006b3587 100644 --- a/pkt-line.c +++ b/pkt-line.c @@ -188,6 +188,12 @@ static int packet_write_gently(const int fd_out, const char *buf, size_t size) return 0; } +void packet_write(int fd_out, const char *buf, size_t size) +{ + if (packet_write_gently(fd_out, buf, size)) + die_errno("packet write failed"); +} + void packet_buf_write(struct strbuf *buf, const char *fmt, ...) { va_list args; diff --git a/pkt-line.h b/pkt-line.h index 66ef610fc..3dad583e2 100644 --- a/pkt-line.h +++ b/pkt-line.h @@ -22,6 +22,7 @@ void packet_flush(int fd); void packet_write_fmt(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3))); void packet_buf_flush(struct strbuf *buf); +void packet_write(int fd_out, const char *buf, size_t size); void packet_buf_write(struct strbuf *buf, const char *fmt, ...) __attribute__((format (printf, 2, 3))); int packet_flush_gently(int fd); int packet_write_fmt_gently(int fd, const char *fmt, ...) __attribute__((format (printf, 2, 3))); -- 2.15.0.rc0.271.g36b669edcc-goog
[PATCH v4 04/11] daemon: recognize hidden request arguments
A normal request to git-daemon is structured as "command path/to/repo\0host=..\0" and due to a bug introduced in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we aren't able to place any extra arguments (separated by NULs) besides the host otherwise the parsing of those arguments would enter an infinite loop. This bug was fixed in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) but a check was put in place to disallow extra arguments so that new clients wouldn't trigger this bug in older servers. In order to get around this limitation teach git-daemon to recognize additional request arguments hidden behind a second NUL byte. Requests can then be structured like: "command path/to/repo\0host=..\0\0version=1\0key=value\0". git-daemon can then parse out the extra arguments and set 'GIT_PROTOCOL' accordingly. By placing these extra arguments behind a second NUL byte we can skirt around both the infinite loop bug in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) as well as the explicit disallowing of extra arguments introduced in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) because both of these versions of git-daemon check for a single NUL byte after the host argument before terminating the argument parsing. Signed-off-by: Brandon Williams--- daemon.c | 71 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/daemon.c b/daemon.c index 30747075f..e37e343d0 100644 --- a/daemon.c +++ b/daemon.c @@ -282,7 +282,7 @@ static const char *path_ok(const char *directory, struct hostinfo *hi) return NULL;/* Fallthrough. Deny by default */ } -typedef int (*daemon_service_fn)(void); +typedef int (*daemon_service_fn)(const struct argv_array *env); struct daemon_service { const char *name; const char *config_name; @@ -363,7 +363,7 @@ static int run_access_hook(struct daemon_service *service, const char *dir, } static int run_service(const char *dir, struct daemon_service *service, - struct hostinfo *hi) + struct hostinfo *hi, const struct argv_array *env) { const char *path; int enabled = service->enabled; @@ -422,7 +422,7 @@ static int run_service(const char *dir, struct daemon_service *service, */ signal(SIGTERM, SIG_IGN); - return service->fn(); + return service->fn(env); } static void copy_to_log(int fd) @@ -462,25 +462,34 @@ static int run_service_command(struct child_process *cld) return finish_command(cld); } -static int upload_pack(void) +static int upload_pack(const struct argv_array *env) { struct child_process cld = CHILD_PROCESS_INIT; argv_array_pushl(, "upload-pack", "--strict", NULL); argv_array_pushf(, "--timeout=%u", timeout); + + argv_array_pushv(_array, env->argv); + return run_service_command(); } -static int upload_archive(void) +static int upload_archive(const struct argv_array *env) { struct child_process cld = CHILD_PROCESS_INIT; argv_array_push(, "upload-archive"); + + argv_array_pushv(_array, env->argv); + return run_service_command(); } -static int receive_pack(void) +static int receive_pack(const struct argv_array *env) { struct child_process cld = CHILD_PROCESS_INIT; argv_array_push(, "receive-pack"); + + argv_array_pushv(_array, env->argv); + return run_service_command(); } @@ -573,8 +582,11 @@ static void canonicalize_client(struct strbuf *out, const char *in) /* * Read the host as supplied by the client connection. + * + * Returns a pointer to the character after the NUL byte terminating the host + * arguemnt, or 'extra_args' if there is no host arguemnt. */ -static void parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen) +static char *parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen) { char *val; int vallen; @@ -602,6 +614,43 @@ static void parse_host_arg(struct hostinfo *hi, char *extra_args, int buflen) if (extra_args < end && *extra_args) die("Invalid request"); } + + return extra_args; +} + +static void parse_extra_args(struct hostinfo *hi, struct argv_array *env, +char *extra_args, int buflen) +{ + const char *end = extra_args + buflen; + struct strbuf git_protocol = STRBUF_INIT; + + /* First look for the host argument */ + extra_args = parse_host_arg(hi, extra_args, buflen); + + /* Look for additional arguments places after a second NUL byte */ + for (; extra_args < end; extra_args += strlen(extra_args) + 1) { + const char *arg = extra_args; + + /* +* Parse the extra arguments, adding most to 'git_protocol' +
[PATCH v4 05/11] upload-pack, receive-pack: introduce protocol version 1
Teach upload-pack and receive-pack to understand and respond using protocol version 1, if requested. Protocol version 1 is simply the original and current protocol (what I'm calling version 0) with the addition of a single packet line, which precedes the ref advertisement, indicating the protocol version being spoken. Signed-off-by: Brandon Williams--- builtin/receive-pack.c | 17 + upload-pack.c | 20 +++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index dd06b3fb4..839c1462d 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -24,6 +24,7 @@ #include "tmp-objdir.h" #include "oidset.h" #include "packfile.h" +#include "protocol.h" static const char * const receive_pack_usage[] = { N_("git receive-pack "), @@ -1963,6 +1964,22 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) else if (0 <= receive_unpack_limit) unpack_limit = receive_unpack_limit; + switch (determine_protocol_version_server()) { + case protocol_v1: + /* +* v1 is just the original protocol with a version string, +* so just fall through after writing the version string. +*/ + if (advertise_refs || !stateless_rpc) + packet_write_fmt(1, "version 1\n"); + + /* fallthrough */ + case protocol_v0: + break; + case protocol_unknown_version: + BUG("unknown protocol version"); + } + if (advertise_refs || !stateless_rpc) { write_head_info(); } diff --git a/upload-pack.c b/upload-pack.c index 7efff2fbf..ef99a029c 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -18,6 +18,7 @@ #include "parse-options.h" #include "argv-array.h" #include "prio-queue.h" +#include "protocol.h" static const char * const upload_pack_usage[] = { N_("git upload-pack [] "), @@ -1067,6 +1068,23 @@ int cmd_main(int argc, const char **argv) die("'%s' does not appear to be a git repository", dir); git_config(upload_pack_config, NULL); - upload_pack(); + + switch (determine_protocol_version_server()) { + case protocol_v1: + /* +* v1 is just the original protocol with a version string, +* so just fall through after writing the version string. +*/ + if (advertise_refs || !stateless_rpc) + packet_write_fmt(1, "version 1\n"); + + /* fallthrough */ + case protocol_v0: + upload_pack(); + break; + case protocol_unknown_version: + BUG("unknown protocol version"); + } + return 0; } -- 2.15.0.rc0.271.g36b669edcc-goog
[PATCH v4 07/11] connect: tell server that the client understands v1
Teach the connection logic to tell a serve that it understands protocol v1. This is done in 2 different ways for the builtin transports, both of which ultimately set 'GIT_PROTOCOL' to 'version=1' on the server. 1. git:// A normal request to git-daemon is structured as "command path/to/repo\0host=..\0" and due to a bug introduced in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) we aren't able to place any extra arguments (separated by NULs) besides the host otherwise the parsing of those arguments would enter an infinite loop. This bug was fixed in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) but a check was put in place to disallow extra arguments so that new clients wouldn't trigger this bug in older servers. In order to get around this limitation git-daemon was taught to recognize additional request arguments hidden behind a second NUL byte. Requests can then be structured like: "command path/to/repo\0host=..\0\0version=1\0key=value\0". git-daemon can then parse out the extra arguments and set 'GIT_PROTOCOL' accordingly. By placing these extra arguments behind a second NUL byte we can skirt around both the infinite loop bug in 49ba83fb6 (Add virtualization support to git-daemon, 2006-09-19) as well as the explicit disallowing of extra arguments introduced in 73bb33a94 (daemon: Strictly parse the "extra arg" part of the command, 2009-06-04) because both of these versions of git-daemon check for a single NUL byte after the host argument before terminating the argument parsing. 2. ssh://, file:// Set 'GIT_PROTOCOL' environment variable with the desired protocol version. With the file:// transport, 'GIT_PROTOCOL' can be set explicitly in the locally running git-upload-pack or git-receive-pack processes. With the ssh:// transport and OpenSSH compliant ssh programs, 'GIT_PROTOCOL' can be sent across ssh by using '-o SendEnv=GIT_PROTOCOL' and having the server whitelist this environment variable. Signed-off-by: Brandon Williams--- connect.c | 37 ++-- t/t5700-protocol-v1.sh | 223 + 2 files changed, 255 insertions(+), 5 deletions(-) create mode 100755 t/t5700-protocol-v1.sh diff --git a/connect.c b/connect.c index a5e708a61..b8695a2fa 100644 --- a/connect.c +++ b/connect.c @@ -871,6 +871,7 @@ struct child_process *git_connect(int fd[2], const char *url, printf("Diag: path=%s\n", path ? path : "NULL"); conn = NULL; } else if (protocol == PROTO_GIT) { + struct strbuf request = STRBUF_INIT; /* * Set up virtual host information based on where we will * connect, unless the user has overridden us in @@ -898,13 +899,25 @@ struct child_process *git_connect(int fd[2], const char *url, * Note: Do not add any other headers here! Doing so * will cause older git-daemon servers to crash. */ - packet_write_fmt(fd[1], -"%s %s%chost=%s%c", -prog, path, 0, -target_host, 0); + strbuf_addf(, + "%s %s%chost=%s%c", + prog, path, 0, + target_host, 0); + + /* If using a new version put that stuff here after a second null byte */ + if (get_protocol_version_config() > 0) { + strbuf_addch(, '\0'); + strbuf_addf(, "version=%d%c", + get_protocol_version_config(), '\0'); + } + + packet_write(fd[1], request.buf, request.len); + free(target_host); + strbuf_release(); } else { struct strbuf cmd = STRBUF_INIT; + const char *const *var; conn = xmalloc(sizeof(*conn)); child_process_init(conn); @@ -917,7 +930,9 @@ struct child_process *git_connect(int fd[2], const char *url, sq_quote_buf(, path); /* remove repo-local variables from the environment */ - conn->env = local_repo_env; + for (var = local_repo_env; *var; var++) + argv_array_push(>env_array, *var); + conn->use_shell = 1; conn->in = conn->out = -1; if (protocol == PROTO_SSH) { @@ -971,6 +986,14 @@ struct child_process *git_connect(int fd[2], const char *url, } argv_array_push(>args, ssh); + + if (get_protocol_version_config() > 0) { + argv_array_push(>args, "-o"); + argv_array_push(>args, "SendEnv="
[PATCH v4 09/11] i5700: add interop test for protocol transition
Signed-off-by: Brandon Williams--- t/interop/i5700-protocol-transition.sh | 68 ++ 1 file changed, 68 insertions(+) create mode 100755 t/interop/i5700-protocol-transition.sh diff --git a/t/interop/i5700-protocol-transition.sh b/t/interop/i5700-protocol-transition.sh new file mode 100755 index 0..97e8e580e --- /dev/null +++ b/t/interop/i5700-protocol-transition.sh @@ -0,0 +1,68 @@ +#!/bin/sh + +VERSION_A=. +VERSION_B=v2.0.0 + +: ${LIB_GIT_DAEMON_PORT:=5700} +LIB_GIT_DAEMON_COMMAND='git.b daemon' + +test_description='clone and fetch by client who is trying to use a new protocol' +. ./interop-lib.sh +. "$TEST_DIRECTORY"/lib-git-daemon.sh + +start_git_daemon --export-all + +repo=$GIT_DAEMON_DOCUMENT_ROOT_PATH/repo + +test_expect_success "create repo served by $VERSION_B" ' + git.b init "$repo" && + git.b -C "$repo" commit --allow-empty -m one +' + +test_expect_success "git:// clone with $VERSION_A and protocol v1" ' + GIT_TRACE_PACKET=1 git.a -c protocol.version=1 clone "$GIT_DAEMON_URL/repo" child 2>log && + git.a -C child log -1 --format=%s >actual && + git.b -C "$repo" log -1 --format=%s >expect && + test_cmp expect actual && + grep "version=1" log +' + +test_expect_success "git:// fetch with $VERSION_A and protocol v1" ' + git.b -C "$repo" commit --allow-empty -m two && + git.b -C "$repo" log -1 --format=%s >expect && + + GIT_TRACE_PACKET=1 git.a -C child -c protocol.version=1 fetch 2>log && + git.a -C child log -1 --format=%s FETCH_HEAD >actual && + + test_cmp expect actual && + grep "version=1" log && + ! grep "version 1" log +' + +stop_git_daemon + +test_expect_success "create repo served by $VERSION_B" ' + git.b init parent && + git.b -C parent commit --allow-empty -m one +' + +test_expect_success "file:// clone with $VERSION_A and protocol v1" ' + GIT_TRACE_PACKET=1 git.a -c protocol.version=1 clone --upload-pack="git.b upload-pack" parent child2 2>log && + git.a -C child2 log -1 --format=%s >actual && + git.b -C parent log -1 --format=%s >expect && + test_cmp expect actual && + ! grep "version 1" log +' + +test_expect_success "file:// fetch with $VERSION_A and protocol v1" ' + git.b -C parent commit --allow-empty -m two && + git.b -C parent log -1 --format=%s >expect && + + GIT_TRACE_PACKET=1 git.a -C child2 -c protocol.version=1 fetch --upload-pack="git.b upload-pack" 2>log && + git.a -C child2 log -1 --format=%s FETCH_HEAD >actual && + + test_cmp expect actual && + ! grep "version 1" log +' + +test_done -- 2.15.0.rc0.271.g36b669edcc-goog
[PATCH v4 03/11] protocol: introduce protocol extension mechanisms
Create protocol.{c,h} and provide functions which future servers and clients can use to determine which protocol to use or is being used. Also introduce the 'GIT_PROTOCOL' environment variable which will be used to communicate a colon separated list of keys with optional values to a server. Unknown keys and values must be tolerated. This mechanism is used to communicate which version of the wire protocol a client would like to use with a server. Signed-off-by: Brandon Williams--- Documentation/config.txt | 17 +++ Documentation/git.txt| 6 Makefile | 1 + cache.h | 8 + protocol.c | 79 protocol.h | 33 6 files changed, 144 insertions(+) create mode 100644 protocol.c create mode 100644 protocol.h diff --git a/Documentation/config.txt b/Documentation/config.txt index dc4e3f58a..b78747abc 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2517,6 +2517,23 @@ The protocol names currently used by git are: `hg` to allow the `git-remote-hg` helper) -- +protocol.version:: + Experimental. If set, clients will attempt to communicate with a + server using the specified protocol version. If unset, no + attempt will be made by the client to communicate using a + particular protocol version, this results in protocol version 0 + being used. + Supported versions: ++ +-- + +* `0` - the original wire protocol. + +* `1` - the original wire protocol with the addition of a version string + in the initial response from the server. + +-- + pull.ff:: By default, Git does not create an extra merge commit when merging a commit that is a descendant of the current commit. Instead, the diff --git a/Documentation/git.txt b/Documentation/git.txt index 6e3a6767e..7518ea3af 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -697,6 +697,12 @@ of clones and fetches. which feed potentially-untrusted URLS to git commands. See linkgit:git-config[1] for more details. +`GIT_PROTOCOL`:: + For internal use only. Used in handshaking the wire protocol. + Contains a colon ':' separated list of keys with optional values + 'key[=value]'. Presence of unknown keys and values must be + ignored. + Discussion[[Discussion]] diff --git a/Makefile b/Makefile index ed4ca438b..9ce68cded 100644 --- a/Makefile +++ b/Makefile @@ -842,6 +842,7 @@ LIB_OBJS += pretty.o LIB_OBJS += prio-queue.o LIB_OBJS += progress.o LIB_OBJS += prompt.o +LIB_OBJS += protocol.o LIB_OBJS += quote.o LIB_OBJS += reachable.o LIB_OBJS += read-cache.o diff --git a/cache.h b/cache.h index 49b083ee0..c74b73671 100644 --- a/cache.h +++ b/cache.h @@ -445,6 +445,14 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS" #define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH" +/* + * Environment variable used in handshaking the wire protocol. + * Contains a colon ':' separated list of keys with optional values + * 'key[=value]'. Presence of unknown keys and values must be + * ignored. + */ +#define GIT_PROTOCOL_ENVIRONMENT "GIT_PROTOCOL" + /* * This environment variable is expected to contain a boolean indicating * whether we should or should not treat: diff --git a/protocol.c b/protocol.c new file mode 100644 index 0..43012b7eb --- /dev/null +++ b/protocol.c @@ -0,0 +1,79 @@ +#include "cache.h" +#include "config.h" +#include "protocol.h" + +static enum protocol_version parse_protocol_version(const char *value) +{ + if (!strcmp(value, "0")) + return protocol_v0; + else if (!strcmp(value, "1")) + return protocol_v1; + else + return protocol_unknown_version; +} + +enum protocol_version get_protocol_version_config(void) +{ + const char *value; + if (!git_config_get_string_const("protocol.version", )) { + enum protocol_version version = parse_protocol_version(value); + + if (version == protocol_unknown_version) + die("unknown value for config 'protocol.version': %s", + value); + + return version; + } + + return protocol_v0; +} + +enum protocol_version determine_protocol_version_server(void) +{ + const char *git_protocol = getenv(GIT_PROTOCOL_ENVIRONMENT); + enum protocol_version version = protocol_v0; + + /* +* Determine which protocol version the client has requested. Since +* multiple 'version' keys can be sent by the client, indicating that +* the client is okay to speak any of them, select the greatest version +* that the client has requested. This is due to the assumption that +* the most recent protocol version will be
Re: [PATCH v1 1/1] Introduce git add --renormalize .
tbo...@web.de writes: > From: Torsten Bögershausen> > Make it safer to normalize the line endings in a repository: > Files that had been commited with CRLF will be commited with LF. > (Unless core.autorclf and .gitattributes specify that Git > should not do line ending conversions) A few issues I saw after a quick read: - The log message tells us old and new ways, but does not make it clear why users are encouraged to use the new way at all. You didn't make the implementation of "add --renormalize" to just start "git add" without calling read_cache() and letting all files added new to the index (which is how the old way worked) to give a sugarcoated equivalent of the old way for a reason, and that should be desribed in the log. - An ugly global variable is introduced instead of passing necessary information through the callchain properly, but the title does not say PATCH/RFC. - The documentation makes it sound as if this new feature is _only_ about CRLF vs LF. SHouldn't this equally apply after the user changes .gitattributes settings that govern the "clean" side of the filter and makes what is in the index "unclean"? The second point is a showstopper from maintainability's point of view, but none of the above should be insurmojntable. Thanks.
Re: [PATCH v3 10/10] ssh: introduce a 'simple' ssh variant
On 10/03, Jonathan Nieder wrote: > Hi, > > Brandon Williams wrote: > > > When using the 'ssh' transport, the '-o' option is used to specify an > > environment variable which should be set on the remote end. This allows > > git to send additional information when contacting the server, > > requesting the use of a different protocol version via the > > 'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL" > > > > Unfortunately not all ssh variants support the sending of environment > > variables to the remote end. To account for this, only use the '-o' > > option for ssh variants which are OpenSSH compliant. This is done by > > checking that the basename of the ssh command is 'ssh' or the ssh > > variant is overridden to be 'ssh' (via the ssh.variant config). > > This also affects -p (port), right? Yeah I'll add a comment in the commit msg indicating that options like -p and -4 -6 are are only supported by some variants. > > What happens if I specify a ssh://host:port/path URL and the SSH > implementation is of 'simple' type? The port would only be sent if your ssh command supported it. > > > Previously if an ssh command's basename wasn't 'plink' or > > Git's commit messages use the present tense to describe the current > state of the code, so this is "Currently". :) I'll fix this :) > > > 'tortoiseplink' git assumed that the command was an OpenSSH variant. > > Since user configured ssh commands may not be OpenSSH compliant, tighten > > this constraint and assume a variant of 'simple' if the basename of the > > command doesn't match the variants known to git. The new ssh variant > > 'simple' will only have the host and command to execute ([username@]host > > command) passed as parameters to the ssh command. > > > > Update the Documentation to better reflect the command-line options sent > > to ssh commands based on their variant. > > > > Reported-by: Jeffrey Yasskin> > Signed-off-by: Brandon Williams > > Thanks for working on this. > > For background, the GIT_SSH implementation that motivated this is > https://github.com/travis-ci/dpl/blob/6c3fddfda1f2a85944c56b068bac0a77c049/lib/dpl/provider.rb#L215, > which does not support -p or -4/-6, either. > > > --- > > Documentation/config.txt | 27 ++-- > > Documentation/git.txt| 9 ++-- > > connect.c| 107 > > ++- > > t/t5601-clone.sh | 9 ++-- > > t/t5700-protocol-v1.sh | 2 + > > 5 files changed, 95 insertions(+), 59 deletions(-) > [...] > > --- a/connect.c > > +++ b/connect.c > > @@ -776,37 +776,44 @@ static const char *get_ssh_command(void) > [...] > > +static enum ssh_variant determine_ssh_variant(const char *ssh_command, > > + int is_cmdline) > [...] > > - if (!strcasecmp(variant, "plink") || > > - !strcasecmp(variant, "plink.exe")) > > - *port_option = 'P'; > > + if (!strcasecmp(variant, "ssh")) > > + ssh_variant = VARIANT_SSH; > > Could this handle ssh.exe, too? Yeah I'll add the additional comparison. > > [...] > > --- a/t/t5601-clone.sh > > +++ b/t/t5601-clone.sh > > Can this get tests for the new defaulting behavior? E.g. > > - default is "simple" > - how "simple" treats an ssh://host:port/path URL > - how "simple" treats ipv4/ipv6 switching > - ssh defaults to "ssh" > - if GIT_SSH=ssh, can set ssh.variant to "simple" to force the "simple" >mode I'll look to adding a few more tests. > > One other worry: this (intentionally) changes the behavior of a > previously-working GIT_SSH=ssh-wrapper that wants to support > OpenSSH-style options but does not declare ssh.variant=ssh. When > discovering this change, what should the author of such an ssh-wrapper > do? > > They could instruct their users to set ssh.variant or GIT_SSH_VARIANT > to "ssh", but then they are at the mercy of future additional options > supported by OpenSSH we may want to start using in the future (e.g., > maybe we will start passing "--" to separate options from the > hostname). So this is not a futureproof option for them. > > They could take the new default behavior or instruct their users to > set ssh.variant or GIT_SSH_VARIANT to "simple", but then they lose > support for handling alternate ports, ipv4/ipv6, and specifying -o > SendEnv to propagate GIT_PROTOCOL or other envvars. They can handle > GIT_PROTOCOL propagation manually, but losing port support seems like > a heavy cost. > > They could send a patch to define yet another variant that is > forward-compatible, for example using an interface similar to what > git-credential(1) uses. Then they can set GIT_SSH to their > OpenSSH-style helper and GIT_FANCY_NEW_SSH to their more modern > helper, so that old Git versions could use GIT_SSH and new Git > versions could use GIT_FANCY_NEW_SSH. This might be their best > option. It feels odd to say that their only good
Re: Does Git build things during 'make install"?
Am 16.10.2017 um 10:23 schrieb Junio C Hamano: > Johannes Sixtwrites: > >> Yes, running "sudo make install" is a nightmare. sudo clears the path, >> and the git command is not found by the make invoked with root >> permissions. This changes the version string that gets compiled into >> the executable, which finally triggers a complete rebuild under >> root. Sad... > > In the meantime, would it help to intall as yourself under DESTDIR > set to where you can write into, and then limit the potential > damange done while pretending to be a privileged user to "cp -R" (or > "tar cf" in $DESTDIR piped to "tar xf" in /)? > > It appears that some dependencies are screwed up around "perl" > related things, which may want to get fixed. I agree that "make && > make install" that runs two 'make' under the same environment and > user shouldn't (re)build anything during the latter 'make', but we > somehow seem to do so. We do so only, if 'make install' does not run in the same environment and if there is no version file. I use the patch below. It works for me, but I could imagine that it suffers from the original problem if there is no git in PATH and there is no version file, i.e., when the source is not a release tarball. - 8< - Subject: [PATCH] version-gen: Use just built git if no other git is in PATH Signed-off-by: Johannes Sixt --- GIT-VERSION-GEN | 3 +++ 1 file changed, 3 insertions(+) diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 0e88e23653..b610aa3249 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -3,6 +3,9 @@ GVF=GIT-VERSION-FILE DEF_VER=v2.15.0-rc1 +# use git that was just compiled if there is no git elsewhere in PATH +PATH=$PATH:. + LF=' ' -- 2.14.2.808.g3bc32f2729
[PATCH v1 1/1] Introduce git add --renormalize .
From: Torsten BögershausenMake it safer to normalize the line endings in a repository: Files that had been commited with CRLF will be commited with LF. (Unless core.autorclf and .gitattributes specify that Git should not do line ending conversions) The old way to normalize a repo was like this: # Make sure that there are not untracked files $ echo "* text=auto" >.gitattributes $ git read-tree --empty $ git add . $ git commit -m "Introduce end-of-line normalization" The new method is one step shorter, more intuitive and does not add untracked files: $ echo "* text=auto" >.gitattributes $ git add --renormalize . $ git commit -m "Introduce end-of-line normalization" Note that "git add --renormalize " is the short form for "git add -u --renormalize ". Signed-off-by: Torsten Bögershausen --- Documentation/git-add.txt | 8 +++- Documentation/gitattributes.txt | 3 +-- builtin/add.c | 27 +-- cache.h | 1 + convert.c | 1 + environment.c | 1 + read-cache.c| 24 ++-- t/t0025-crlf-renormalize.sh | 30 ++ 8 files changed, 80 insertions(+), 15 deletions(-) create mode 100755 t/t0025-crlf-renormalize.sh diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index f4169fb1ec..b6e431903d 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p] [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] - [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] + [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--renormalize] [--chmod=(+|-)x] [--] [...] DESCRIPTION @@ -172,6 +172,12 @@ for "git add --no-all ...", i.e. ignored removed files. warning (e.g., if you are manually performing operations on submodules). +--renormalize:: + Normalizes the line endings from CRLF to LF of tracked files. + This applies to files which are either "text" or "text=auto" + in .gitattributes (or core.autocrlf is true or input) +--renormalize implies -u + --chmod=(+|-)x:: Override the executable bit of the added files. The executable bit is only changed in the index, the files on disk are left diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index 4c68bc19d5..071dec2bc4 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -232,8 +232,7 @@ From a clean working directory: - $ echo "* text=auto" >.gitattributes -$ git read-tree --empty # Clean index, force re-scan of working directory -$ git add . +$ git add --renormalize . $ git status# Show files that will be normalized $ git commit -m "Introduce end-of-line normalization" - diff --git a/builtin/add.c b/builtin/add.c index a648cf4c56..ee8e756fdc 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -123,6 +123,25 @@ int add_files_to_cache(const char *prefix, return !!data.add_errors; } +static int renormalize_tracked_files(const struct pathspec *pathspec, int flags) +{ + int i, retval = 0; + + for (i = 0; i < active_nr; i++) { + struct cache_entry *ce = active_cache[i]; + + if (ce_stage(ce)) + continue; /* do not touch unmerged paths */ + if (!S_ISREG(ce->ce_mode) && !S_ISLNK(ce->ce_mode)) + continue; /* do not touch non blobs */ + if (pathspec && !ce_path_match(ce, pathspec, NULL)) + continue; + retval |= add_file_to_cache(ce->name, flags); + } + + return retval; +} + static char *prune_directory(struct dir_struct *dir, struct pathspec *pathspec, int prefix) { char *seen; @@ -276,6 +295,7 @@ static struct option builtin_add_options[] = { OPT_BOOL('e', "edit", _interactive, N_("edit current diff and apply")), OPT__FORCE(_too, N_("allow adding otherwise ignored files")), OPT_BOOL('u', "update", _worktree_changes, N_("update tracked files")), + OPT_BOOL(0, "renormalize", _renormalize, N_("renormalize EOL of tracked files (implies -u)")), OPT_BOOL('N', "intent-to-add", _to_add, N_("record only the fact that the path will be added later")), OPT_BOOL('A', "all", _explicit, N_("add changes from all tracked and untracked files")), { OPTION_CALLBACK, 0, "ignore-removal", _explicit, @@ -406,7 +426,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) chmod_arg[1] != 'x' || chmod_arg[2]))
Greetings in the name of God, Business proposal in God we trust
Greetings in the name of God Dear Friend Greetings in the name of God,please let this not sound strange to you for my only surviving lawyer who would have done this died early this year.I prayed and got your email id from your country guestbook. I am Mrs Suran Yoda from London,I am 72 years old,i am suffering from a long time cancer of the lungs which also affected my brain,from all indication my conditions is really deteriorating and it is quite obvious that,according to my doctors they have advised me that i may not live for the next two months,this is because the cancer stage has gotten to a very bad stage.I am married to (Dr Andrews Yoda) who worked with the Embassy of United Kingdom in South Africa for nine years,Before he died in 2004. I was bred up from a motherless babies home and was married to my late husband for Thirty years without a child,my husband died in a fatal motor accident Before his death we were true believers.Since his death I decided not to re-marry,I sold all my inherited belongings and deposited all the sum of $6.5 Million dollars with Bank in South Africa.Though what disturbs me mostly is the cancer. Having known my condition I decided to donate this fund to church,i want you as God fearing person,to also use this money to fund church,orphanages and widows,I took this decision,before i rest in peace because my time will so on be up. The Bible made us to understand that blessed are the hands that giveth. I took this decision because I don`t have any child that will inherit this money and my husband's relatives are not Christians and I don`t want my husband hard earned money to be misused by unbelievers. I don`t want a situation where these money will be used in an ungodly manner,hence the reason for taking this bold decision.I am not afraid of death hence i know where am going.Presently,I'm with my laptop in a hospital here in London where I have been undergoing treatment for cancer of the lungs. As soon as I receive your reply I shall give you the contact of the Bank.I will also issue you a letter of authority that will prove you as the new beneficiary of my fund.Please assure me that you will act accordingly as I stated.Hoping to hear from you soon. Remain blessed in the name of the Lord. Yours in Christ, Mrs Suran Yoda
[PATCH v4 2/3] implement fetching of moved submodules
We store the changed submodules paths to calculate which submodule needs fetching. This does not work for moved submodules since their paths do not stay the same in case of a moved submodules. In case of new submodules we do not have a path in the current checkout, since they just appeared in this fetch. It is more general to collect the submodule names for changes instead of their paths to include the above cases. If we do not have a configuration for a gitlink we rely on constructing a default name from the path if a git repository can be found at its path. We skip non-configured gitlinks whose default name collides with a configured one. With the change described above we implement 'on-demand' fetching of changes in moved submodules. Signed-off-by: Heiko Voigt--- submodule-config.h | 3 + submodule.c | 138 t/t5526-fetch-submodules.sh | 35 +++ 3 files changed, 138 insertions(+), 38 deletions(-) diff --git a/submodule-config.h b/submodule-config.h index e3845831f6..a5503a5d17 100644 --- a/submodule-config.h +++ b/submodule-config.h @@ -22,6 +22,9 @@ struct submodule { int recommend_shallow; }; +#define SUBMODULE_INIT { NULL, NULL, NULL, RECURSE_SUBMODULES_NONE, \ + NULL, NULL, SUBMODULE_UPDATE_STRATEGY_INIT, {0}, -1 }; + struct submodule_cache; struct repository; diff --git a/submodule.c b/submodule.c index 63e7094e16..71d1773e2e 100644 --- a/submodule.c +++ b/submodule.c @@ -21,7 +21,7 @@ #include "parse-options.h" static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF; -static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP; +static struct string_list changed_submodule_names = STRING_LIST_INIT_DUP; static int initialized_fetch_ref_tips; static struct oid_array ref_tips_before_fetch; static struct oid_array ref_tips_after_fetch; @@ -674,11 +674,11 @@ const struct submodule *submodule_from_ce(const struct cache_entry *ce) } static struct oid_array *submodule_commits(struct string_list *submodules, - const char *path) + const char *name) { struct string_list_item *item; - item = string_list_insert(submodules, path); + item = string_list_insert(submodules, name); if (item->util) return (struct oid_array *) item->util; @@ -687,39 +687,67 @@ static struct oid_array *submodule_commits(struct string_list *submodules, return (struct oid_array *) item->util; } +struct collect_changed_submodules_cb_data { + struct string_list *changed; + const struct object_id *commit_oid; +}; + +/* + * this would normally be two functions: default_name_from_path() and + * path_from_default_name(). Since the default name is the same as + * the submodule path we can get away with just one function which only + * checks whether there is a submodule in the working directory at that + * location. + */ +static const char *default_name_or_path(const char *path_or_name) +{ + int error_code; + + if (!is_submodule_populated_gently(path_or_name, _code)) + return NULL; + + return path_or_name; +} + static void collect_changed_submodules_cb(struct diff_queue_struct *q, struct diff_options *options, void *data) { + struct collect_changed_submodules_cb_data *me = data; + struct string_list *changed = me->changed; + const struct object_id *commit_oid = me->commit_oid; int i; - struct string_list *changed = data; for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; struct oid_array *commits; + const struct submodule *submodule; + const char *name; + if (!S_ISGITLINK(p->two->mode)) continue; - if (S_ISGITLINK(p->one->mode)) { - /* -* NEEDSWORK: We should honor the name configured in -* the .gitmodules file of the commit we are examining -* here to be able to correctly follow submodules -* being moved around. -*/ - commits = submodule_commits(changed, p->two->path); - oid_array_append(commits, >two->oid); - } else { - /* Submodule is new or was moved here */ - /* -* NEEDSWORK: When the .git directories of submodules -* live inside the superprojects .git directory some -* day we should fetch new submodules directly into -* that location too when config or options request -* that so
[PATCH v4 3/3] submodule: simplify decision tree whether to or not to fetch
To make extending this logic later easier. Signed-off-by: Heiko Voigt--- submodule.c | 74 ++--- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/submodule.c b/submodule.c index 71d1773e2e..82d206eb65 100644 --- a/submodule.c +++ b/submodule.c @@ -1187,6 +1187,31 @@ struct submodule_parallel_fetch { }; #define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0, 0} +static int get_fetch_recurse_config(const struct submodule *submodule, + struct submodule_parallel_fetch *spf) +{ + if (spf->command_line_option != RECURSE_SUBMODULES_DEFAULT) + return spf->command_line_option; + + if (submodule) { + char *key; + const char *value; + + int fetch_recurse = submodule->fetch_recurse; + key = xstrfmt("submodule.%s.fetchRecurseSubmodules", submodule->name); + if (!repo_config_get_string_const(the_repository, key, )) { + fetch_recurse = parse_fetch_recurse_submodules_arg(key, value); + } + free(key); + + if (fetch_recurse != RECURSE_SUBMODULES_NONE) + /* local config overrules everything except commandline */ + return fetch_recurse; + } + + return spf->default_option; +} + static int get_next_submodule(struct child_process *cp, struct strbuf *err, void *data, void **task_cb) { @@ -1214,46 +1239,21 @@ static int get_next_submodule(struct child_process *cp, } } - default_argv = "yes"; - if (spf->command_line_option == RECURSE_SUBMODULES_DEFAULT) { - int fetch_recurse = RECURSE_SUBMODULES_NONE; - - if (submodule) { - char *key; - const char *value; - - fetch_recurse = submodule->fetch_recurse; - key = xstrfmt("submodule.%s.fetchRecurseSubmodules", submodule->name); - if (!repo_config_get_string_const(the_repository, key, )) { - fetch_recurse = parse_fetch_recurse_submodules_arg(key, value); - } - free(key); - } - - if (fetch_recurse != RECURSE_SUBMODULES_NONE) { - if (fetch_recurse == RECURSE_SUBMODULES_OFF) - continue; - if (fetch_recurse == RECURSE_SUBMODULES_ON_DEMAND) { - if (!unsorted_string_list_lookup(_submodule_names, - submodule->name)) - continue; - default_argv = "on-demand"; - } - } else { - if (spf->default_option == RECURSE_SUBMODULES_OFF) - continue; - if (spf->default_option == RECURSE_SUBMODULES_ON_DEMAND) { - if (!unsorted_string_list_lookup(_submodule_names, - submodule->name)) - continue; - default_argv = "on-demand"; - } - } - } else if (spf->command_line_option == RECURSE_SUBMODULES_ON_DEMAND) { - if (!unsorted_string_list_lookup(_submodule_names, + switch (get_fetch_recurse_config(submodule, spf)) + { + default: + case RECURSE_SUBMODULES_DEFAULT: + case RECURSE_SUBMODULES_ON_DEMAND: + if (!submodule || !unsorted_string_list_lookup(_submodule_names, submodule->name)) continue; default_argv = "on-demand"; + break; + case RECURSE_SUBMODULES_ON: + default_argv = "yes"; + break; + case RECURSE_SUBMODULES_OFF: + continue; } strbuf_addf(_path, "%s/%s", spf->work_tree, ce->name); -- 2.14.1.145.gb3622a4
[PATCH v4 1/3] fetch: add test to make sure we stay backwards compatible
The current implementation of submodules supports on-demand fetch if there is no .gitmodules entry for a submodule. Let's add a test to document this behavior. Signed-off-by: Heiko Voigt--- t/t5526-fetch-submodules.sh | 42 +- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 42251f7f3a..43a22f680f 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -478,7 +478,47 @@ test_expect_success "don't fetch submodule when newly recorded commits are alrea git fetch >../actual.out 2>../actual.err ) && ! test -s actual.out && - test_i18ncmp expect.err actual.err + test_i18ncmp expect.err actual.err && + ( + cd submodule && + git checkout -q master + ) +' + +test_expect_success "'fetch.recurseSubmodules=on-demand' works also without .gitmodule entry" ' + ( + cd downstream && + git fetch --recurse-submodules + ) && + add_upstream_commit && + head1=$(git rev-parse --short HEAD) && + git add submodule && + git rm .gitmodules && + git commit -m "new submodule without .gitmodules" && + printf "" >expect.out && + head2=$(git rev-parse --short HEAD) && + echo "From $pwd/." >expect.err.2 && + echo " $head1..$head2 master -> origin/master" >>expect.err.2 && + head -3 expect.err >>expect.err.2 && + ( + cd downstream && + rm .gitmodules && + git config fetch.recurseSubmodules on-demand && + # fake submodule configuration to avoid skipping submodule handling + git config -f .gitmodules submodule.fake.path fake && + git config -f .gitmodules submodule.fake.url fakeurl && + git add .gitmodules && + git config --unset submodule.submodule.url && + git fetch >../actual.out 2>../actual.err && + # cleanup + git config --unset fetch.recurseSubmodules && + git reset --hard + ) && + test_i18ncmp expect.out actual.out && + test_i18ncmp expect.err.2 actual.err && + git checkout HEAD^ -- .gitmodules && + git add .gitmodules && + git commit -m "new submodule restored .gitmodules" ' test_expect_success 'fetching submodules respects parallel settings' ' -- 2.14.1.145.gb3622a4
[PATCH v4 0/3] implement fetching of moved submodules
The previous RFC iteration can be found here: https://public-inbox.org/git/20171006222544.GA26642@sandbox/ This should now be in a state ready for review for inclusion. The main difference from last iteration is that we now also support unconfigured gitlinks for push and fetch for backwards compatibility. To implement this compatibility we construct a default name for gitlinks if there is a repository found at their location in the worktree. Cheers Heiko Heiko Voigt (3): fetch: add test to make sure we stay backwards compatible implement fetching of moved submodules submodule: simplify decision tree whether to or not to fetch submodule-config.h | 3 + submodule.c | 200 +--- t/t5526-fetch-submodules.sh | 77 - 3 files changed, 210 insertions(+), 70 deletions(-) -- 2.14.1.145.gb3622a4
Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()
"Philip Oakley"writes: > Hi, 'Truncate' is real English, but it is not that common in normal usage. > > My dictionary suggests that it means 'cut off at the tip' such as a > truncated cone. However the thesaurus is far more relaxed about the > common idioms that truncate at the tail such as: clip, crop, cut > short, trim, abbreviate, curtail, etc. > > So perhaps "could not trim '%s'". Truncate is fine, as there is already another instance that barfs with "cannot truncate" upon an error from ftruncate(), and the patch merely matches the two error messages.
Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()
From: "Johannes Schindelin"On Mon, 16 Oct 2017, Junio C Hamano wrote: Ralf Thielow writes: > When the write opertion fails, we write that we could > not read. Change the error message to match the operation > and remove the full stop at the end. > > When ftruncate() fails, we write that we couldn't finish > the operation on the todo file. It is more accurate to write > that we couldn't truncate as we do in other calls of ftruncate(). Wouldn't it be more accurate to say we couldn't ftruncate, though? This is an end-user facing error message, right? Should we not let users who are happily oblivious of POSIX nomenclature remain happily oblivious? In other words, I would be finer with "truncate" than with "ftruncate... wait, huh? Is that even English?" Hi, 'Truncate' is real English, but it is not that common in normal usage. My dictionary suggests that it means 'cut off at the tip' such as a truncated cone. However the thesaurus is far more relaxed about the common idioms that truncate at the tail such as: clip, crop, cut short, trim, abbreviate, curtail, etc. So perhaps "could not trim '%s'". -- Philip
Re: [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name
Hi Junio, On Mon, 16 Oct 2017, Junio C Hamano wrote: > Johannes Schindelinwrites: > > >> >> -Also respects `:remotename` to state the name of the *remote* instead > >> >> of > >> >> -the ref. > >> >> +Also respects `:remotename` to state the name of the *remote* instead > >> >> +of the ref, and `:remoteref` to state the name of the *reference* as > >> >> +locally known by the remote. > >> > > >> > What does "locally known by the remote" mean in this sentence? > >> > >> Good question. I cannot offhand offer a better and concise > >> phrasing, but I think can explain what it wants to describe ;-). > > > > Yep, described it well. > > > > Maybe "and `:remoteref` to state the name by which the remote knows the > > *reference*"? > > I dunno. > > The original and your update both seem to come from a worldview > where there is a single conceptual thing called "reference" that is > shared between our repository and the remote repository we pull from > (or push to), and the name we call it is "refs/remotes/origin/devel" > while the name they use to call it is "refs/heads/devel". If you > subscribe to that worldview, then the updated sentence might make it > clearer than the original. > > But I do not share that worldview, and I am not sure (note: I am > truly unsure---it's not like I am convinced it is a good idea but > sugarcoating my expression, at least in this case) if subscribing to > the worldview helps users' understanding. > > In my view "refs/remotes/origin/devel" is a reference we use to keep > track of (or "a reference that corresponds to") the reference they > have called "refs/heads/devel" at the remote, and these are two > separate entities, so it's not like "this single thing is called > differently by us and them". > > Stepping back a bit; here is how the description begins. > > upstream:: > The name of a local ref which can be considered ``upstream'' > from the displayed ref. > > So 'upstream' is like 'refs/remotes/origin/devel' in the example in > the message you are responding to. Perhaps we can make it clear in > the description, and then add :remote* modifiers are about asking > where that remote-tracking branch comes from. For example, instead > of these "Also respects...", something like: > > For a %(upstream) that is a remote-tracking branch, the name of > the remote repository it is copied from can be obtained with > %(upstream:remotename). Simiarly, the branch at the remote > repository whose tip is copioed to this remote-tracking branch > can be obtined with %(upstream:remoteref) as a full refname. > > may reduce the chance of confusion? Let's take two more steps back. First, for-each-ref is a low-level command (I do not remember whether our nickname for "low-level" is "plumbing" or "porcelain" or anything, so I stick with "low-level" which I deem descriptive enough). That is, users of this command (and therefore, readers of this man page) need to be quite familiar with Git's worldview. Second, the main purpose of this patch series is to answer the question "what is the default in `git push :`?" for *many* refs at once. Maybe it would be better to describe the functionality by describing the question it tries to answer. Ciao, Dscho
Re: [PATCH v2] sequencer.c: fix and unify error messages in rearrange_squash()
Hi Junio, On Mon, 16 Oct 2017, Junio C Hamano wrote: > Ralf Thielowwrites: > > > When the write opertion fails, we write that we could > > not read. Change the error message to match the operation > > and remove the full stop at the end. > > > > When ftruncate() fails, we write that we couldn't finish > > the operation on the todo file. It is more accurate to write > > that we couldn't truncate as we do in other calls of ftruncate(). > > Wouldn't it be more accurate to say we couldn't ftruncate, though? This is an end-user facing error message, right? Should we not let users who are happily oblivious of POSIX nomenclature remain happily oblivious? In other words, I would be finer with "truncate" than with "ftruncate... wait, huh? Is that even English?" Ciao, Dscho
Re: [ANNOUNCE] Git for Windows 2.14.2(3)
Hi Steve, On Sun, 15 Oct 2017, Johannes Schindelin wrote: > On Fri, 13 Oct 2017, Steve Hoelzer wrote: > > > On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin > >wrote: > > > > > > It is my pleasure to announce that Git for Windows 2.14.2(3) is > > > available from: > > > > > > https://git-for-windows.github.io/ > > > > > > Changes since Git for Windows v2.14.2(2) (October 5th 2017) > > > > > > New Features > > > > > > * Comes with Git LFS v2.3.3. > > > > I just ran "git update" and afterward "git version" reported > > 2.14.2(3), but "git lfs version" still said 2.3.2. > > > > I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2. > > Ah bummer. I forgot to actually update it in the VM where I build the > releases :-( > > Will work on it tomorrow. I'll actually use this opportunity to revamp a part of Git for Windows' release engineering process to try to prevent similar things from happening in the future. Also, cURL v7.56.1 is slated to be released in exactly one week, and I have some important installer work to do this week, so I'll just defer the new Git for Windows version tentatively to Monday, 23rd. Git LFS users can always install Git LFS v2.3.3 specifically in the meantime, or use Git for Windows' snapshot versions (https://wingit.blob.core.windows.net/files/index.html). Ciao, Johannes
Re: [PATCH] check-ref-format: require a repository for --branch
Junio C Hamanowrites: > Having said that, there may still be a use case where a Porcelain > script wants a way to see if a $name it has is appropriate as a > branch name before it has a repository (e.g. a wrapper to "git > clone" that wants to verify the name it is going to give to the "-b" > option), and a check desired in such a context is different from > (and is stricter than) feeding refs/heads/$name to the same command > without the "--branch" option. > > So I think the right endgame in the longer term is: > ... Here is to illustrate what I mean in a patch form. It resurrects the gentle setup, and uses a purely textual format check when we are outside the repository, while bypassing the @{magic} interpolation codepath that requires us to be in a repository. When we are in a repository, we operate the same way as before. Designed to be applied directly on top of 4d03f955 ("check-ref-format: require a repository for --branch", 2017-07-14), so it is missing the "'HEAD' is not a good branch name" I showed a few days ago. builtin/check-ref-format.c | 16 +--- cache.h | 14 ++ sha1_name.c | 22 +++--- strbuf.h| 6 ++ t/t1402-check-ref-format.sh | 10 +- 5 files changed, 61 insertions(+), 7 deletions(-) diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c index 1e5f9835f0..4e62852089 100644 --- a/builtin/check-ref-format.c +++ b/builtin/check-ref-format.c @@ -38,12 +38,22 @@ static char *collapse_slashes(const char *refname) static int check_ref_format_branch(const char *arg) { + int nongit, malformed; struct strbuf sb = STRBUF_INIT; + const char *name = arg; - setup_git_directory(); - if (strbuf_check_branch_ref(, arg)) + setup_git_directory_gently(); + + if (!nongit) + malformed = (strbuf_check_branch_ref(, arg) || +!skip_prefix(sb.buf, "refs/heads/", )); + else + malformed = check_branch_ref_format(arg); + + if (malformed) die("'%s' is not a valid branch name", arg); - printf("%s\n", sb.buf + 11); + printf("%s\n", name); + strbuf_release(); return 0; } diff --git a/cache.h b/cache.h index 52b91f5b64..3815925122 100644 --- a/cache.h +++ b/cache.h @@ -1444,6 +1444,20 @@ extern int parse_oid_hex(const char *hex, struct object_id *oid, const char **en #define INTERPRET_BRANCH_HEAD (1<<2) extern int interpret_branch_name(const char *str, int len, struct strbuf *, unsigned allowed); + +/* + * NEEDSWORK: declare strbuf_branchname() and strbuf_check_branch_ref() + * here, not in strbuf.h + */ + +/* + * Check if a 'name' is suitable to be used as a branch name; this can + * be and is stricter than what check_refname_format() returns for a + * string that is a concatenation of "name" after "refs/heads/"; a + * name that begins with "-" is not allowed, for example. + */ +extern int check_branch_ref_format(const char *name); + extern int get_oid_mb(const char *str, struct object_id *oid); extern int validate_headref(const char *ref); diff --git a/sha1_name.c b/sha1_name.c index 5e2ec37b65..c95080258f 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1319,15 +1319,31 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed) strbuf_add(sb, name + used, len - used); } -int strbuf_check_branch_ref(struct strbuf *sb, const char *name) +static int strbuf_check_branch_ref_format(struct strbuf *sb) { - strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL); - if (name[0] == '-') + if (*sb->buf == '-') return -1; strbuf_splice(sb, 0, 0, "refs/heads/", 11); return check_refname_format(sb->buf, 0); } +int check_branch_ref_format(const char *name) +{ + struct strbuf sb = STRBUF_INIT; + int result; + + strbuf_addstr(, name); + result = strbuf_check_branch_ref_format(); + strbuf_release(); + return result; +} + +int strbuf_check_branch_ref(struct strbuf *sb, const char *name) +{ + strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL); + return strbuf_check_branch_ref_format(sb); +} + /* * This is like "get_sha1_basic()", except it allows "sha1 expressions", * notably "xyz^" for "parent of xyz" diff --git a/strbuf.h b/strbuf.h index d785258649..3da95685b2 100644 --- a/strbuf.h +++ b/strbuf.h @@ -568,6 +568,12 @@ static inline void strbuf_complete_line(struct strbuf *sb) strbuf_complete(sb, '\n'); } +/* + * NEEDSWORK: the following two functions should not be in this file; + * these are about refnames, and should be declared next to + * interpret_branch_name() in cache.h + */ + /* * Copy "name" to "sb", expanding any special @-marks as handled by * interpret_branch_name(). The result is a non-qualified branch name diff --git
Re: Does Git build things during 'make install"?
Johannes Sixtwrites: > Yes, running "sudo make install" is a nightmare. sudo clears the path, > and the git command is not found by the make invoked with root > permissions. This changes the version string that gets compiled into > the executable, which finally triggers a complete rebuild under > root. Sad... In the meantime, would it help to intall as yourself under DESTDIR set to where you can write into, and then limit the potential damange done while pretending to be a privileged user to "cp -R" (or "tar cf" in $DESTDIR piped to "tar xf" in /)? It appears that some dependencies are screwed up around "perl" related things, which may want to get fixed. I agree that "make && make install" that runs two 'make' under the same environment and user shouldn't (re)build anything during the latter 'make', but we somehow seem to do so.
DEAR FRIEND.
Dear Friend, I am Mr.Nawfal Nagi the head of file department of Bank of Africa(B.O.A) here in Burkina Faso / Ouagadougou. In my department we discover an abandoned sum of (US$18 million US Dollars) in an account that belongs to one of our foreign customer who died along with his family in plane crash. It is therefore upon this discovery that I now decided to make this business proposal to you and release the money to you as the next of kin or relation to the deceased for the safety and subsequent disbursement since nobody is coming for it. I agree that 40% of this money will be for you, while 60% would be for me. Then after the money is been transferred into your account, I will visit your country for an investment under your kind control. You have to contact my Bank directly as the real next of kin of this deceased account with next of kin application form. You have to send me those your information below to enable me use it and get you next of kin application form from bank, so that you will contact Bank for the transfer of this money into your account. Your Full Name___ Your Home Address Your Age ___ Your Handset Number Your Occupation ___ I am waiting for your urgent respond to enable us proceed further for the transfer through my private e-mail(mrnawfalnag...@gmail.com) Yours faithfully, Mr.Nawfal Nagi
What's cooking in git.git (Oct 2017, #03; Mon, 16)
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. 2.15-rc1 has been tagged, but 2.15-rc2 is going to slip. The topics that are cooking in 'next' that are not urgent fixes are classified as "Will cook in 'next'", and will not graduate to 'master' until the final. We haven't decided how to resolve the "git add -i" regression (see the thread at https://public-inbox.org/git/xmqqzi8vvht6@gitster.mtv.corp.google.com/ for the two approaches), and 'next' has one of them ("demote 'always' to 'auto' when given to color.ui from the configuration file"), while 'pu' has f6b2410f20 that takes a different approach ("It was a mistake to allow plumbing to pay attention to color.ui config, so revert it to unbreak 'add -i'"). 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 -- [New Topics] * jk/ui-color-always-to-auto-maint (2017-10-13) 2 commits (merged to 'next' on 2017-10-13 at bb16e1edc8) + color: document that "git -c color.*=always" is a bit special + color: downgrade "always" to "auto" only for on-disk configuration It turns out that "git -c color.ui=always cmd" is relied on by many third-party tools as a way to force coloured output no matter what the end-user configuration is, and a recent attempt to downgrade 'always' to 'auto' to fix the regression to "git add -p" broke it. * jk/ref-filter-colors-fix (2017-10-14) 4 commits - tag: respect color.ui config - Revert "color: check color.ui in git_default_config()" - Revert "t6006: drop "always" color config tests" - Revert "color: make "always" the same as "auto" in config" This is the "theoretically more correct" approach of simply stepping back to the state before plumbing commands started paying attention to "color.ui" configuration variable. * jc/branch-name-sanity (2017-10-14) 3 commits (merged to 'next' on 2017-10-16 at 174646d1c3) + branch: forbid refs/heads/HEAD + branch: split validate_new_branchname() into two + branch: streamline "attr_only" handling in validate_new_branchname() "git branch" and "git checkout -b" are now forbidden from creating a branch whose name is "HEAD". Will cook in 'next'. * jk/revision-pruning-optim (2017-10-14) 1 commit (merged to 'next' on 2017-10-16 at 2662baa21d) + revision: quit pruning diff more quickly when possible Pathspec-limited revision traversal was taught not to keep finding unneeded differences once it knows two trees are different inside given pathspec. Will cook in 'next'. * js/rebase-i-final (2017-10-16) 1 commit (merged to 'next' on 2017-10-16 at 72362f5f9c) + sequencer.c: fix and unify error messages in rearrange_squash() Error message fix. Will merge to 'master'. * wk/merge-options-gpg-sign-doc (2017-10-12) 1 commit (merged to 'next' on 2017-10-16 at ae61d824da) + Documentation/merge-options.txt: describe -S/--gpg-sign for 'pull' Doc updates. Will cook in 'next'. * wk/pull-signoff (2017-10-13) 1 commit (merged to 'next' on 2017-10-16 at 5e48f349d9) + pull: pass --signoff/--no-signoff to "git merge" "git pull" has been taught to accept "--[no-]signoff" option and pass it down to "git merge". Will cook in 'next'. * sb/diff-color-move (2017-10-16) 1 commit (merged to 'next' on 2017-10-16 at 69de1bad9d) + diff: fix infinite loop with --color-moved --ignore-space-change A recently added "--color-moved" feature of "diff" fell into infinite loop when ignoring whitespace changes, which has been fixed. Will merge to 'master'. -- [Stalled] * mk/use-size-t-in-zlib (2017-08-10) 1 commit . zlib.c: use size_t for size The wrapper to call into zlib followed our long tradition to use "unsigned long" for sizes of regions in memory, which have been updated to use "size_t". Needs resurrecting by making sure the fix is good and still applies (or adjusted to today's codebase). * mg/status-in-progress-info (2017-05-10) 2 commits - status --short --inprogress: spell it as --in-progress - status: show in-progress info for short status "git status" learns an option to report various operations (e.g. "merging") that the user is in the middle of. cf.* nd/worktree-move (2017-04-20) 6 commits - worktree remove: new command - worktree move: refuse to move worktrees with submodules - worktree move: accept destination as directory - worktree move: new command - worktree.c: add update_worktree_location() - worktree.c: add validate_worktree() "git worktree" learned move and remove subcommands. Expecting a reroll. cf. <20170420101024.7593-1-pclo...@gmail.com>
Re: [PATCH] check-ref-format: require a repository for --branch
Jeff Kingwrites: > On Mon, Jul 17, 2017 at 10:27:09AM -0700, Jonathan Nieder wrote: >> ... >> I don't think it's right. Today I can do >> >> $ cd /tmp >> $ git check-ref-format --branch master >> master >> >> You might wonder why I'd ever do such a thing. But that's what "git >> check-ref-format --branch" is for --- it is for taking a >> argument and turning it into a branch name. For example, if you have >> a script with an $opt_branch variable that defaults to "master", it >> may do >> >> resolved_branch=$(git check-ref-format --branch "$opt_branch") >> >> even though it is in a mode that not going to have to use >> $resolved_branch and it is not running from a repository. > > I'm not sure I buy that. What does "turning it into a branch name" even > mean when you are not in a repository? Clearly @{-1} must fail. And > everything else is just going to output the exact input you provided. > So any script calling "check-ref-format --branch" outside of a repo > would be better off not calling it at all. I threw this topic in stalled category, hoping that one or the other opinion eventually turns out to be more prevalent, but it didn't seem to have happened X-<. Things like @{-1} would not make any sense when the command is run outside a repository, and the documentation is quite clear that it is the primary reason why we added "--branch" option to the command, i.e. With the `--branch` option, it expands the ``previous branch syntax'' `@{-n}`. For example, `@{-1}` is a way to refer the last branch you were on. This option should be used by porcelains to accept this syntax anywhere a branch name is expected, so they can act as if you typed the branch name. So I am tempted to take this patch to make sure that we won't gain more people who abuse the command outside a repository. Having said that, there may still be a use case where a Porcelain script wants a way to see if a $name it has is appropriate as a branch name before it has a repository (e.g. a wrapper to "git clone" that wants to verify the name it is going to give to the "-b" option), and a check desired in such a context is different from (and is stricter than) feeding refs/heads/$name to the same command without the "--branch" option. So I think the right endgame in the longer term is: - Find (or add if it doesn't exist) a way to recommend to Porcelain scripts to use to expand an end-user generated string, and to map it to a branch name (it may be "rev-parse --symbolic-full-name $name"; I dunno). - Keep check-ref-format as (or revert it to be) a tool to "check". This would involve split strbuf_check_branch_ref() into two: - one that does not do the @{-1} thing and is used ONLY for format validity check (including rejecting a name that begins with a dash, which is OK for a random ref but not acceptable as a branch name); - the other that does @{-1} thing before doing the above. and then making the code call the former, not the latter. The end result would be that check-ref-format becomes textual check only, and can be usable (agian) outside repository, with or without "--branch". As the current code does not allow us do that yet, I think it is safer to forbid use of --branch outside the repository for now, purely as a bugfix. [Footnote] *1* In a sense, @{-1} is not something the scripts need to check its validity of---it is the branch you came from, so by definition it must be with a good name. What the scripts want is instead see what the branch actually is, which is not what "check-ref-format" is about. a31dca03 ("check-ref-format --branch: give Porcelain a way to grok branch shorthand", 2009-03-21) says: The command may not be the best place to add this new feature, but $ git check-ref-format --branch "@{-1}" allows Porcelains to figure out what branch you were on before the last branch switching.