Re: [PATCH] tag: add tag.gpgSign config option to force all tags be GPG-signed

2017-10-26 Thread Jonathan Nieder
Hi again, Mkrtchyan, Tigran wrote: > Jonathan Nieder wrote: >> Tigran Mkrtchyan wrote: >>> In some workflows we have no control on how git command is executed, >>> however a signed tags are required. >> >> Don't leave me hanging: this leaves me super

Re: [PATCH] tag: add tag.gpgSign config option to force all tags be GPG-signed

2017-10-26 Thread Jonathan Nieder
Hi, Tigran Mkrtchyan wrote: > In some workflows we have no control on how git command is executed, > however a signed tags are required. Don't leave me hanging: this leaves me super curious. Can you tell me more about these workflows? Thanks and hope that helps, Jonathan

Re: Consequences of CRLF in index?

2017-10-26 Thread Jonathan Nieder
Johannes Sixt wrote: > Am 26.10.2017 um 13:01 schrieb Lars Schneider: >> * -text >> *.sh text eol=lf > > Why would that be necessary? I cannot have CRLF in shell scripts etc., not > even on Windows. (And in addition I do not mind CR in C++ source code.) All > I want is: Git, please, by all

Re: Consequences of CRLF in index?

2017-10-25 Thread Jonathan Nieder
Hi again, Lars Schneider wrote: >> On 24 Oct 2017, at 20:14, Jonathan Nieder <jrnie...@gmail.com> wrote: >> In any event, you also probably want to declare what you're doing >> using .gitattributes. By checking in the files as CRLF, you are >> declaring that you d

Re: [PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

2017-10-25 Thread Jonathan Nieder
Hi, Stefan Beller wrote: > On Wed, Oct 25, 2017 at 5:51 AM, Johannes Schindelin > wrote: >> This breaks on Windows. And it is not immediately obvious how so, so let >> me explain: [nice explanation snipped] >> As a consequence, the test fails. Could you please squash

Re: [PATCH 3/4] xdiff: use stronger hash function internally

2017-10-24 Thread Jonathan Nieder
Stefan Beller wrote: > --- a/xdiff/xutils.c > +++ b/xdiff/xutils.c > @@ -24,7 +24,8 @@ > #include > #include "xinclude.h" > > - > +#include "cache.h" > +#include "hashmap.h" #includes like "git-compat-util.h" and "cache.h" can only be used as the *first* #include. Otherwise the feature

Re: Consequences of CRLF in index?

2017-10-24 Thread Jonathan Nieder
Hi, Lars Schneider wrote: > I've migrated a large repo (110k+ files) with a lot of history (177k commits) > and a lot of users (200+) to Git. Unfortunately, all text files in the index > of the repo have CRLF line endings. In general this seems not to be a problem > as the project is developed

Re: [PATCH v2] column: show auto columns when pager is active

2017-10-23 Thread Jonathan Nieder
Junio C Hamano wrote: > Subject: column: do not include pager.c > > Everything this file needs from the pager API (e.g. term_columns(), > pager_in_use()) is already declared in the header file it includes. > > Noticed-by: Jonathan Nieder <jrnie...@gmail.com> > Signed-o

[PATCH 5/5] ssh: 'simple' variant does not support --port

2017-10-23 Thread Jonathan Nieder
to OpenSSH would also support -G and would not require such an update.) Reported-by: William Yan <w...@google.com> Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> Acked-by: Stefan Beller <sbel...@google.com> --- That's the end of the series. Thanks for reading. connect.c

[PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

2017-10-23 Thread Jonathan Nieder
is-ci/dpl/blob/6c3fddfda1f2a85944c56b068bac0a77c049/lib/dpl/provider.rb#L215 Reported-by: William Yan <w...@google.com> Improved-by: Jonathan Tan <jonathanta...@google.com> Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> --- This is the main one. Simplified by

[PATCH 4/5] ssh: 'simple' variant does not support -4/-6

2017-10-23 Thread Jonathan Nieder
If the user passes -4/--ipv4 or -6/--ipv6 to "git fetch" or "git push" and the ssh command configured with GIT_SSH does not support such a setting, error out instead of ignoring the option and continuing. Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> Ack

[PATCH 2/5] connect: split ssh command line options into separate function

2017-10-23 Thread Jonathan Nieder
The git_connect function is growing long. Split the portion that discovers an ssh command and options it accepts before the service name and path to a separate function to make it easier to read. No functional change intended. Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> Re

[PATCH 1/5] connect: split git:// setup into a separate function

2017-10-23 Thread Jonathan Nieder
The git_connect function is growing long. Split the PROTO_GIT-specific portion to a separate function to make it easier to read. No functional change intended. Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> Reviewed-by: Stefan Beller <sbel...@google.com> --- As be

[PATCH v2 0/5] Coping with unrecognized ssh wrapper scripts in GIT_SSH

2017-10-23 Thread Jonathan Nieder
Jonathan Tan wrote: >> On 10/23, Jonathan Nieder wrote: >>> If this looks good, I can reroll in a moment. > > Yes, this looks good. Thanks. Here goes. The interdiff is upthread. Thanks, all, for the quick review. Jonathan Nieder (5): connect: split git:// setup int

Re: [PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

2017-10-23 Thread Jonathan Nieder
Brandon Williams wrote: > On 10/23, Jonathan Nieder wrote: >> Separately from how to document it, what do you think a good behavior >> would be? Should the "auto" configuration trigger command line based >> detection just like no configuration a

Re: [PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

2017-10-23 Thread Jonathan Nieder
Stefan Beller wrote: > On Mon, Oct 23, 2017 at 2:31 PM, Jonathan Nieder <jrnie...@gmail.com> wrote: >> 1. First, check whether $GIT_SSH supports OpenSSH options by running >> >> $GIT_SSH -G >> >> This returns status 0 and prints configuration

Re: [PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

2017-10-23 Thread Jonathan Nieder
Hi, Jonathan Tan wrote: > The new documentation seems to imply that setting ssh.variant (or > GIT_SSH_VARIANT) to "auto" is equivalent to not setting it at all, but > looking at the code, it doesn't seem to be the case (not setting it at > all invokes checking the first word of core.sshCommand,

Re: [PATCH v2] column: show auto columns when pager is active

2017-10-23 Thread Jonathan Nieder
Hi, Kevin Daudt wrote: > --- a/column.c > +++ b/column.c > @@ -5,6 +5,7 @@ > #include "parse-options.h" > #include "run-command.h" > #include "utf8.h" > +#include "pager.c" Should this be pager.h? Thanks, Jonathan

[PATCH 5/5] ssh: 'simple' variant does not support --port

2017-10-23 Thread Jonathan Nieder
to OpenSSH would also support -G and would not require such an update.) Reported-by: William Yan <w...@google.com> Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> --- That's the end of the series. Thanks for reading. connect.c| 15 --- t/t5601-clone.sh

[PATCH 4/5] ssh: 'simple' variant does not support -4/-6

2017-10-23 Thread Jonathan Nieder
If the user passes -4/--ipv4 or -6/--ipv6 to "git fetch" or "git push" and the ssh command configured with GIT_SSH does not support such a setting, error out instead of ignoring the option and continuing. Signed-off-by: Jonathan Nieder <jrnie...@gmail.com>

[PATCH 3/5] ssh: 'auto' variant to select between 'ssh' and 'simple'

2017-10-23 Thread Jonathan Nieder
without the autodetection step (1), as before. [*] https://github.com/travis-ci/dpl/blob/6c3fddfda1f2a85944c56b068bac0a77c049/lib/dpl/provider.rb#L215 Reported-by: William Yan <w...@google.com> Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> --- repo fix: https://gerrit-review.googlesour

[PATCH 2/5] connect: split ssh command line options into separate function

2017-10-23 Thread Jonathan Nieder
The git_connect function is growing long. Split the portion that discovers an ssh command and options it accepts before the service name and path to a separate function to make it easier to read. No functional change intended. Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> --- con

[PATCH 1/5] connect: split git:// setup into a separate function

2017-10-23 Thread Jonathan Nieder
The git_connect function is growing long. Split the PROTO_GIT-specific portion to a separate function to make it easier to read. No functional change intended. Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> --- connect.c

[PATCH 0/5] Coping with unrecognized ssh wrapper scripts in GIT_SSH

2017-10-23 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > On 10/03, Jonathan Nieder wrote: >> 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. Thanks again for this patch. Wi

Re: [PATCH v3] Documentation/git-config.txt: reword missleading sentence

2017-10-18 Thread Jonathan Nieder
Hi Nathan et al, PAYRE NATHAN p1508475 wrote: > From: PAYRE NATHAN p1508475 nit: this 'From' line doesn't match any of the authors with sign-offs below. I'm wondering if the authorship of the commit (from "git commit --author" or git's "user.name" / "user.email"

Re: [PATCH 3/3] check-ref-format doc: --branch validates and expands

2017-10-17 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder <jrnie...@gmail.com> writes: >> From: Junio C Hamano <gits...@pobox.com> >> >> "git check-ref-format --branch $name" feature was originally >> introduced (and was advertised) as a way for scripts to take any

Re: [PATCH] fetch doc: src side of refspec could be full SHA-1

2017-10-17 Thread Jonathan Nieder
when is empty. is most > +typically a ref, but it can also be an fully spelled hex object > +name. nits: s/most typically/typically/ s/an fully/a fully/ With those tweaks, Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> Thanks.

Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads

2017-10-17 Thread Jonathan Nieder
eters anyway. > > Signed-off-by: Rene Scharfe <l@web.de> > --- > Documentation/git-ls-remote.txt | 1 - > builtin/ls-remote.c | 4 +++- > 2 files changed, 3 insertions(+), 2 deletions(-) Yes, I think this is a good step. Reviewed-by: Jonathan Nieder <jrnie.

Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP

2017-10-17 Thread Jonathan Nieder
.c | 1 + > git.c| 2 +- > t/t0012-help.sh | 1 + > t/t5512-ls-remote.sh | 6 ++ > 4 files changed, 9 insertions(+), 1 deletion(-) Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> [...] > --- a/git.c > +++ b/git.c > @@ -421,7 +

Re: [PATCH 1/2] git: add NO_INTERNAL_HELP flag for builtins

2017-10-17 Thread Jonathan Nieder
ty pitfalls in it I don't know about? It's in POSIX, so it looks pretty safe. I would have been tempted to use comm, which is already used in t6500-gc.sh: comm -1 -3 no_internal_help builtins >builtins_internal_help Other nits: - preparatory 'cat' commands like the above tend to go inside test_expect

Re: [PATCH] check-ref-format: require a repository for --branch

2017-10-17 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder <jrnie...@gmail.com> writes: >> 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 out

Re: [PATCH] check-ref-format: require a repository for --branch

2017-10-17 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder <jrnie...@gmail.com> writes: >> 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 out

Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

2017-10-17 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder <jrnie...@gmail.com> writes: >> Handles the nongit case in strbuf_check_branch_ref instead of >> introducing a new check_branch_ref_format helper. > > I view that as a regression, actually. Don't we want a function > that

[PATCH 3/3] check-ref-format doc: --branch validates and expands

2017-10-17 Thread Jonathan Nieder
From: Junio C Hamano "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 it is usable when Git expects to see a branch name, and also

[PATCH 2/3] check-ref-format --branch: strip refs/heads/ using skip_prefix

2017-10-17 Thread Jonathan Nieder
f-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> --- builtin/check-ref-format.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c index 6c40ff110b..bc67d3f0a8 1

[PATCH 1/3] check-ref-format --branch: do not expand @{...} outside repository

2017-10-17 Thread Jonathan Nieder
conditional to strbuf_check_branch_ref instead of its caller; fleshed out commit message; some style tweaks in tests] Reported-by: Marko Kungla <marko.kun...@gmail.com> Helped-by: Jeff King <p...@peff.net> Signed-off-by: Junio C Hamano <gits...@pobox.com> Signed-off-by:

[PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch

2017-10-17 Thread Jonathan Nieder
Hi, Junio C Hamano wrote: > Subject: [PATCH] check-ref-format: --branch cannot grok @{-1} outside a > repository How about this? It is written to be more conservative than the patch I am replying to, but except for the commit message, it should be pretty much equivalent. [...] > ---

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

2017-10-17 Thread Jonathan Nieder
Hi, Junio C Hamano wrote: > Jeff King writes: >> On Sat, Oct 14, 2017 at 12:01:46PM +0900, Junio C Hamano wrote: >>> 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

Re: [PATCH] check-ref-format: require a repository for --branch

2017-10-16 Thread Jonathan Nieder
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

Re: [PATCH] check-ref-format: require a repository for --branch

2017-10-16 Thread Jonathan Nieder
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 >>

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

2017-10-11 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder <jrnie...@gmail.com> writes: >> Should we document this special case treatment of color.* in -c >> somewhere? E.g. > > Perhaps, although I'd save that until we actually add the new option > to "git" potty, which

Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration

2017-10-11 Thread Jonathan Nieder
> + > + /* > + * Otherwise, we're looking at on-disk config; > + * downgrade to auto. > + */ > + return GIT_COLOR_AUTO; > + } Yes, this looks good to me. Should we document this special case tr

Re: [PATCH 2/2] color: discourage use of ui.color=always

2017-10-11 Thread Jonathan Nieder
2 files changed, 8 insertions(+), 1 deletion(-) This warning only kicks in when `always` is being silently downgraded to `auto`. It makes sense to me. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH] doc: emphasize stash "--keep-index" stashes staged content

2017-10-11 Thread Jonathan Nieder
Hi, Robert P. J. Day wrote: > It's not immediately obvious from the man page that the "--keep-index" > option still adds the staged content to the stash, so make that > abundantly clear. > > Signed-off-by: Robert P. J. Day > --- > > diff --git

Re: git repack leaks disk space on ENOSPC

2017-10-11 Thread Jonathan Nieder
Hi Andreas, Andreas Krey wrote: > I observed (again) an annoying behavior of 'git repack': Do you have context for this 'again'? E.g. was this discussed previously on-list? > When the new pack file cannot be fully written because > the disk gets full beforehand, the tmp_pack file isn't >

Re: Unable to use --patch with git add

2017-10-11 Thread Jonathan Nieder
Hi Ayush, Ayush Goel wrote: > Thank you for your mail. It works :) > > For future reference, is there a page for known issues of git? We usually try not to have known issues that would require such a warning for long. And when we fail, reminders like yours are very welcome, though a search

Re: v2.15.0-rc1 test failure

2017-10-11 Thread Jonathan Nieder
Hi, Adam Dinwoodie wrote: > t0021.15 has PERL as a requirement, and I see semi-regular failures from > Git tests that are Perl-based in one way or another (git-svn tests are > the most common problems). I've not spotted t0021 failing in that way, > but it sounds like the same class of problem.

Re: [PATCH] checkout doc: clarify command line args for "checkout paths" mode

2017-10-10 Thread Jonathan Nieder
you use the interactive interface to show > + the "diff" output and choose which hunks to use in the > + result. See below for the descriptoin of `--patch` option. nit: s/descriptoin/description/ With that tweak, Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> Thanks.

Re: [PATCH] checkout doc: clarify command line args for "checkout paths" mode

2017-10-10 Thread Jonathan Nieder
Hi, Junio C Hamano wrote: [...] > The source of the confusion is that -p(atch) is described as if it > is just another "optional" part and its description is lumped > together with the non patch mode, even though the actual end user > experience is vastly different. Makes sense. This should

Re: [PATCH v3 03/10] protocol: introduce protocol extention mechanisms

2017-10-10 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > Given some of this discussion though, maybe we want to change the > semantics of 'protocol.version' so that both servers and clients respect > it. As of right now, these patches have the server always allow > protocol v0 and v1? Though that doesn't do much right

Re: What happened to "git status --color=(always|auto|never)"?

2017-10-10 Thread Jonathan Nieder
Hi, Jeff King wrote: > On Tue, Oct 10, 2017 at 09:51:38PM +0900, Junio C Hamano wrote: >> I think the right fix to the original problem (you cannot remove >> auto-color from the plumbing) is to stop paying attention to color >> configuration from the default config. I wonder if something like

Re: [PATCH v2 13/24] builtin/pack-objects: convert to struct object_id

2017-10-09 Thread Jonathan Nieder
> --- > builtin/pack-objects.c | 135 > + > 1 file changed, 68 insertions(+), 67 deletions(-) Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH v2 12/24] pack-bitmap: convert traverse_bitmap_commit_list to object_id

2017-10-09 Thread Jonathan Nieder
| 4 ++-- > pack-bitmap.c | 8 > pack-bitmap.h | 2 +- > 4 files changed, 11 insertions(+), 11 deletions(-) Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH v2 11/24] refs: convert dwim_log to struct object_id

2017-10-09 Thread Jonathan Nieder
brian m. carlson wrote: > Signed-off-by: brian m. carlson > --- > builtin/reflog.c | 4 ++-- > reflog-walk.c| 2 +- > refs.c | 8 > refs.h | 2 +- > sha1_name.c | 2 +- > 5 files changed, 9 insertions(+), 9 deletions(-)

Re: [PATCH v2 10/24] builtin/reflog: convert remaining unsigned char uses to object_id

2017-10-09 Thread Jonathan Nieder
changed, 9 insertions(+), 9 deletions(-) Thanks. Looks good. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH v2 09/24] refs: convert dwim_ref and expand_ref to struct object_id

2017-10-09 Thread Jonathan Nieder
brian m. carlson wrote: > All of the callers of these functions just pass the hash member of a > struct object_id, so convert them to use a pointer to struct object_id > directly. > > Signed-off-by: brian m. carlson > --- > archive.c | 2 +- > branch.c

Re: [PATCH v2 08/24] refs: convert read_ref and read_ref_full to object_id

2017-10-09 Thread Jonathan Nieder
ex, lock_file, COMMIT_LOCK)) > die(_("unable to write new index file")); > > - read_ref_full("HEAD", 0, rev.hash, NULL); > + read_ref_full("HEAD", 0, , NULL); Yep, this is nicer (and likewise for the rest of them). Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: What happened to "git status --color=(always|auto|never)"?

2017-10-09 Thread Jonathan Nieder
Nazri Ramliy wrote: > On Tue, Oct 10, 2017 at 8:16 AM, Jonathan Nieder <jrnie...@gmail.com> wrote: >> Nazri Ramliy wrote: >>> I used to work before, but now: >>> >>> $ git version >>> git version 2.15.0.rc0.39.g2f0e14e649 >>> >>>

Re: [PATCH] submodule: spell out API of submodule_move_head

2017-10-09 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder <jrnie...@gmail.com> writes: >> Junio C Hamano wrote: >>> This is not a new issue (the removed comment did not mention this at >>> all), but is it correct to say that updates to "index and work tree" >

Re: [PATCH] submodule: spell out API of submodule_move_head

2017-10-09 Thread Jonathan Nieder
Junio C Hamano wrote: > Stefan Beller writes: >> +/** >> + * Move the HEAD and content of the active submodule at 'path' from object >> id >> + * 'old' to 'new'. >> + * >> + * Updates the submodule at 'path' and files in its work tree to commit >> + * 'new'. The commit

Re: What happened to "git status --color=(always|auto|never)"?

2017-10-09 Thread Jonathan Nieder
Hi, Nazri Ramliy wrote: > I used to work before, but now: > > $ git version > git version 2.15.0.rc0.39.g2f0e14e649 > > $ git status --color=always > error: unknown option `color=always' > usage: git status [] [--] ... Which version did it work in? That would allow me to bisect. Thanks,

Re: [PATCH v2 07/24] refs: convert resolve_refdup and refs_resolve_refdup to struct object_id

2017-10-09 Thread Jonathan Nieder
--word-diff, I also see some line-wrapping changes, which all seem reasonable. (Coccinelle's line-wrapping heuristics seem to be pretty specific to Linux kernel style.) In other words, this does what it says on the cover in a straightforward and reviewable way. Thanks for that. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH v2 06/24] Convert check_connected to use struct object_id

2017-10-09 Thread Jonathan Nieder
; builtin/receive-pack.c | 10 +- > connected.c| 18 +- > connected.h| 4 ++-- > 5 files changed, 20 insertions(+), 20 deletions(-) Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH v2 05/24] refs: update ref transactions to use struct object_id

2017-10-09 Thread Jonathan Nieder
ference does not exist > * already. > * > * See the above comment "Reference transaction updates" for more > @@ -535,35 +535,35 @@ int ref_transaction_update(struct ref_transaction > *transaction, > */ (Possible diff generation bug: there's exactly one line represent

Re: [PATCH v2 04/24] refs: convert update_ref and refs_update_ref to use struct object_id

2017-10-09 Thread Jonathan Nieder
int pick_commits(struct todo_list *todo_list, > struct replay_opts *opts) > } > msg = reflog_message(opts, "finish", "%s onto %s", > head_ref.buf, buf.buf); > - if (update_ref(msg,

Re: [PATCH v2 03/24] refs: convert delete_ref and refs_delete_ref to struct object_id

2017-10-09 Thread Jonathan Nieder
re passed through to > + * deleting it. If old_oid is NULL, delete the reference if it > + * exists, regardless of its old value. It is an error for old_oid to > + * be null_oid. msg and flags are passed through to > * ref_transaction_delete(). > */ > int refs_delete_ref(struct ref_store *refs, const char *msg, Thanks for updating the comment. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH v2 02/24] refs/files-backend: convert struct ref_to_prune to object_id

2017-10-09 Thread Jonathan Nieder
provement to the clarity of the code. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH v2 01/24] walker: convert to struct object_id

2017-10-09 Thread Jonathan Nieder
_targets_stdin needs an overflow check to avoid overflowing its "int". [...] > @@ -321,7 +321,7 @@ int walker_fetch(struct walker *walker, int targets, char > **target, > done: > ref_transaction_free(transaction); > free(msg); > - free(sha1); > + free(oids); Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH] submodule: spell out API of submodule_move_head

2017-10-09 Thread Jonathan Nieder
Stefan Beller wrote: > This way users of this function do not need to read the implementation to > know what it will do. > > Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> > Signed-off-by: Stefan Beller <sbel...@google.com> > --- > submodule

Re: [Question] Documenting platform implications on CVE to git

2017-10-06 Thread Jonathan Nieder
Hi, Randall S. Becker wrote: > The first one, mostly. When looking at CVE-2017-14867, there are places like > https://nvd.nist.gov/vuln/detail/CVE-2017-14867 where the issue is > discussed. It provides hyperlinks to various platform discussions. > Unfortunately for me, I am not an HPE employee -

Re: [Question] Documenting platform implications on CVE to git

2017-10-06 Thread Jonathan Nieder
Hi Randall, Randall S. Becker wrote: > I wonder whether there is some mechanism for providing official responses > from platform ports relating to security CVE reports, like CVE-2017-14867. This question is too abstract for me. Can you say more concretely what you are trying to do? E.g. are

Re: [PATCH 2/2] tests: fix diff order arguments in test_cmp

2017-10-06 Thread Jonathan Nieder
- Yes, this should make the output from failing tests easier to take in at a glance. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> How did you find these? E.g. is there a grep pattern that reviewers can use to repeat your results?

Re: [PATCH 1/2] tests: use shell negation instead of test_must_fail for test_cmp

2017-10-06 Thread Jonathan Nieder
Hi, Jeff King wrote: > On Fri, Oct 06, 2017 at 12:00:05PM -0700, Stefan Beller wrote: >> The `test_must_fail` should only be used to indicate a git command is >> failing. `test_cmp` is not a git command, such that it doesn't need the >> special treatment of `test_must_fail` (which e.g. includes

Re: [PATCH 1/2] tests: use shell negation instead of test_must_fail for test_cmp

2017-10-06 Thread Jonathan Nieder
the world given to us sanely works. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> I wonder if it would be useful to have a nod to that advice in the docstring in t/test-lib-functions.sh, too.

Re: is there a truly compelling rationale for .git/info/exclude?

2017-10-06 Thread Jonathan Nieder
Hi, Robert P. J. Day wrote: > On Fri, 6 Oct 2017, Junio C Hamano wrote: >> Don't waste time by seeking a "compelling" reason. A mere "this is >> the most expedite way to gain convenience" back when something was >> introduced could be an answer, and it is way too late to complain >> about such

Re: Git config multiple values

2017-10-06 Thread Jonathan Nieder
Hi, Jeff King wrote: > On Fri, Oct 06, 2017 at 01:10:17PM +0200, aleksander.baranowski wrote: >> It's just an opinion, but this behaviour is no consistent for me. >> >> If it's not the bug it's a feature just let me know. > > It's a feature, though I agree that git-config is rather baroque.

Re: [PATCH v2] api-argv-array.txt: Remove broken link to string-list API

2017-10-05 Thread Jonathan Nieder
hen, prevent the broken link from making > it to the end-user documentation. > > Signed-off-by: Todd Zullinger <t...@pobox.com> > --- > Documentation/technical/api-argv-array.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Jonathan Nieder <jrnie...@gmai

Re: [RFC] Reporting index properties

2017-10-05 Thread Jonathan Nieder
Alex Vandiver wrote: > As part of gathering some automated performance statistics of git, it > would be useful to be able to extract some vital properties of the > index. While `git update-index` has ways to _set_ the index version, > and enable/disable various extensions, AFAICT there is no

Re: couple questions about git "logical variables" and "git var"

2017-10-05 Thread Jonathan Nieder
Jeff King wrote: > On Thu, Oct 05, 2017 at 05:11:04AM -0400, rpj...@crashcourse.ca wrote: >> - GIT_AUTHOR_IDENT >> - GIT_COMMITTER_IDENT >> - GIT_EDITOR >> - GIT_PAGER >> >> first question -- what is it about precisely those four variables that makes >> them "logical" variables in git

Re: [PATCH 0/6] Teach Status options around showing ignored files

2017-10-05 Thread Jonathan Nieder
Hi, jameson.mille...@gmail.com wrote: > This patch series is the second part of [1], which was split into 2 > parts. The first part, added an optimization in the directory listing > logic to not scan the contents of ignored directories and was merged > to master with commit 5aaa7fd3. This patch

Re: [PATCH] .mailmap: normalize name for René Scharfe

2017-10-05 Thread Jonathan Nieder
ortlog -nse" run confirms that this works. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> Thanks. > diff --git a/.mailmap b/.mailmap > index ab85e0d16d..224db83887 100644 > --- a/.mailmap > +++ b/.mailmap > @@ -194,6 +194,7 @@ Philippe Bruhat <b...@cpan.org> > R

Re: [PATCH v2] ref-filter.c: pass empty-string as NULL to atom parsers

2017-10-04 Thread Jonathan Nieder
ot;refname:") or "does not take args" > (e.g. "body:") as an error message. > > Signed-off-by: Taylor Blau <m...@ttaylorr.com> > Reviewed-by: Jeff King <p...@peff.net> > Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> > Signed-off-by: Junio C

Re: Line ending normalization doesn't work as expected

2017-10-04 Thread Jonathan Nieder
Junio C Hamano wrote: > Jonathan Nieder <jrnie...@gmail.com> writes: >> git checkout --renormalize . >> git status; # Show files that will be normalized >> git commit; # Commit the result >> >> What do you think? Wou

Re: [PATCH 1/3] path.c: fix uninitialized memory access

2017-10-04 Thread Jonathan Nieder
, which knows not to go beyond the > end of the string. > > Reported-by: Thomas Gummerer <t.gumme...@gmail.com> > Signed-off-by: Jeff King <p...@peff.net> > Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> This is indeed Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> Thanks.

Re: disable interactive prompting

2017-10-04 Thread Jonathan Nieder
Hi Ernesto, Ernesto Alfonso wrote: > Waiting for git-push synchronously slows me down, so I have a bash > alias/function to do this in the background. But when my origin is https, I > get an undesired interactive prompt. I've tried to disable by > redirecting stdin: > > git push ${REMOTE}

Re: Line ending normalization doesn't work as expected

2017-10-04 Thread Jonathan Nieder
Hi Robert, Robert Dailey wrote: > You guys are obviously worlds ahead of me on the internals of things, > but from my perspective I like to avoid the "plumbing" commands as > much as I can. I suspect what we are dancing around is the need for some command like git checkout

[PATCH v2] strbuf doc: reuse after strbuf_release is fine

2017-10-03 Thread Jonathan Nieder
to strbuf_reset for that. The same semantics apply to strbuf_detach. Add a similar note to its docstring to make that clear. Improved-by: Jeff King <p...@peff.net> Signed-off-by: Jonathan Nieder <jrnie...@gmail.com> --- Jeff King wrote: > I think it's actually OK to use the stri

Re: Git for Windows: mingw.c's strange usage of creation time as ctime?

2017-10-03 Thread Jonathan Nieder
+git-for-windows Hi Marc, Marc Strapetz wrote: > compat/mingw.c assigns the Windows file creation time [1] to Git's > ctime fields at line 591 and at line 705: > > buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime)); > > ftCreationTime > ftLastWriteTime is actually possible after copying

Re: [PATCH 0/3] fixes for running the test suite with --valgrind

2017-10-03 Thread Jonathan Nieder
Jeff King wrote: > I think using SANITIZE=memory would catch these, but it needs some > suppressions tuning. The weird "zlib reads uninitialized memory" error > is a problem (valgrind sees this, too, but we have suppressions). What version of zlib do you use? I've heard some good things about

Re: [PATCH 1/3] path.c: fix uninitialized memory access

2017-10-03 Thread Jonathan Nieder
Jeff King wrote: > On Tue, Oct 03, 2017 at 03:45:01PM -0700, Jonathan Nieder wrote: >> In other words, an alternative fix would be >> >> if (*path == '.' && path[1] == '/') { >> ... >> } >> >> which would not

Re: [PATCH] test-stringlist: avoid buffer underrun when sorting nothing

2017-10-03 Thread Jonathan Nieder
etion(-) Thanks for fixing it. Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Re: [PATCH 2/3] http-push: fix construction of hex value from path

2017-10-03 Thread Jonathan Nieder
Hi, Thomas Gummerer wrote: > The get_oid_hex_from_objpath takes care of creating a oid from a > pathname. It does this by memcpy'ing the first two bytes of the path to > the "hex" string, then skipping the '/', and then copying the rest of the > path to the "hex" string. Currently it fails to

Re: [PATCH 1/3] path.c: fix uninitialized memory access

2017-10-03 Thread Jonathan Nieder
Hi, Thomas Gummerer wrote: > In cleanup_path we're passing in a char array, run a memcmp on it, and > run through it without ever checking if something is in the array in the > first place. This can lead us to access uninitialized memory, for > example in t5541-http-push-smart.sh test 7, when

Re: [PATCH] branch: reset instead of release a strbuf

2017-10-03 Thread Jonathan Nieder
Hi, Stefan Beller wrote: > Our documentation advises to not re-use a strbuf, after strbuf_release > has been called on it. Use the proper reset instead. > > Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> This is indeed Reviewed-by: Jonathan Nieder <jrnie...@gmail.com&g

Re: [bug] git version 2.4.12 color.ui = always breaks git add -p

2017-10-03 Thread Jonathan Nieder
Hi Christian, Christian Rebischke wrote: > I played around with my gitconfig and I think I found a bug while doing > so. I set the following lines in my gitconfig: > > [color] > ui = always > > When I try to use `git add -p ` I don't see the 'Stage > this hunk'-dialog anymore. First I

Re: [PATCH] branch: reset instead of release a strbuf

2017-10-03 Thread Jonathan Nieder
for (i = 0; i < argc; i++, strbuf_reset()) { > char *target = NULL; > int flags = 0; Should there be a strbuf_release (or UNLEAK if you are very very sure) call at the end of the function to replace this? With that change (but not without it), Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> Thanks.

Re: [PATCH v3 10/10] ssh: introduce a 'simple' ssh variant

2017-10-03 Thread Jonathan Nieder
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

Re: [PATCH v2] request-pull: capitalise "Git" to make it a proper noun

2017-10-02 Thread Jonathan Nieder
ads > >"...in the Git repository at:" > > Besides, this brings us in line with the documentation, see > Documentation/howto/using-signed-tag-in-pull-request.txt > > Signed-off-by: Ann T Ropea <bedhan...@gmx.de> > Acked-by: Jonathan Nieder <jrnie...@

Security of .git/config and .git/hooks

2017-10-02 Thread Jonathan Nieder
Hi, This topic has been mentioned on this mailing list before but I had trouble finding a relevant reference. Links welcome. Suppose that I add the following to .git/config in a repository on a shared computer: [pager] log = rm -fr / fsck = rm -fr /

Re: [PATCH] PR msg: capitalise "Git" to make it a proper noun

2017-10-02 Thread Jonathan Nieder
Hi, Thanks for working to improve Git! Bedhanger wrote: > Subject: [PATCH] PR msg: capitalise "Git" to make it a proper noun nit: What is a PR msg? Looking with "git log git-request-pull.sh", I see that previous patches called the subsystem request-pull, so this could be

<    2   3   4   5   6   7   8   9   10   11   >