Re: [PATCH v1 0/2] Fix default macOS build locally and on Travis CI

[Torsten Bögershausen]

On 10/11/16 00:39, Junio C Hamano wrote:


I've followed what was available at the public-inbox archive, but it
is unclear what the conclusion was.

For the first one your "how about" non-patch, to which Peff said
"that's simple and good", looked good to me as well, but is it
available as a final patch that I can just take and apply (otherwise
I think I can do the munging myself, but I'd rather be spoon-fed
when able ;-).


If you can munge the Peff version that may be easiest ?
I couldn't find a branch in your repo, but am happy to review
whatever shows up there and in pu.

What's cooking in git.git (Nov 2016, #01; Wed, 9)

[Junio C Hamano]

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.

Hopefully 2.11-rc1 can be tagged by the end of the week.  Sorry for
the delay (and thanks for great help from y'all, especially Peff).

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* cc/split-index-typofix (2016-11-01) 1 commit
 - split-index: s/eith/with/ typo fix

 Typofix in a comment in code


* jk/no-looking-at-dotgit-outside-repo (2016-11-01) 1 commit
 - sha1_name: make wraparound of the index into ring-buffer explicit

 A small code cleanup.


* rs/cocci (2016-11-01) 1 commit
 - cocci: avoid self-references in object_id transformations

 Improve the rule to convert "unsigned char [20]" into "struct
 object_id *" in contrib/coccinelle/

--
[New Topics]

* as/merge-attr-sleep (2016-11-08) 1 commit
  (merged to 'next' on 2016-11-09 at 17fbe796e6)
 + t6026-merge-attr: don't fail if sleep exits early

 Fix for a racy false-positive test failure.

 Will merge to 'master'.


* jk/alt-odb-cleanup (2016-11-08) 1 commit
  (merged to 'next' on 2016-11-09 at f7463a1abc)
 + alternates: re-allow relative paths from environment

 Fix a corner-case regression in a topic that graduated during the
 v2.11 cycle.

 Will merge to 'master'.


* jk/filter-process-fix (2016-11-02) 4 commits
  (merged to 'next' on 2016-11-09 at 535b4f4de9)
 + t0021: fix filehandle usage on older perl
 + t0021: use $PERL_PATH for rot13-filter.pl
 + t0021: put $TEST_ROOT in $PATH
 + t0021: use write_script to create rot13 shell script

 Test portability improvements and cleanups for t0021.

 Will merge to 'master'.


* js/prepare-sequencer (2016-11-08) 1 commit
 - sequencer: silence -Wtautological-constant-out-of-range-compare

 Silence a clang warning introduced by a recently graduated topic.

 Will merge to 'master'.


* ls/filter-process (2016-11-08) 2 commits
  (merged to 'next' on 2016-11-09 at 7d217ebb5e)
 + t0021: compute file size with a single process instead of a pipeline
 + t0021: expect more variations in the output of uniq -c

 Test portability improvements and optimization for an
 already-graduated topic.

 Will merge to 'master'.


* jc/retire-compaction-heuristics (2016-11-02) 3 commits
 - SQUASH???
 - SQUASH???
 - diff: retire the original experimental "compaction" heuristics


* jc/abbrev-autoscale-config (2016-11-01) 1 commit
 - config.abbrev: document the new default that auto-scales


* jk/nofollow-attr-ignore (2016-11-02) 5 commits
 - exclude: do not respect symlinks for in-tree .gitignore
 - attr: do not respect symlinks for in-tree .gitattributes
 - exclude: convert "check_index" into a flags field
 - attr: convert "macro_ok" into a flags field
 - add open_nofollow() helper


* sb/submodule-config-cleanup (2016-11-02) 3 commits
 - submodule-config: clarify parsing of null_sha1 element
 - submodule-config: rename commit_sha1 to commit_or_tree
 - submodule config: inline config_from_{name, path}

--
[Stalled]

* hv/submodule-not-yet-pushed-fix (2016-10-10) 3 commits
 - batch check whether submodule needs pushing into one call
 - serialize collection of refs that contain submodule changes
 - serialize collection of changed submodules

 The code in "git push" to compute if any commit being pushed in the
 superproject binds a commit in a submodule that hasn't been pushed
 out was overly inefficient, making it unusable even for a small
 project that does not have any submodule but have a reasonable
 number of refs.

 Waiting for review.
 cf. 


* sb/push-make-submodule-check-the-default (2016-10-10) 2 commits
 - push: change submodule default to check when submodules exist
 - submodule add: extend force flag to add existing repos

 Turn the default of "push.recurseSubmodules" to "check" when
 submodules seem to be in use.

 Will hold to wait for hv/submodule-not-yet-pushed-fix


* jc/bundle (2016-03-03) 6 commits
 - index-pack: --clone-bundle option
 - Merge branch 'jc/index-pack' into jc/bundle
 - bundle v3: the beginning
 - bundle: keep a copy of bundle file name in the in-core bundle header
 - bundle: plug resource leak
 - bundle doc: 'verify' is not about verifying the bundle

 The beginning of "split bundle", which could be one of the
 ingredients to allow "git clone" traffic off of the core server
 network to CDN.

 While I think it would make it easier for people to experiment and
 build on if the topic is merged to 'next', I am at the same time a
 bit reluctant to merge an unproven new topic that introduces a new
 file format, 

Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate

[Markus Hitter]

Am 10.11.2016 um 00:31 schrieb Ian Jackson:
> I am proposing to set this configuration setting automatically in
> dgit.  Other tools that work with particular git tags would do the
> same.  There would be no need for users to do anything.
> 
> Having this as an option in a menu would be quite wrong, because it
> would end up with the user and the tooling fighting.  This is why I
> don't want to put this in gitk's existing config file mechanism.

Having this conversation watched for a while I get the impression that your 
point is essentially about introducing another type of references, next to 
branches and (ordinary) tags. Let's call them "interesting tags", "itags".

The logical path to get this, IMHO, isn't to add some configuration variable, 
but to store such interesting tags in .git/refs/itags/, just like the other 
reference types. Then one would create such interesting tags with

  git tag -i 
or
  git tag --interesting 

To reduce the backwards compatibility problem these itags could be stored in 
.git/refs/tags as well, itag-aware tools would sort the duplicates out.


Markus

-- 
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Junio C Hamano]

Jeff King  writes:

> On Wed, Nov 09, 2016 at 02:58:37PM -0800, Junio C Hamano wrote:
>
> I'm slightly confused. Did you mean "supporting any in-tree symlink to
> an out-of-tree destination" in your first sentence?

I was trying to say that these "control files used solely by git"
have no business being a symbolic link pointing at anywhere, even
inside the same tree; actually, especially if it is inside the same
tree.

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Jeff King]

On Wed, Nov 09, 2016 at 04:18:29PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Wed, Nov 09, 2016 at 02:58:37PM -0800, Junio C Hamano wrote:
> >
> > I'm slightly confused. Did you mean "supporting any in-tree symlink to
> > an out-of-tree destination" in your first sentence?
> 
> I was trying to say that these "control files used solely by git"
> have no business being a symbolic link pointing at anywhere, even
> inside the same tree; actually, especially if it is inside the same
> tree.

OK. That is what my patch does (modulo .gitmodules, which I did not
think of). But I think that is the opposite of Duy's opinion, as his
review seemed to object to that.

As you know my ulterior motive is dealing with malicious out-of-tree
symlinks, and I would be happy to deal with that directly. That still
leaves symlinked ".gitmodules" etc in a funny state (they work in the
filesystem but not in the index), but since nobody is _complaining_,
it's a bug we could leave for another day.

So I dunno.

-Peff

Re: 2.11.0-rc1 will not be tagged for a few days

[Junio C Hamano]

Jeff King  writes:

> I think we also need to make a final decision on the indent/compaction
> heuristic naming. After reading Michael's [0], and the claim by you and
> Stefan that the "compaction" name was declared sufficiently experimental
> that we could later take it away, I'm inclined to follow this plan:
>
>   1. Ship v2.11 with what is in master; i.e., both compaction and indent
>  heuristics, triggerable by config or command line.
>
>   2. Post-v2.11, retire the compaction heuristic as a failed experiment.
>  Keeping it in v2.11 doesn't hurt anything (it was already
>  released), and lets us take our time coming up with and cooking the
>  patch.
>
>   3. Post-v2.11, flip the default for diff.indentHeuristic to "true".
>  Keep at least the command line option around indefinitely for
>  experimenting (i.e., "this diff looks funny; I wonder if
>  --no-indent-heuristic makes it look better").
>
>  Config option can either stay or go at that point. I have no
>  preference.
>
> The nice thing about that plan is it punts on merging any new code to
> post-v2.11. :)

OK, I can go with that plan.  Thanks for an outline.

Re: [PATCH v1 0/2] Fix default macOS build locally and on Travis CI

[Junio C Hamano]

larsxschnei...@gmail.com writes:

> From: Lars Schneider 
>
> Apple removed the OpenSSL header files in macOS and therefore Git does
> not build out of the box on macOS anymore. See previous discussion with
> Torsten here: http://public-inbox.org/git/565b3036.8000...@web.de/
>
> This mini series makes Git build out of the box on macOS, again, and
> disables the HTTPD tests on macOS TravisCI as they don't work anymore
> with the new macOS TravisCI default image:
> https://blog.travis-ci.com/2016-10-04-osx-73-default-image-live/
>
> Thanks,
> Lars
>
>
> Lars Schneider (2):
>   config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11
>   travis-ci: disable GIT_TEST_HTTPD for macOS
>
>  .travis.yml  | 3 ++-
>  config.mak.uname | 6 ++
>  2 files changed, 8 insertions(+), 1 deletion(-)

I've followed what was available at the public-inbox archive, but it
is unclear what the conclusion was.  

For the first one your "how about" non-patch, to which Peff said
"that's simple and good", looked good to me as well, but is it
available as a final patch that I can just take and apply (otherwise
I think I can do the munging myself, but I'd rather be spoon-fed
when able ;-).

I do not have a strong opinion on the second one.  For an interim
solution, disabling webserver tests certainly is expedite and safe,
so I am fine taking it as-is, but I may have missed strong
objections.

Thanks.

Re: 2.11.0-rc1 will not be tagged for a few days

[Junio C Hamano]

Jeff King  writes:

> It's just that I found myself writing up notes like "this should be
> merged", and it occurred to me that I could communicate the same things
> by sending you a proposed history. So I'm curious if you find dissecting
> it via "git log" more or less convenient than a list of notes. :)

Yes, I think this is a very readable form of communication.

What's especially nice in what you did is that comparing outputs of
these two commands

$ git log --no-merges --oneline master..peff/for-junio/master
$ git log --no-merges --oneline ^pu master..peff/for-junio/master

clearly shows that you reused what I already had in 'pu' and the
ones missing from 'pu' are the ones I need to check and possibly
may want to sign-off myself.

I also need to note that while this is very handy format for the
recipient, hence a very good way to do "pass the pumpkin" between
maintainers, it is a less "open" way of communication than laying
out everything on the list (cf. http://youtu.be/L8OOzaqS37s), but
it is the most appropriate method in this case, I would think.

> It looks like Johannes prefers replacements for some of the fixes from
> yesterday.

As to that change, I agree with you that I do not care too deeply
about the shape of an interim step as long as the finished product
uses the better one between the alternatives, but at the same time,
because we are including it as a hot-fix in a released product, even
though it is an interim step in the bigger picture, I want it to be
using the better one, too.  So I am leaning towards taking what you
queued for me for the reasons you stated in the other thread [*1*].

That would probably mean Dscho needs a bit of rebase in the
remaining parts of his series that adds new elements to the enum,
but I am hoping that he can afford that time now in the feature
freeze period.  If we take the _INVALID thing instead now, the
remaining series from Dscho that we would review and queue for the
next cycle would need to begin with a "fixup" patch that stops doing
the _INVALID thing and instead using the "compare after casting to
size_t", so it is just the matter of doing the adjustment before or
after the remainder of the series are posted on the list for the
next release, and the smaller number of "oops that was not nice,
let's fix it" patches in the published history, the better the long
term health of the project, I would think.

The other fixes in your collection looked all sensible.  Thanks.


[Footnote]

*1* The convention of keeping _INVALID at the end can be arguably
made safe from future programmer screw-ups, as long as it is
guarded by a good comment nearby.  But the solution to cast
would avoid having to have that potential brittleness in the
first place.  I find the other merit of taking care of negative
enum automatically quite attractive, too.

Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate

[Ian Jackson]

Junio C Hamano writes ("Re: [PATCH 5/6] config docs: Provide for config to 
specify tags not to abbreviate"):
> Ian Jackson  writes:
> > This is not correct, because as I have explained, this should be a
> > per-tree configuration:
> 
> I do not have fundamental opposition to make it part of .git/config,
> but the name "gitk.something" or if you are enhancing git-gui at the
> time perhaps "gui.something" would be appropriate.  
> 
> But it is still silly to have this kind of information that is very
> specific to Gitk in two places, one that is pretty Gitk specific
> that core-git does not know anything about, the other that are part
> of the configuration storage of the core-git.  In the longer term,
> it is necessary for them to be accessible from gitk's "Edit ->
> Preferences" mechanism somehow, I would think, rather than forcing
> users to sometimes go to GUI to tweak and sometimes run "git config".

I am proposing to set this configuration setting automatically in
dgit.  Other tools that work with particular git tags would do the
same.  There would be no need for users to do anything.

Having this as an option in a menu would be quite wrong, because it
would end up with the user and the tooling fighting.  This is why I
don't want to put this in gitk's existing config file mechanism.

It would be wrong for dgit to edit the user's gitk config file, for
many reasons.

To put it another way, this setting is a way for a tool like dgit to
communicate with gitk (or other programs which have to make guesses
about how prominently to present certain information to the user).
It's not intended to be a way for users, certainly not non-expert
users, to communicate with gitk.

The way I have structured my proposed patches in gitk would make it
easy to provide a gui option to adjust these settings.  Such a gui
option ought to save its value in the gitk config file, and those
values ought to override what comes from `git config'.

But such a system would not obviate the need for a legitimate way for
programs like dgit to communicate with gitk.

Thanks,
Ian.

-- 
Ian Jackson    These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Jeff King]

On Wed, Nov 09, 2016 at 02:58:37PM -0800, Junio C Hamano wrote:

> Duy Nguyen  writes:
> 
> > Let's err on the safe side and disable symlinks to outside repo by
> > default (or even all symlinks on .gitattributes and .gitignore as the
> > first step)
> >
> > What I learned from my changes in .gitignore is, if we have not
> > forbidden something, people likely find some creative use for it.
> 
> Yup.  Supporting any symlink in-tree is like requiring Git to be
> used only on symlink-capable filesystems.  Not allowing it sounds
> like a very sensible option and unlike true contents, there is no
> downside to give that limitation to things like .git.

I'm slightly confused. Did you mean "supporting any in-tree symlink to
an out-of-tree destination" in your first sentence?

> Shouldn't we do the same for .gitmodules while we are at it?

Good catch. Though I am inclined to have a flag that just covers all
out-of-tree symlinks, regardless of names.

-Peff

Re: [PATCH v5 01/16] Git.pm: add subroutines for commenting lines

[Junio C Hamano]

Jakub Narębski  writes:

>> I prefer to have like this instead
>> 
>> sub prefix_lines {
>> my $prefix = shift;
>> my $string = join("\n", @_);
>> $string =~ s/^/$prefix/mg;
>> return $string;
>> }
>> 
>> So both subroutines can take several strings as arguments.
>
> I like the interface, but the implementation looks a bit inefficient.
> Why not simply:
> ...
> If those strings can contain embedded newlines (so that they can be
> called as in Junio example), then your solution is a must-be
>
>   sub prefix_lines {
>   my $prefix = shift;
>   my $string = join("\n", @_);
>   $string =~ s/^/$prefix/mg;
>   return $string;
>   }
>
> Well, nevermind then

OK, so in short, is that a "Reviewed-by:" from you ;-)?

I agree with the conclusion.  Thanks for a review.

Re: git mergetool indefinite hang on version 2.10+

[Jeff King]

On Wed, Nov 09, 2016 at 02:54:59PM -0800, Mike Dacre wrote:

> I have no idea how to debug this. As of git version 2.10.0 git hangs
> indefinitely when I run `git mergetool`, the result is the same if I
> run `git mergetool --tool-help`.

Mergetool is a shell script. Try setting GIT_TRACE=1 in the environment
to see which git programs it's running. Of course it may be
hanging in a non-git program, too. Try:

  PATH=$(git --exec-path):$PATH
  sh -x $(which git-mergetool) --tool-help

to get a dump from the shell.

> I am on Arch Linux, with all of my software updated to the latest
> versions as of this morning. git 2.9.3 and lower works perfectly.
> 
> Any ideas?

If it works on v2.9.3 and the bug reproduces, you should be able to use
"git bisect" to find the commit that introduces the problem. There
weren't a lot of changes to the shell scripts in that time frame, so
just as a guess, it may be related to d323c6b64 (i18n: git-sh-setup.sh:
mark strings for translation, 2016-06-17), which was the only
significant change in that time.

-Peff

Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate

[Junio C Hamano]

Ian Jackson  writes:

>> But I think the right
>> place to do so would be Edit -> Preferences menu in Gitk, and the
>> settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
>> gitk-specific place.
>
> This is not correct, because as I have explained, this should be a
> per-tree configuration:

I do not have fundamental opposition to make it part of .git/config,
but the name "gitk.something" or if you are enhancing git-gui at the
time perhaps "gui.something" would be appropriate.  

But it is still silly to have this kind of information that is very
specific to Gitk in two places, one that is pretty Gitk specific
that core-git does not know anything about, the other that are part
of the configuration storage of the core-git.  In the longer term,
it is necessary for them to be accessible from gitk's "Edit ->
Preferences" mechanism somehow, I would think, rather than forcing
users to sometimes go to GUI to tweak and sometimes run "git config".

Re: [PATCH 0/3] gitk: memory consumption improvements

[Junio C Hamano]

Markus Hitter  writes:

> Am 08.11.2016 um 22:37 schrieb Junio C Hamano:
>> Are all semi-modern Tcl/Tk in service have this -undo thing so that
>> we can pass unconditionally to the text widget like the patch does?
>
> Good point. As far as my research goes, this flag was introduced in Nov. 2001:
>
> http://core.tcl.tk/tk/info/5265df93d207cec0

Sounds safe enough then ;-)

Thanks for an additional research.

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Junio C Hamano]

Duy Nguyen  writes:

> Let's err on the safe side and disable symlinks to outside repo by
> default (or even all symlinks on .gitattributes and .gitignore as the
> first step)
>
> What I learned from my changes in .gitignore is, if we have not
> forbidden something, people likely find some creative use for it.

Yup.  Supporting any symlink in-tree is like requiring Git to be
used only on symlink-capable filesystems.  Not allowing it sounds
like a very sensible option and unlike true contents, there is no
downside to give that limitation to things like .git.

Shouldn't we do the same for .gitmodules while we are at it?

Re: Regarding "git log" on "git series" metadata

[Junio C Hamano]

Josh Triplett  writes:

>> This would definitely need protocol extension when transferring
>> objects across repositories.
>
> It'd also need a repository format extension locally.  Otherwise, if you
> ever touched that repository with an older git (or a tool built on an
> older libgit2 or JGit or other library), you could lose data.

True.  Thanks for sanity-checking me.

git mergetool indefinite hang on version 2.10+

[Mike Dacre]

Hi all,

I have no idea how to debug this. As of git version 2.10.0 git hangs
indefinitely when I run `git mergetool`, the result is the same if I
run `git mergetool --tool-help`.

The result is identical regardless of which version of vim I have
installed, or where vim in installed. It is also the same if I remove
my .gitconfig file.

I am on Arch Linux, with all of my software updated to the latest
versions as of this morning. git 2.9.3 and lower works perfectly.

Any ideas?

Thanks,

Mike

Re: [PATCHv2 32/36] pathspec: allow querying for attributes

[Stefan Beller]

On Wed, Nov 9, 2016 at 1:57 AM, Duy Nguyen  wrote:
> On Sat, Oct 29, 2016 at 1:54 AM, Stefan Beller  wrote:
>> The pathspec mechanism is extended via the new
>> ":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
>> requires paths to not just match the given pattern but also have the
>> specified attrs attached for them to be chosen.
>>
>> Signed-off-by: Stefan Beller 
>> Signed-off-by: Junio C Hamano 
>> ---
>>  Documentation/glossary-content.txt |  20 +
>>  dir.c  |  35 
>
> Pathspec can be processed in a couple more places. The big two are
> match_pathspec and tree_entry_interesting, the former traverses a list
> while the latter does a tree. You don't have to implement attr
> matching in tree_entry_interesting right now because nobody needs it,
> probably. But you need to make sure if somebody accidentally calls
> tree_entry_interesting with an attr pathspec, then it should
> die("BUG"), not silently ignore attr.
>

I am looking into this now.

Re: [PATCH] sequencer: shut up clang warning

[Jakub Narębski]

W dniu 09.11.2016 o 14:56, Johannes Schindelin pisze:

> When comparing a value of type `enum todo_command` with a value that is
> outside the defined enum constants, clang greets the developer with this
> warning:
> 
>   comparison of constant 2 with expression of type
>   'const enum todo_command' is always true
> 
> While this is arguably true *iff* the value was never cast from a
> free-form int, we should keep the cautious code in place.
> 
> To shut up clang, we simply introduce an otherwise pointless enum constant
> and compare against that.
> 
> Noticed by Torsten Bögershausen.
> 
> Signed-off-by: Johannes Schindelin 
> ---

>  sequencer.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 5fd75f3..f80e9c0 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -619,7 +619,8 @@ static int allow_empty(struct replay_opts *opts, struct 
> commit *commit)
>  
>  enum todo_command {
>   TODO_PICK = 0,
> - TODO_REVERT
> + TODO_REVERT,
> + TODO_INVALID
>  };

Why not name it TODO_N, or N_TODO, or something like that?

-- 
Jakub Narębski

Re: [PATCH v5 01/16] Git.pm: add subroutines for commenting lines

[Jakub Narębski]

W dniu 09.11.2016 o 18:02, Vasco Almeida pisze:
> A Ter, 08-11-2016 às 17:06 -0800, Junio C Hamano escreveu:
>> Vasco Almeida  writes:

>>> +sub comment_lines {
>>> +   my $comment_line_char = config("core.commentchar") || '#';
>>> +   return prefix_lines("$comment_line_char ", @_);
>>> +}
>>> +
>>
>> This makes it appear as if comment_lines can take arbitrary number
>> of strings as its arguments (because the outer caller just passes @_
>> thru), but in fact because prefix_lines ignores anything other than
>> $_[0] and $_[1], only the first parameter given to comment_lineS sub
>> is inspected for lines in it and the prefix-char prefixed at the
>> beginning of each of them.
>>
>> Which is not a great interface, as it is quite misleading.
>>
>> Perhaps
>>
>>  prefix_lines("#", join("\n", @_));
>>
>> or something like that may make it less confusing.
> 
> I prefer to have like this instead
> 
> sub prefix_lines {
> my $prefix = shift;
> my $string = join("\n", @_);
> $string =~ s/^/$prefix/mg;
> return $string;
> }
> 
> So both subroutines can take several strings as arguments.

I like the interface, but the implementation looks a bit inefficient.
Why not simply:

  sub prefix_lines {
  my $prefix = shift;
  return "$prefix" . join("\n$prefix", @_) . "\n";
  }

That is, if we can assume that those lines are not terminated by
newlines themselves.

If they can be (but cannot have embedded newlines), then

  sub prefix_lines {
  my $prefix = shift;
  return "$prefix" . join("\n$prefix", map(chomp, @_)) . "\n";
  }

If those strings can contain embedded newlines (so that they can be
called as in Junio example), then your solution is a must-be

  sub prefix_lines {
  my $prefix = shift;
  my $string = join("\n", @_);
  $string =~ s/^/$prefix/mg;
  return $string;
  }

Well, nevermind then
-- 
Jakub Narębski

Re: [PATCH 32/36] pathspec: allow querying for attributes

[Junio C Hamano]

Stefan Beller  writes:

> On Wed, Nov 9, 2016 at 1:45 AM, Duy Nguyen  wrote:
>> On Fri, Oct 28, 2016 at 1:29 AM, Junio C Hamano  wrote:
>>> ...
>>> The strategy this round takes to make it unnecessary to punt
>>> preloading (i.e. dropping "pathspec: disable preload-index when
>>> attribute pathspec magic is in use" patch the old series had) is to
>>> make the attribute subsystem thread-safe.  But that thread-safety in
>>> the initial round is based on a single Big Attribute Lock, so it may
>>> turn out that the end result performs better for this codepath if we
>>> did not to make any call into the attribute subsystem.
>>
>> It does sound good and I want to say "yes please do this", but is it
>> making pathspec api a bit more complex (to express "assume all
>> attr-related criteria match")? I guess we can have an api to simply
>> filter out attr-related magic (basically set attr_match_nr back to
>> zero) then pass a safe (but more relaxing) pathspec to the threaded
>> code. That would not add big maintenance burden.
>
> So with the current implementation, we already have the shortcut as:
>
>  if (item->attr_match_nr && !match_attrs(name, namelen, item))
>
> i.e. if attr_match_nr is zero, we do not even look at the mutexes and such,
> so I am not sure what you intend to say in this email?

I am not sure what relevance the "we call into attribute subsystem
only when there is any need to check attributes" obvious short-cut
has to what is being discussed.

The issue is specific to what preloading is about.  It is merely an
attempt to run cheap checks that could be easily multi-threaded with
multiple threads early in the program that we _know_ we would need
to eventually refresh the index before doing some interesting work.
A full refresh_index() will be done eventually, and because it needs
to trigger thread-unsafe part of the API, it needs to be done in the
main thread.  Doing the preload allows us to mark index entries that
do not have to be scanned again in the upcoming refresh_index()
call.  It is OK for preload-index.c::preload_thread() to skip and
not mark some index entries (iow, its sole purpose is to leave a
note in each index entry "this is fresh, you do not need to look at
it again", and it can choose to skip an entry, which essentially
means "this I didn't check, so you, refresh_index(), need to check
yourself").

preload_thread() for example skips index entries that needs to
trigger "racy Git avoidance" logic that is heavyweight (it has to go
to the filesystem and the object store), and it is a sensible thing
to skip because they are rather rare.

The message by Duy you are responding to was his response to me who
wondered if the attribute based pathspec match also falls into the
same category.  Just like racy Git code was deemed too heavyweight
to be called from preloading codepath and CE_MATCH_RACY_IS_DIRTY bit
was added as a way to ask ie_match_stat() API to avoid it (and hence
we are skipping, the caller is also telling "if you suspect a racy,
without checking for real, just answer 'I cannot say it is
clean/fresh'"), if we invent a new flag and pass it through
match_pathspec() down to match_pathspec_item() and have that if()
statement you quoted also skip match_attrs() for a pathspec element
with attribute based narrowing (as we are skipping, the flag may
also have say "instead of checking the attributes, just pretend that
the path did not satisfy the attribute narrowing"), would it benefit
the overall performance?

To answer Duy's "would it make sense to force the caller to create a
new pathspec from an existing one by filtering out pathspec elements
with attr-based narrowing?" question, I do not think it does.

[PATCH] remote-curl: don't hang when a server dies before any output

[David Turner]

In the event that a HTTP server closes the connection after giving a
200 but before giving any packets, we don't want to hang forever
waiting for a response that will never come.  Instead, we should die
immediately.

One case where this happens is when attempting to fetch a dangling
object by SHA.

Still to do: it would be good to give a better error message
than "fatal: The remote end hung up unexpectedly".

Signed-off-by: David Turner 
---

Note: if you run t5551 before applying the code patch, the second new
test will hang forever.

FWIW, I also saw this kind of hang at Twitter from time to time, but I
was never able to reliably reproduce it there, and thus never able to
fix it.  I suspect that a bad load balancer might have been killing
connections just after the headers, but this is pure speculation.

I am sorry that this patch does not provide a more useful error
message.  For us, it is important to fix the hangs (so that we can
retry on another server, for instance), but less important to be
user-friendly (since we were only seeing these with automated
processes).  I hope that someone who actually understands the http
code, and has some time, could help improve this aspect of the code.

If not, then at least we won't have inexplicable hangs.

 remote-curl.c   |  8 
 t/t5551-http-fetch-smart.sh | 30 ++
 2 files changed, 38 insertions(+)

diff --git a/remote-curl.c b/remote-curl.c
index f14c41f..ee44236 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -400,6 +400,7 @@ struct rpc_state {
size_t pos;
int in;
int out;
+   int any_written;
struct strbuf result;
unsigned gzip_request : 1;
unsigned initial_buffer : 1;
@@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,
 {
size_t size = eltsize * nmemb;
struct rpc_state *rpc = buffer_;
+   if (size)
+   rpc->any_written = 1;
write_or_die(rpc->in, ptr, size);
return size;
 }
@@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
 
+
+   rpc->any_written = 0;
err = run_slot(slot, NULL);
if (err == HTTP_REAUTH && !large_request) {
credential_fill(_auth);
@@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)
if (err != HTTP_OK)
err = -1;
 
+   if (!rpc->any_written)
+   err = -1;
+
curl_slist_free_all(headers);
free(gzip_body);
return err;
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 1ec5b27..43665ab 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be 
split across POSTs' '
test_line_count = 2 posts
 '
 
+test_expect_success 'test allowreachablesha1inwant' '
+   test_when_finished "rm -rf test_reachable.git" &&
+   server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+   master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
+   git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
+
+   git init --bare test_reachable.git &&
+   git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" 
&&
+   git -C test_reachable.git fetch origin "$master_sha"
+'
+
+test_expect_success 'test allowreachablesha1inwant with unreachable' '
+   test_when_finished "rm -rf test_reachable.git; git reset --hard $(git 
rev-parse HEAD)" &&
+
+   #create unreachable sha
+   echo content >file2 &&
+   git add file2 &&
+   git commit -m two &&
+   git push public HEAD:refs/heads/doomed &&
+   git push public :refs/heads/doomed &&
+
+   server="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
+   master_sha=$(git -C "$server" rev-parse refs/heads/master) &&
+   git -C "$server" config uploadpack.allowreachablesha1inwant 1 &&
+
+   git init --bare test_reachable.git &&
+   git -C test_reachable.git remote add origin "$HTTPD_URL/smart/repo.git" 
&&
+   test_must_fail git -C test_reachable.git fetch origin "$(git rev-parse 
HEAD)"
+'
+
 test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '
(
cd "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" &&
-- 
2.8.0.rc4.22.g8ae061a

Re: [git-for-windows] [ANNOUNCE] Prerelease: Git for Windows v2.11.0-rc0

[Lars Schneider]

On 05 Nov 2016, at 10:50, Johannes Schindelin  
wrote:

> Dear Git users,
> 
> I finally got around to rebase the Windows-specific patches (which seem to
> not make it upstream as fast as we get new ones) on top of upstream Git
> v2.11.0-rc0, and to bundle installers, portable Git and MinGit [*1*]:
> 
> https://github.com/git-for-windows/git/releases/tag/v2.11.0-rc0.windows.1


I tested a new feature in 2.11 on Windows today and it failed. After some 
confusion I realized that the feature is not on your 2.11 branch. Consider this:

My last patch on your 2.11 branch is 
"pack-protocol: fix maximum pkt-line size" (7841c4):
https://github.com/git-for-windows/git/commits/v2.11.0-rc0.windows.1?author=larsxschneider

My last patch on the Git 2.11 branch is 
"read-cache: make sure file handles are not inherited by child processes" 
(a0a6cb):
https://github.com/git/git/commits/v2.11.0-rc0?author=larsxschneider

Do you have a clue what is going on?

Thanks,
Lars

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Jeff King]

On Wed, Nov 09, 2016 at 12:41:22PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > For matching specific names, we have to deal with case-folding.  It's
> > easy to hit the common ones like ".GITIGNORE" with fspathcmp(). But if
> > this is actually protection against malicious repositories, we have to
> > match all of the horrible filesystem-specific junk that we did for
> > ".git".
> >
> > Symlinks are likewise tricky.
> 
> Wouldn't it be the simplest to say these:
> 
>  (1) The code attempts to read ".gitignore" (or ".git")
>  in general from the filesystem, or the index, or a tree. No
>  case permutations are attempted.
> 
>  (2) When the code tries to do the above, we open with nofollow (or
>  protect racily with lstat(2) which may be the best we could do)
>  when reading from the filesystem, or check the ce_mode type
>  when reading from the index or from a tree, and ignore if the
>  path we are using is a symbolic link.
> 
> That way, case funny filesystems that cause trouble like the ".git"
> thing would not have a chance to interfere and fool us, no?

That's what my series does. The question is just whether people will be
unhappy with (2), because they are using symbolic links in their
.gitignore files.

-Peff

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Junio C Hamano]

Jeff King  writes:

> For matching specific names, we have to deal with case-folding.  It's
> easy to hit the common ones like ".GITIGNORE" with fspathcmp(). But if
> this is actually protection against malicious repositories, we have to
> match all of the horrible filesystem-specific junk that we did for
> ".git".
>
> Symlinks are likewise tricky.

Wouldn't it be the simplest to say these:

 (1) The code attempts to read ".gitignore" (or ".git")
 in general from the filesystem, or the index, or a tree. No
 case permutations are attempted.

 (2) When the code tries to do the above, we open with nofollow (or
 protect racily with lstat(2) which may be the best we could do)
 when reading from the filesystem, or check the ce_mode type
 when reading from the index or from a tree, and ignore if the
 path we are using is a symbolic link.

That way, case funny filesystems that cause trouble like the ".git"
thing would not have a chance to interfere and fool us, no?

Re: [PATCH v2] rebase: add --forget to cleanup rebase, leave everything else untouched

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> ---
>  v2 changes just the subject line

That's not sufficient, is it?  What you did in the documentation
would raise the same "Hmph, is this only about HEAD?" and unlike the
commit subject, it will carve it in stone for end-users.

Please reply me

[Rebecca Taylor]

My dear friend,

I am Mrs Rebecca Taylor from Kuwait married to Late Mr. Alfred Taylor from 
Ivory Coast West Africa, of paroisse saint sebastien Church. I and My late 
husband has done numerous charity service within and outside our region since 
1981.

We have been doing our best to promote charity work anywhere the Lord laid us 
to. Since His death I have been having Lung Cancer and now my doctor has 
confirm to me that i would not last for the next 16 Weeks due to the cancer 
illness. My husband died as the result of a Political tussle in Cote d' Ivoire 
2011 that took many innocent lives between the former president Laurent Gbago 
and President Alassane Ouattara.

I have been distributing all i have since the Doctor gave me the notification 
and I also want to entrust the sum of Six Million Five hundred thousand U.S 
Dollars to your care so that you can use it for charity services in your area 
to the Glory of God and Service to humanity.

I will be waiting to hear from you and receive your assurance to be able to 
fulfill this charity work in your area in sincerity and good will.

Yours sincerely

sister
Rebecca Taylor

Cleaning ignored files

[Roman Terekhov]

Hi,

I want to ask about git clean -dXf command behaviour.

I do the following:

$ mkdir gitignore_test
$ cd gitignore_test/
$ git init
Initialized empty Git repository in ~/gitignore_test/.git/

$ echo *.sln > .gitignore
$ git add .gitignore
$ git commit -m "add gitignore"
[master (root-commit) ef78a3c] add gitignore
 1 file changed, 1 insertion(+)
 create mode 100644 .gitignore

$ mkdir src
$ touch test.sln
$ touch src/test.sln
$ tree
.
├── src
│   └── test.sln
└── test.sln

1 directory, 2 files

$ git clean -dXf
Removing test.sln

$ tree
.
└── src
└── test.sln

1 directory, 1 file


Why git clean -dXf does not remove all my test.sln files, but just one of them?

Roman Terekhov

Re: [PATCH 32/36] pathspec: allow querying for attributes

[Stefan Beller]

On Wed, Nov 9, 2016 at 1:45 AM, Duy Nguyen  wrote:
> On Fri, Oct 28, 2016 at 1:29 AM, Junio C Hamano  wrote:
>> The reason why I am bringing this up in this discussion thread on
>> this patch is because I wonder if we would benefit by a similar
>> "let's not do too involved things and be cheap by erring on the safe
>> and lazy side" strategy in the call to ce_path_match() call made in
>> this function to avoid making calls to the attr subsystem.
>>
>> In other words, would it help the system by either simplifying the
>> processing done or reducing the cycle spent in preload_thread() if
>> we could tell ce_path_match() "A pathspec we are checking may
>> require not just the pattern to match but also attributes given to
>> the path to satisfy the criteria, but for the purpose of preloading,
>> pretend that the attribute satisfies the match criteria" (or
>> "pretend that it does not match"), thereby not having to make any
>> call into the attribute subsystem at all from this codepath?
>>
>> The strategy this round takes to make it unnecessary to punt
>> preloading (i.e. dropping "pathspec: disable preload-index when
>> attribute pathspec magic is in use" patch the old series had) is to
>> make the attribute subsystem thread-safe.  But that thread-safety in
>> the initial round is based on a single Big Attribute Lock, so it may
>> turn out that the end result performs better for this codepath if we
>> did not to make any call into the attribute subsystem.
>
> It does sound good and I want to say "yes please do this", but is it
> making pathspec api a bit more complex (to express "assume all
> attr-related criteria match")? I guess we can have an api to simply
> filter out attr-related magic (basically set attr_match_nr back to
> zero) then pass a safe (but more relaxing) pathspec to the threaded
> code. That would not add big maintenance burden.
>

So with the current implementation, we already have the shortcut as:

 if (item->attr_match_nr && !match_attrs(name, namelen, item))

i.e. if attr_match_nr is zero, we do not even look at the mutexes and such,
so I am not sure what you intend to say in this email?

Re: [PATCH v5 01/16] Git.pm: add subroutines for commenting lines

[Vasco Almeida]

A Ter, 08-11-2016 às 17:06 -0800, Junio C Hamano escreveu:
> Vasco Almeida  writes:
> 
> > 
> > Add subroutines prefix_lines and comment_lines.
> > 
> > Signed-off-by: Vasco Almeida 
> > ---
> >  perl/Git.pm | 23 +++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/perl/Git.pm b/perl/Git.pm
> > index b2732822a..17be59fb7 100644
> > --- a/perl/Git.pm
> > +++ b/perl/Git.pm
> > @@ -1438,6 +1438,29 @@ sub END {
> >  
> >  } # %TEMP_* Lexical Context
> >  
> > +=item prefix_lines ( PREFIX, STRING )
> > +
> > +Prefixes lines in C with C.
> > +
> > +=cut
> > +
> > +sub prefix_lines {
> > +   my ($prefix, $string) = @_;
> > +   $string =~ s/^/$prefix/mg;
> > +   return $string;
> > +}
> > +
> > +=item comment_lines ( STRING )
> > +
> > +Comments lines following core.commentchar configuration.
> > +
> > +=cut
> > +
> > +sub comment_lines {
> > +   my $comment_line_char = config("core.commentchar") || '#';
> > +   return prefix_lines("$comment_line_char ", @_);
> > +}
> > +
> 
> This makes it appear as if comment_lines can take arbitrary number
> of strings as its arguments (because the outer caller just passes @_
> thru), but in fact because prefix_lines ignores anything other than
> $_[0] and $_[1], only the first parameter given to comment_lineS sub
> is inspected for lines in it and the prefix-char prefixed at the
> beginning of each of them.
> 
> Which is not a great interface, as it is quite misleading.
> 
> Perhaps
> 
>   prefix_lines("#", join("\n", @_));
> 
> or something like that may make it less confusing.

I prefer to have like this instead

sub prefix_lines {
my $prefix = shift;
my $string = join("\n", @_);
$string =~ s/^/$prefix/mg;
return $string;
}

So both subroutines can take several strings as arguments.

Re: 2.11.0-rc1 will not be tagged for a few days

[Jeff King]

On Tue, Nov 08, 2016 at 10:29:07PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I'm collecting v2.11-rc1 topics in the "refs/heads/for-junio/" section
> > of git://github.com/peff/git.git.
> >
> > I've also got proposed merges for "master" there, though note that none
> > of the topics has actually cooked at all in next (the fixes are trivial
> > enough that it may be OK, though).
> 
> I am not quite back up at full steam yet, but I expect I'd be more
> or less functioning tomorrow, so I'll fetch them from your tree and
> continue.

It looks like Johannes prefers replacements for some of the fixes from
yesterday. Just FYI that you probably do not want to take the "master" I
pushed there literally, but instead just view it as a proposed list of
topics.

I did not really expect you to take it literally in the first place.
It's just that I found myself writing up notes like "this should be
merged", and it occurred to me that I could communicate the same things
by sending you a proposed history. So I'm curious if you find dissecting
it via "git log" more or less convenient than a list of notes. :)

-Peff

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Jeff King]

On Wed, Nov 09, 2016 at 04:22:12PM +0700, Duy Nguyen wrote:

> > Symlinks are likewise tricky.  If we see that a symlink points to
> > "foo/../bar", then we don't know if it leaves the repository unless we
> > also look at "foo" to see if it is also a symlink. So you really end up
> > having to resolve the symlink yourself (and when checking out multiple
> > files, there's an ordering dependency).
> 
> We do have this dependency problem right now (e.g. files A and
> .gitattributes are checked out at the same time and .gitattributes has
> some attribute on A). It looks like we resolve it by reading the index
> version at checkout time. We probably can do the same for gitattribute
> symlinks.

Right, but then we can't use filesystem functions like realpath() to do
the lookup. I guess we could do a pass after the checkout is done to
"fix" any out-of-tree symlinks we just created.

This is exactly the sort of complexity I was trying to avoid with my
original series. :)

If that isn't an option, I think I prefer something like the
core.saneSymlinks approach I mentioned. It has the additional bonus of
protecting not just git commands, but other commands that might inspect
the filesystem.

> > So one reasonable fix might be to have a config option like
> > "core.saneSymlinks" that enforces both of those rules for _all_ symlinks
> > that we checkout to the working tree. And it could either refuse to
> > check them out, or replace them with a file containing the symlink
> > content (as we do on systems that don't support symlinks, IIRC).
> 
> I wonder if anyone want core.saneSymlinks on, but they have some links
> that do not meet the above checks and still want to follow them
> anyway. One way to add such an exception is mark the path with an
> attribute "follow". Yeah I have a dependency loop :(

That could come later if somebody wants it, I think (especially if the
config option is not on by default). I have a feeling that callers will
either care about out-of-tree symlinks or not. Trusting the repository
to say "but these ones are OK" doesn't work for the paranoid ones, and
everybody else just assumes the repository is sane.

-Peff

Re: [PATCH] sequencer: shut up clang warning

[Jeff King]

On Wed, Nov 09, 2016 at 02:56:25PM +0100, Johannes Schindelin wrote:

> When comparing a value of type `enum todo_command` with a value that is
> outside the defined enum constants, clang greets the developer with this
> warning:
> 
>   comparison of constant 2 with expression of type
>   'const enum todo_command' is always true
> 
> While this is arguably true *iff* the value was never cast from a
> free-form int, we should keep the cautious code in place.
> 
> To shut up clang, we simply introduce an otherwise pointless enum constant
> and compare against that.

This does silence the warning.

I slightly prefer mine because:

  1. It does not carry an implicit requirement that TODO_INVALID remain
 the final enum value.

  2. It also protects the range check against a negative enum value.

But this is code that is getting changed later by you anyway, and I do
not care that strongly.

-Peff

Re: [PATCH] t6026-merge-attr: don't fail if sleep exits early

[Jeff King]

On Wed, Nov 09, 2016 at 03:36:40PM +0100, Andreas Schwab wrote:

> On Nov 09 2016, Johannes Schindelin  wrote:
> 
> > The reason why we do not ignore kill errors is that we want to make sure
> > that the script *actually ran*. Otherwise, the thing we need to test here
> > does not necessarily get tested.
> 
> That can be tested by looking for the pid file.

I agree that makes the intent a lot more obvious. Having a necessary
condition of the test stuffed into a test_when_finished block seems
counter-intuitive.

-Peff

Re: [PATCH v1 03/19] split-index: add {add,remove}_split_index() functions

[Christian Couder]

On Wed, Nov 9, 2016 at 10:24 AM, Duy Nguyen  wrote:
> On Mon, Nov 7, 2016 at 5:08 PM, Duy Nguyen  wrote:
>> On Sun, Oct 30, 2016 at 5:06 AM, Christian Couder
>>  wrote:
>>> On Tue, Oct 25, 2016 at 11:58 AM, Duy Nguyen  wrote:
 On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
  wrote:
> +void remove_split_index(struct index_state *istate)
> +{
> +   if (istate->split_index) {
> +   /*
> +* can't discard_split_index(_index); because that
> +* will destroy split_index->base->cache[], which may
> +* be shared with the_index.cache[]. So yeah we're
> +* leaking a bit here.

 In the context of update-index, this is a one-time thing and leaking
 is tolerable. But because it becomes a library function now, this leak
 can become more serious, I think.

 The only other (indirect) caller is read_index_from() so probably not
 bad most of the time (we read at the beginning of a command only).
 sequencer.c may discard and re-read the index many times though,
 leaking could be visible there.
>>>
>>> So is it enough to check if split_index->base->cache[] is shared with
>>> the_index.cache[] and then decide if discard_split_index(_index)
>>> should be called?
>>
>> It's likely shared though. We could un-share cache[] by duplicating
>> index entries in the_index.cache[] if they point back to
>> split_index->base
>
> I have a patch for this. So don't have to do anything else in this
> area. I'll probably just pile my patch on top of your series, or post
> it once the series graduates to master.

Great, thanks!

Re: [PATCH] t6026-merge-attr: don't fail if sleep exits early

[Andreas Schwab]

On Nov 09 2016, Johannes Schindelin  wrote:

> The reason why we do not ignore kill errors is that we want to make sure
> that the script *actually ran*. Otherwise, the thing we need to test here
> does not necessarily get tested.

That can be tested by looking for the pid file.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[PATCH] sequencer: shut up clang warning

[Johannes Schindelin]

When comparing a value of type `enum todo_command` with a value that is
outside the defined enum constants, clang greets the developer with this
warning:

comparison of constant 2 with expression of type
'const enum todo_command' is always true

While this is arguably true *iff* the value was never cast from a
free-form int, we should keep the cautious code in place.

To shut up clang, we simply introduce an otherwise pointless enum constant
and compare against that.

Noticed by Torsten Bögershausen.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/enum-warning-v1
Fetch-It-Via: git fetch https://github.com/dscho/git enum-warning-v1

Peff, if you would like me to include more of your commit message,
please let me know.

I will adjust the sequencer-i patches to keep the TODO_INVALID
constant.

 sequencer.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5fd75f3..f80e9c0 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -619,7 +619,8 @@ static int allow_empty(struct replay_opts *opts, struct 
commit *commit)
 
 enum todo_command {
TODO_PICK = 0,
-   TODO_REVERT
+   TODO_REVERT,
+   TODO_INVALID
 };
 
 static const char *todo_command_strings[] = {
@@ -629,7 +630,7 @@ static const char *todo_command_strings[] = {
 
 static const char *command_to_string(const enum todo_command command)
 {
-   if (command < ARRAY_SIZE(todo_command_strings))
+   if (command < TODO_INVALID)
return todo_command_strings[command];
die("Unknown command: %d", command);
 }

base-commit: be5a750939c212bc0781ffa04fabcfd2b2bd744e
-- 
2.10.1.583.g721a9e0

Re: [PATCH] sequencer: silence -Wtautological-constant-out-of-range-compare

[Johannes Schindelin]

Hi Peff,

On Tue, 8 Nov 2016, Jeff King wrote:

> diff --git a/sequencer.c b/sequencer.c
> index 5fd75f30d..6f0ff9e41 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -629,7 +629,7 @@ static const char *todo_command_strings[] = {
>  
>  static const char *command_to_string(const enum todo_command command)
>  {
> - if (command < ARRAY_SIZE(todo_command_strings))
> + if ((size_t)command < ARRAY_SIZE(todo_command_strings))
>   return todo_command_strings[command];
>   die("Unknown command: %d", command);

I have come to prefer a slightly different approach. Will send it out in a
moment.

Ciao,
Dscho

Re: Bug: git config does not respect read-only .gitconfig file

[Jonathan Word]

> It is unreasonable to drop the write-enable bit of
> a file in a writable directory and expect it to stay unmodified. The
> W-bit on the file is not usable as a security measure, and we do not
> use it as such.

The point here is not a matter of security - it is of expectations.

When a user drops write access on the global ~/.gitconfig I think
a reasonable user would expect future `git config --global` calls to
fail by default. The possibility of an override is a different matter,
and my initial proposal included the details of enabling direct
override. I don't think there is any presumption that this is a
security related discussion.

> I do not offhand know how much a new feature "this repository can be
> modified by pushing into and fetching from, but its configuration
> cannot be modified" is a sensible thing to have.

I agree that per-repository files almost never run into an issue with
this. Our problem is strictly with the global ~/.gitconfig which in our
use case is owned by a shared system account and used implicitly
by many developers. Thus any one of those devs can call
`git config` without any signal that they are changing something
that ought not to be changed and should think carefully.

This would be equivalent to dropping write access to a file that
your account owns so that vi / emacs / etc.. will warn that the
file is read-only before modifying it (useful for any number of
sensitive files). Obviously from a security perspective you have
a number of means of potential override, however all require additional
steps that surface the initial intention that the file should not
change - or should only change rarely after additional confirmation.

> perhaps the lock_file()
> function can have access(path, W_OK) check before it returns a
> tempfile that has been successfully opened?

That sounds ideal

On Tue, Nov 8, 2016 at 8:22 PM, Junio C Hamano  wrote:
> Jeff King  writes:
>
>> Probably converting "rename(from, to)" to first check "access(to,
>> W_OK)". That's racy, but it's the best we could do.
>
> Hmph, if these (possibly problematic) callers are all following the
> usual "lock, write to temp, rename" pattern, perhaps the lock_file()
> function can have access(path, W_OK) check before it returns a
> tempfile that has been successfully opened?
>
> Having said that, I share your assessment that this is not a code or
> design problem.  It is unreasonable to drop the write-enable bit of
> a file in a writable directory and expect it to stay unmodified. The
> W-bit on the file is not usable as a security measure, and we do not
> use it as such.
>
> I do not offhand know how much a new feature "this repository can be
> modified by pushing into and fetching from, but its configuration
> cannot be modified" is a sensible thing to have.  But it is quite
> clear that even if we were to implement such feature, we wouldn't be
> using W-bit on .git/config to signal that.
>

[PATCH] t6026: ensure that long-running script really is

[Johannes Schindelin]

When making sure that background tasks are cleaned up in 5babb5b
(t6026-merge-attr: clean up background process at end of test case,
2016-09-07), we considered to let the background task sleep longer, just
to be certain that it will still be running when we want to kill it
after the test.

Sadly, the assumption appears not to hold true that the test case passes
quickly enough to kill the background task within a second.

Simply increase it to an hour. No system can be possibly slow enough to
make above-mentioned assumption incorrect.

Reported by Andreas Schwab.

Signed-off-by: Johannes Schindelin 
---
Published-As: https://github.com/dscho/git/releases/tag/t6026-sleep-v1
Fetch-It-Via: git fetch https://github.com/dscho/git t6026-sleep-v1

 t/t6026-merge-attr.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 7a6e33e..348d78b 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -183,16 +183,16 @@ test_expect_success 'up-to-date merge without common 
ancestor' '
 
 test_expect_success 'custom merge does not lock index' '
git reset --hard anchor &&
-   write_script sleep-one-second.sh <<-\EOF &&
-   sleep 1 &
+   write_script sleep-an-hour.sh <<-\EOF &&
+   sleep 3600 &
echo $! >sleep.pid
EOF
test_when_finished "kill \$(cat sleep.pid)" &&
 
test_write_lines >.gitattributes \
-   "* merge=ours" "text merge=sleep-one-second" &&
+   "* merge=ours" "text merge=sleep-an-hour" &&
test_config merge.ours.driver true &&
-   test_config merge.sleep-one-second.driver ./sleep-one-second.sh &&
+   test_config merge.sleep-an-hour.driver ./sleep-an-hour.sh &&
git merge master
 '
 

base-commit: be5a750939c212bc0781ffa04fabcfd2b2bd744e
-- 
2.10.1.583.g721a9e0

Re: [PATCH] t6026-merge-attr: don't fail if sleep exits early

[Johannes Schindelin]

Hi 

On Tue, 8 Nov 2016, Jeff King wrote:

> On Tue, Nov 08, 2016 at 06:03:04PM +0100, Andreas Schwab wrote:
> 
> > Commit 5babb5bdb3 ("t6026-merge-attr: clean up background process at end
> > of test case") added a kill command to clean up after the test, but this
> > can fail if the sleep command exits before the cleanup is executed.
> > Ignore the error from the kill command.
> > 
> > Signed-off-by: Andreas Schwab 
> > ---
> > The failure can be simulated by adding a sleep after the last command to
> > delay the cleanup.
> 
> Thanks for the reproduction hint. I sometimes run the test suite through
> a "stress" script that sees if a test script racily fails under load,
> but I wasn't able to trigger it here (I guess a full second is just too
> long even under high load). But the extra sleep makes it obvious.
> 
> Looks like the original is in v2.10.1, so this should probably go to
> maint, as well as the upcoming v2.11-rc1.

The reason why we do not ignore kill errors is that we want to make sure
that the script *actually ran*. Otherwise, the thing we need to test here
does not necessarily get tested.

So I would rather go with the patch Hannes hinted at when he said that he
did not want to change the file name (as it would have made the diff less
obvious): increase the number of seconds.

Will send out a superseding patch in a minute,
Dscho

Re: [PATCH 0/3] gitk: memory consumption improvements

[Markus Hitter]

Am 08.11.2016 um 22:37 schrieb Junio C Hamano:
> Are all semi-modern Tcl/Tk in service have this -undo thing so that
> we can pass unconditionally to the text widget like the patch does?

Good point. As far as my research goes, this flag was introduced in Nov. 2001:

http://core.tcl.tk/tk/info/5265df93d207cec0


To defend Gitk developers, the Tk guys apparently change their mind on the 
default value from time to time. Official documentation says nothing about a 
default, the proposal from 2001 talks about -undo 0 as default and there are 
recent commits changing this default:

http://core.tcl.tk/tk/info/549d2f56757408f3


Markus

-- 
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/

Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate

[Ian Jackson]

Junio C Hamano writes ("Re: [PATCH 5/6] config docs: Provide for config to 
specify tags not to abbreviate"):
> And I do not think we would want "log" or any core side Porcelain
> command to have too many "information losing" options like this
> "truncate refnames down to a point where it is no longer unique and
> meaningful".  GUI tools can get away with doing sos because they can
> arrange these truncated labels to react to end-user input (e.g. the
> truncated Tag in the history display of gitk could be made to react
> to mouse-over and pop-up to show a full name, for example), but the
> output from the core side is pretty much fixed once it is emitted.
> 
> So my first preference would be to teach gitk such a "please
> clarify" UI-reaction, if it does not know how to do so yet.  There
> is no need for a configuration variable anywhere with this approach.

gitk already has a way for the user to find out what the elided tag
names are.  The underlying difficulty is that the situation that the
gitk behaviour is designed for (long tag names, perhaps several to a
commit, not particularly interesting), is not applicable to these
particular tags.

Whether the tag is `particularly interesting' depends, as I say, on
both what tree it is in, and on its name.  It might be appropriate for
terminal-based tools to highlight these tags too, or show them when
tags are not normally displayed.

`core.interestingTags' ?

> If you do want to add a configuration to show fuller name in the
> tag, which would make it unnecessary for the user to do "please
> clarify, as I am hovering over what I want to get details of"
> action, that may also be a good way to go.

I think in my use case, which I hope to become common within Debian,
this is going to be essential.

>  But I think the right
> place to do so would be Edit -> Preferences menu in Gitk, and the
> settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
> gitk-specific place.

This is not correct, because as I have explained, this should be a
per-tree configuration:

If it can't be a `git config' option, even `git config gui.something',
then I guess I will have to teach gitk to read a config file in
GIT_DIR too.  But I think that is silly given that git already has a
config file reading system which handles per-tree configs.

If we can't get agreement from the git-core developers on a config to
be used, and documented, for any tool which has similar behaviour, I
think the right answer is `git config gitk.', which would
be documented in gitk.

Thanks,
Ian.

-- 
Ian Jackson    These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

Re: [PATCH v1 1/2] config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11

[Torsten Bögershausen]

On 09.11.16 10:29, Lars Schneider wrote:
> 
>> On 09 Nov 2016, at 09:18, Torsten Bögershausen  wrote:
>>
>> On 07.11.16 18:26, Jeff King wrote:
>>> On Sun, Nov 06, 2016 at 08:35:04PM +0100, Lars Schneider wrote:
>>>
 Good point. I think I found an even easier way to achieve the same.
 What do you think about the patch below?

 [...]

 diff --git a/Makefile b/Makefile
 index 9d6c245..f53fcc9 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1047,6 +1047,7 @@ ifeq ($(uname_S),Darwin)
endif
endif
ifndef NO_APPLE_COMMON_CRYPTO
 +  NO_OPENSSL = YesPlease
APPLE_COMMON_CRYPTO = YesPlease
COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
endif
>>>
>>> That is much simpler.
>> []
>> I don't know if that is a correct solution.
>>
>> If I have Mac OS 10.12 and Mac Ports installed, I may want to use
>> OPENSSL from Mac Ports.
> 
> Can't you define `NO_APPLE_COMMON_CRYPTO` in that case? 
> I think if you use OpenSSL then you don't need the Apple crypto lib, right?

After re-reading the Makefile: that makes sense :-)

Do you want to send a new patch ?

Feel free to omit
"Original-patch-by: Torsten Bögershausen "

Re: [PATCHv2 32/36] pathspec: allow querying for attributes

[Duy Nguyen]

On Sat, Oct 29, 2016 at 1:54 AM, Stefan Beller  wrote:
> The pathspec mechanism is extended via the new
> ":(attr:eol=input)pattern/to/match" syntax to filter paths so that it
> requires paths to not just match the given pattern but also have the
> specified attrs attached for them to be chosen.
>
> Signed-off-by: Stefan Beller 
> Signed-off-by: Junio C Hamano 
> ---
>  Documentation/glossary-content.txt |  20 +
>  dir.c  |  35 

Pathspec can be processed in a couple more places. The big two are
match_pathspec and tree_entry_interesting, the former traverses a list
while the latter does a tree. You don't have to implement attr
matching in tree_entry_interesting right now because nobody needs it,
probably. But you need to make sure if somebody accidentally calls
tree_entry_interesting with an attr pathspec, then it should
die("BUG"), not silently ignore attr.

The way to do that is GUARD_PATHSPEC macro which makes sure if only
recognized magic is allowed through. This macro guards all pathspec
processing functions. So you can add a new PATHSPEC_ATTR macro (or
some other name) to the "Pathspec magic" group near the beginning of
pathspec.h, set it whenever attr magic is present when you
parse_pathspec(), then lift the GUARD_PATHSPEC restriction in
match_pathspec() only because this function can handle it. Whenever
attr magic is used by any other functions, it will die() the way we
want.
-- 
Duy

Re: [PATCH 32/36] pathspec: allow querying for attributes

[Duy Nguyen]

On Fri, Oct 28, 2016 at 1:29 AM, Junio C Hamano  wrote:
> The reason why I am bringing this up in this discussion thread on
> this patch is because I wonder if we would benefit by a similar
> "let's not do too involved things and be cheap by erring on the safe
> and lazy side" strategy in the call to ce_path_match() call made in
> this function to avoid making calls to the attr subsystem.
>
> In other words, would it help the system by either simplifying the
> processing done or reducing the cycle spent in preload_thread() if
> we could tell ce_path_match() "A pathspec we are checking may
> require not just the pattern to match but also attributes given to
> the path to satisfy the criteria, but for the purpose of preloading,
> pretend that the attribute satisfies the match criteria" (or
> "pretend that it does not match"), thereby not having to make any
> call into the attribute subsystem at all from this codepath?
>
> The strategy this round takes to make it unnecessary to punt
> preloading (i.e. dropping "pathspec: disable preload-index when
> attribute pathspec magic is in use" patch the old series had) is to
> make the attribute subsystem thread-safe.  But that thread-safety in
> the initial round is based on a single Big Attribute Lock, so it may
> turn out that the end result performs better for this codepath if we
> did not to make any call into the attribute subsystem.

It does sound good and I want to say "yes please do this", but is it
making pathspec api a bit more complex (to express "assume all
attr-related criteria match")? I guess we can have an api to simply
filter out attr-related magic (basically set attr_match_nr back to
zero) then pass a safe (but more relaxing) pathspec to the threaded
code. That would not add big maintenance burden.

> I am probably being two step ahead of ourselves by saying the above,
> which is just something to keep in mind as a possible solution if
> performance in this preload codepath becomes an issue when the
> pathspec has attributes match specified.
-- 
Duy

Re: [PATCH v1 1/2] config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11

[Lars Schneider]

> On 09 Nov 2016, at 09:18, Torsten Bögershausen  wrote:
> 
> On 07.11.16 18:26, Jeff King wrote:
>> On Sun, Nov 06, 2016 at 08:35:04PM +0100, Lars Schneider wrote:
>> 
>>> Good point. I think I found an even easier way to achieve the same.
>>> What do you think about the patch below?
>>> 
>>> [...]
>>> 
>>> diff --git a/Makefile b/Makefile
>>> index 9d6c245..f53fcc9 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1047,6 +1047,7 @@ ifeq ($(uname_S),Darwin)
>>> endif
>>> endif
>>> ifndef NO_APPLE_COMMON_CRYPTO
>>> +   NO_OPENSSL = YesPlease
>>> APPLE_COMMON_CRYPTO = YesPlease
>>> COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
>>> endif
>> 
>> That is much simpler.
> []
> I don't know if that is a correct solution.
> 
> If I have Mac OS 10.12 and Mac Ports installed, I may want to use
> OPENSSL from Mac Ports.

Can't you define `NO_APPLE_COMMON_CRYPTO` in that case? 
I think if you use OpenSSL then you don't need the Apple crypto lib, right?

- Lars


> How about this:
> 
> 
> diff --git a/Makefile b/Makefile
> index ee89c06..e93511f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1038,17 +1038,22 @@ ifeq ($(uname_S),Darwin)
>ifeq ($(shell test -d /sw/lib && echo y),y)
>BASIC_CFLAGS += -I/sw/include
>BASIC_LDFLAGS += -L/sw/lib
> +   HAS_OPENSSL = Yes
>endif
>endif
>ifndef NO_DARWIN_PORTS
>ifeq ($(shell test -d /opt/local/lib && echo y),y)
>BASIC_CFLAGS += -I/opt/local/include
>BASIC_LDFLAGS += -L/opt/local/lib
> +   HAS_OPENSSL = Yes
>endif
>endif
>ifndef NO_APPLE_COMMON_CRYPTO
>APPLE_COMMON_CRYPTO = YesPlease
>COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
> +   ifndef HAS_OPENSSL
> +   NO_OPENSSL = YesPlease
> +   endif
>endif
>NO_REGEX = YesPlease
>PTHREAD_LIBS =
> 

Re: [PATCH v1 03/19] split-index: add {add,remove}_split_index() functions

[Duy Nguyen]

On Mon, Nov 7, 2016 at 5:08 PM, Duy Nguyen  wrote:
> On Sun, Oct 30, 2016 at 5:06 AM, Christian Couder
>  wrote:
>> On Tue, Oct 25, 2016 at 11:58 AM, Duy Nguyen  wrote:
>>> On Sun, Oct 23, 2016 at 4:26 PM, Christian Couder
>>>  wrote:
 +void remove_split_index(struct index_state *istate)
 +{
 +   if (istate->split_index) {
 +   /*
 +* can't discard_split_index(_index); because that
 +* will destroy split_index->base->cache[], which may
 +* be shared with the_index.cache[]. So yeah we're
 +* leaking a bit here.
>>>
>>> In the context of update-index, this is a one-time thing and leaking
>>> is tolerable. But because it becomes a library function now, this leak
>>> can become more serious, I think.
>>>
>>> The only other (indirect) caller is read_index_from() so probably not
>>> bad most of the time (we read at the beginning of a command only).
>>> sequencer.c may discard and re-read the index many times though,
>>> leaking could be visible there.
>>
>> So is it enough to check if split_index->base->cache[] is shared with
>> the_index.cache[] and then decide if discard_split_index(_index)
>> should be called?
>
> It's likely shared though. We could un-share cache[] by duplicating
> index entries in the_index.cache[] if they point back to
> split_index->base

I have a patch for this. So don't have to do anything else in this
area. I'll probably just pile my patch on top of your series, or post
it once the series graduates to master.
-- 
Duy

Re: [PATCH 4/5] attr: do not respect symlinks for in-tree .gitattributes

[Duy Nguyen]

On Wed, Nov 9, 2016 at 5:21 AM, Jeff King  wrote:
> On Tue, Nov 08, 2016 at 08:38:55AM +0700, Duy Nguyen wrote:
>
>> > Another approach is to have a config option to disallow symlinks to
>> > destinations outside of the repository tree (I'm not sure if it should
>> > be on or off by default, though).
>>
>> Let's err on the safe side and disable symlinks to outside repo by
>> default (or even all symlinks on .gitattributes and .gitignore as the
>> first step)
>
> Both of those are actually much harder than you might think.
>
> For matching specific names, we have to deal with case-folding.  It's
> easy to hit the common ones like ".GITIGNORE" with fspathcmp(). But if
> this is actually protection against malicious repositories, we have to
> match all of the horrible filesystem-specific junk that we did for
> ".git".

We could realpath() it and check if the result path is inside
realpath($GIT_WORK_TREE). The real work would be done by OS. We will
need to check if it points to .git/something, but I think we have that
covered. The approach is a bit heavy for such a sanity check though

> Symlinks are likewise tricky.  If we see that a symlink points to
> "foo/../bar", then we don't know if it leaves the repository unless we
> also look at "foo" to see if it is also a symlink. So you really end up
> having to resolve the symlink yourself (and when checking out multiple
> files, there's an ordering dependency).

We do have this dependency problem right now (e.g. files A and
.gitattributes are checked out at the same time and .gitattributes has
some attribute on A). It looks like we resolve it by reading the index
version at checkout time. We probably can do the same for gitattribute
symlinks.

> I think it might be enough to check:
>
>   - leading "../" tokens in the symlink's destination can be checked
> against the symlink's path. So "../foo" is OK for path "one/two",
> but not for path "one".
>
>   - interior "../" can be disallowed entirely. Technically
> "foo/../bar/../baz" _can_ be a fine symlink destination, but why?
> It's identical to "baz" unless you are following a bunch of interior
> symlinks. And if those are interior symlinks, it's still confusing
> and unnecessarily obfuscated, and a good sign that somebody is
> trying to do something tricky.

Sounds good.

> So one reasonable fix might be to have a config option like
> "core.saneSymlinks" that enforces both of those rules for _all_ symlinks
> that we checkout to the working tree. And it could either refuse to
> check them out, or replace them with a file containing the symlink
> content (as we do on systems that don't support symlinks, IIRC).

I wonder if anyone want core.saneSymlinks on, but they have some links
that do not meet the above checks and still want to follow them
anyway. One way to add such an exception is mark the path with an
attribute "follow". Yeah I have a dependency loop :(
-- 
Duy

[PATCH v2] rebase: add --forget to cleanup rebase, leave everything else untouched

[Nguyễn Thái Ngọc Duy]

There are occasions when you decide to abort an in-progress rebase and
move on to do something else but you forget to do "git rebase --abort"
first. Or the rebase has been in progress for so long you forgot about
it. By the time you realize that (e.g. by starting another rebase)
it's already too late to retrace your steps. The solution is normally

rm -r .git/

and continue with your life. But there could be two different
directories for  (and it obviously requires some
knowledge of how rebase works), and the ".git" part could be much
longer if you are not at top-dir, or in a linked worktree. And
"rm -r" is very dangerous to do in .git, a mistake in there could
destroy object database or other important data.

Provide "git rebase --forget" for this exact use case.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 v2 changes just the subject line

 Documentation/git-rebase.txt   |  5 -
 contrib/completion/git-completion.bash |  4 ++--
 git-rebase.sh  |  6 +-
 t/t3407-rebase-abort.sh| 24 
 4 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index de222c8..5a58fb3 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[ []]
 'git rebase' [-i | --interactive] [options] [--exec ] [--onto ]
--root []
-'git rebase' --continue | --skip | --abort | --edit-todo
+'git rebase' --continue | --skip | --abort | --forget | --edit-todo
 
 DESCRIPTION
 ---
@@ -252,6 +252,9 @@ leave out at most one of A and B, in which case it defaults 
to HEAD.
will be reset to where it was when the rebase operation was
started.
 
+--forget::
+   Abort the rebase operation but leave HEAD where it is.
+
 --keep-empty::
Keep the commits that do not change anything from its
parents in the result.
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 21016bf..3143cb0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1734,10 +1734,10 @@ _git_rebase ()
 {
local dir="$(__gitdir)"
if [ -f "$dir"/rebase-merge/interactive ]; then
-   __gitcomp "--continue --skip --abort --edit-todo"
+   __gitcomp "--continue --skip --abort --forget --edit-todo"
return
elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
-   __gitcomp "--continue --skip --abort"
+   __gitcomp "--continue --skip --abort --forget"
return
fi
__git_complete_strategy && return
diff --git a/git-rebase.sh b/git-rebase.sh
index 04f6e44..de712b7 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -43,6 +43,7 @@ continue!  continue
 abort! abort and check out the original branch
 skip!  skip current patch and continue
 edit-todo! edit the todo list during an interactive rebase
+forget!abort but keep HEAD where it is
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -241,7 +242,7 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
-   --continue|--skip|--abort|--edit-todo)
+   --continue|--skip|--abort|--forget|--edit-todo)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -399,6 +400,9 @@ abort)
finish_rebase
exit
;;
+forget)
+   exec rm -rf "$state_dir"
+   ;;
 edit-todo)
run_specific_rebase
;;
diff --git a/t/t3407-rebase-abort.sh b/t/t3407-rebase-abort.sh
index a6a6c40..6bc5e71 100755
--- a/t/t3407-rebase-abort.sh
+++ b/t/t3407-rebase-abort.sh
@@ -99,4 +99,28 @@ testrebase() {
 testrebase "" .git/rebase-apply
 testrebase " --merge" .git/rebase-merge
 
+test_expect_success 'rebase --forget' '
+   cd "$work_dir" &&
+   # Clean up the state from the previous one
+   git reset --hard pre-rebase &&
+   test_must_fail git rebase master &&
+   test_path_is_dir .git/rebase-apply &&
+   head_before=$(git rev-parse HEAD) &&
+   git rebase --forget &&
+   test $(git rev-parse HEAD) = $head_before &&
+   test ! -d .git/rebase-apply
+'
+
+test_expect_success 'rebase --merge --forget' '
+   cd "$work_dir" &&
+   # Clean up the state from the previous one
+   git reset --hard pre-rebase &&
+   test_must_fail git rebase --merge master &&
+   test_path_is_dir .git/rebase-merge &&
+   head_before=$(git rev-parse HEAD) &&
+   git rebase --forget &&
+   test $(git rev-parse HEAD) = $head_before &&
+   test ! -d .git/rebase-merge
+'
+
 test_done
-- 
2.8.2.524.g6ff3d78

Re: Forbid access to /gitweb but authorize the sub projets

[Dennis Kaarsemaker]

On Mon, 2016-11-07 at 14:07 +0100, Alexandre Duplaix wrote:
> Hello,
> 
> I have several projects under https://myserver/gitweb and I would like 
> to forbid the access to the root, so that the users can't list the 
> differents projects.
> 
> However, I need to let the access to the sub projects (ex: 
> https://myserver/gitweb/?p=project1;a=summary
> 
> How can I do please ?

My favourite way of doing this is abusing the fact that gitweb.conf is
perl code that's loaded with do $filename.

This makes it easy to override such things. Try this in gitweb.conf for example:

sub no_index {
die_error(403, "No access to the repository list");
}
$actions{project_list} = \_index;
$actions{project_index} = \_index;
$actions{opml} = \_index;

-- 
Dennis Kaarsemaker
http://www.kaarsemaker.net

Re: [PATCH v1 1/2] config.mak.in: set NO_OPENSSL and APPLE_COMMON_CRYPTO for macOS >10.11

[Torsten Bögershausen]

On 07.11.16 18:26, Jeff King wrote:
> On Sun, Nov 06, 2016 at 08:35:04PM +0100, Lars Schneider wrote:
> 
>> Good point. I think I found an even easier way to achieve the same.
>> What do you think about the patch below?
>>
>> [...]
>>
>> diff --git a/Makefile b/Makefile
>> index 9d6c245..f53fcc9 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1047,6 +1047,7 @@ ifeq ($(uname_S),Darwin)
>>  endif
>>  endif
>>  ifndef NO_APPLE_COMMON_CRYPTO
>> +NO_OPENSSL = YesPlease
>>  APPLE_COMMON_CRYPTO = YesPlease
>>  COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
>>  endif
> 
> That is much simpler.
[]
I don't know if that is a correct solution.

If I have Mac OS 10.12 and Mac Ports installed, I may want to use
OPENSSL from Mac Ports.
How about this:


diff --git a/Makefile b/Makefile
index ee89c06..e93511f 100644
--- a/Makefile
+++ b/Makefile
@@ -1038,17 +1038,22 @@ ifeq ($(uname_S),Darwin)
ifeq ($(shell test -d /sw/lib && echo y),y)
BASIC_CFLAGS += -I/sw/include
BASIC_LDFLAGS += -L/sw/lib
+   HAS_OPENSSL = Yes
endif
endif
ifndef NO_DARWIN_PORTS
ifeq ($(shell test -d /opt/local/lib && echo y),y)
BASIC_CFLAGS += -I/opt/local/include
BASIC_LDFLAGS += -L/opt/local/lib
+   HAS_OPENSSL = Yes
endif
endif
ifndef NO_APPLE_COMMON_CRYPTO
APPLE_COMMON_CRYPTO = YesPlease
COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
+   ifndef HAS_OPENSSL
+   NO_OPENSSL = YesPlease
+   endif
endif
NO_REGEX = YesPlease
PTHREAD_LIBS =