Re: [PATCH] commit: ensure correct permissions of the commit message

[Johannes Schindelin]

Hi Peff,

On Wed, 6 Jan 2016, Jeff King wrote:

> I think fopen_for_writing() is fine, or fopen_truncate().

I like fopen_for_writing() better than fopen_truncate() because it states
the intention more clearly.

Thanks!
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Segmentation fault found while fuzzing .pack file under 2.7.0.rc3

[Jacek Wielemborek]

W dniu 06.01.2016 o 01:23, Jonathan Nieder pisze:
> Jeff King wrote:
> 
>> Git packfiles come from two places:
>>
>>   1. Local maintenance repacks loose and already-packed objects into a
>>  new packfile. We trust the local repack process to generate a valid
>>  packfile (though the contents of individual objects may be
>>  untrusted, of course).
> 
> I think we should reconsider such trust.  If one user creates a
> malicious pack, if another user uses read-only git commands to access
> the repository (after inspecting .git/config to make sure it doesn't
> contain anything scary) the result should not be arbitrary code
> execution.
> 
> Producing bogus output or aborting is okay; arbitrary code execution
> less so.
> 
> Thanks,
> Jonathan

I'd be happy to help you go through the fuzzing process - I don't have
enough horsepower and codebase knowledge to do it on my own though. If
you have an afl-fuzz question though, let me know.



signature.asc
Description: OpenPGP digital signature

Re: Git 2.7.0 gitignore behaviour regression

[Duy Nguyen]

On Tue, Jan 5, 2016 at 10:06 PM, Jeff King  wrote:
> On Tue, Jan 05, 2016 at 02:40:16PM +, Mike McQuaid wrote:
>
>> Homebrew has a series of convoluted .gitignore rules due to our
>> special/weird use-case of wanting to ignore everything in a working
>> directory except a select few files/directories. We experienced a bug
>> with our .gitignore file for users using Git 2.7.0. This may well be a
>> valid WONTFIX or intentional behaviour change but I wanted to flag it
>> in case it wasn’t.
>>
>> Here’s a minimal test case:
>>
>> - Create an empty git repository in a directory with `git init`
>> - Create a directory named ‘a' containing a file named ‘b' with `mkdir a && 
>> touch a/b`
>> - Create a ‘gitignore’ file with the following contents:
>> ```
>> */
>> /a
>> !/a/*
>> ```
>> - Run `git status --short`.
>>
>> The output with Git 2.6.4 is:
>> ```
>> ?? .gitignore
>> ```
>>
>> The output with Git 2.7.0 is:
>> ```
>> ?? .gitignore
>> ?? a/
>> ```
>
> Thanks for giving a clear example. This bisects to Duy's 57534ee (dir.c:
> don't exclude whole dir prematurely if neg pattern may match,
> 2015-09-21). AFAICT (and I don't recall looking over this patch
> previously), what you are seeing is the intended effect of the patch.

Yeah.. I think it's the only relevant commit in 2.7.0 cycle anyway.
These patterns "/a" followed by "!/a/*" were wrecking my head. But I
finally decided 2.7 output makes more sense. You asked to un-ignore
everything inside 'a' so we can't treat 'a' as (entirely) ignored and
hide it away.

> I'm sympathetic that in making that use-case work, we might have
> regressed another one, but it's hard to tell from the small example. Can
> you elaborate on your use case? Why are you both ignoring and unignoring
> everything in the directory?

Also how bad this affects you (widely-used 'wrong' behavior can become
'right', and my change a regression as a result)
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/15] ref-filter: convert variable 'width' to an unsigned int

[Karthik Nayak]

On Wed, Jan 6, 2016 at 2:42 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> In align_atom_parser() the 'width' variable is an int, which requires
>> an explicit type conversion when used in strtoul_ui(). Hence make it
>> an unsigned int.
>>
>> Helped-by: Eric Sunshine 
>> Signed-off-by: Karthik Nayak 
>> ---
>
> Shouldn't this be done in [09/15] from the beginning?  As it's not
> like [09/15] was a pure code movement, this step looks like "Oops,
> I screwed up earlier, and here is a fixup".
>
> If we want to do this as a separate step, it would be easier to
> follow if it is done _before_ 09/15 to the original of the code
> movement, I think.
>

Haha, does seem that way. Will merge it in.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 08/15] ref-filter: introduce color_atom_parser()

[Karthik Nayak]

On Wed, Jan 6, 2016 at 2:36 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> @@ -90,7 +105,7 @@ static struct {
>>   { "symref" },
>>   { "flag" },
>>   { "HEAD" },
>> - { "color" },
>> + { "color", FIELD_STR, color_atom_parser },
>>   { "align" },
>>   { "end" },
>>  };
>
> This is minor, as I do not think anybody sane would say
> "for-each-ref --sort=color" and expect anything sensible, but
>

I completely understand and the only reason that I added FIELD_STR
cause that was the default either ways.

> I think we shouldn't mark these bogus attributes [*1*] as FIELD_STR
> (and it is not FIELD_ULONG, either).  Perhaps add a new FIELD_BOGUS
> (or some better name to make it clear that this is not a "value"
> that belongs to the ref and can be used to sort, e.g. "FAKE") value
> and mark them as such?
>
> That would allow us to error out when they are specified as a
> sorting criteria.
>
>
> [Footnote]
>
> *1* Bogus from the point of view of "what are the various attributes
> specific to these items that the command is iterating over,
> i.e. refs (or the objects at the tips of these refs)", as color,
> align, etc. are not attributes per refs.

I'm sure something like FIELD_BOGUS would make a lot of sense, but I wont
include that into this series. Maybe after this?

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Dear Sir or Madam, let the New Year be more successful for you! - please help our site - пожалуйста, помогите нашему сайту - будь ласка, допоможіть нашому сайту http://kyiv230school.pp.ua/

[admin]

Dear Sir or Madam, let the New Year be more successful for you!

good day,
ask at the top of any page click once on the banner,
so that we could pay for hosting our site,
Thank you

ad...@kyiv230school.pp.ua
http://kyiv230school.pp.ua/

добрий день,
просимо на будь-якій сторінці вгорі один раз натиснути на рекламний банер,
щоб ми змогли оплатити хостинг нашого сайту,
Дякуємо

добрый день, 
просим на любой странице вверху один раз нажать на рекламный баннер,
чтобы мы смогли оплатить хостинг нашего сайта,
спасибо

Sorry if this letter has caused you inconvenience. Your address is taken from 
public sources.
To unsubscribe, please send us a a message to our email address.

ad...@kyiv230school.pp.ua
http://kyiv230school.pp.ua/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] reflog-walk: don't segfault on non-commit sha1's in the reflog

[Duy Nguyen]

On Wed, Jan 6, 2016 at 4:13 PM, Dennis Kaarsemaker
 wrote:
> On di, 2016-01-05 at 20:52 -0500, Eric Sunshine wrote:
>> On Tue, Jan 5, 2016 at 8:28 PM, Eric Sunshine <
>> sunsh...@sunshineco.com> wrote:
>> > On Tue, Jan 5, 2016 at 8:20 PM, Dennis Kaarsemaker
>> >  wrote:
>> > > On di, 2016-01-05 at 20:05 -0500, Eric Sunshine wrote:
>> > > > Hmm, this test is successful for me on OS X even without the
>> > > > reflog-walk.c changes applied.
>> > > >
>> > > > And this test actually fails (inversely) because it's expecting
>> > > > a
>> > > > failure, but doesn't get one since the command produces the
>> > > > expected
>> > > > output.
>> > >
>> > > That's... surprising to say the least. What's the content of
>> > > 'actual',
>> > > and which git.git commit are you on?
>> >
>> > % cat t/trash\ directory.t1410-reflog/actual
>> > b60a214 refs/tests/tree-in-reflog@{0}: Restoring to commit
>> > 140c527 refs/tests/tree-in-reflog@{1}: Forcing tree
>> > b60a214 refs/tests/tree-in-reflog@{2}: Creating ref
>> > %
>> >
>> > This is with only the t/t1410-reflog.sh changes from your patch
>> > applied atop current 'master' (SHA1 7548842).
>>
>> By the way, the segfault does occur for me on Linux and FreeBSD.
>>
>> And, in all cases, on all tested platforms, with the full patch
>> applied, both tests behave sanely (in the expected fashion). So, even
>> though the crash doesn't manifest everywhere, the fact that the tests
>> are meaningfully testing it on the "affected" platforms may mean that
>> it's not worth worrying about why it doesn't segfault on OS X.
>>
>> (Of course, practicality aside, one might want to satisfy one's
>> intellectual curiosity about why it behaves differently on OS X.)
>
> The only explanation I can think of (and that's with practically no
> knowledge of OS X internals) is that OS X's memory allocation strategy
> is unlucky. Git is definitely writing to a location it should not write
> to. On linux and freebsd this is unallocated memory, so you get a
> segfault. On OS X, it happens to be memory actually allocated by git,
> resulting not in a segfault but in silent corruption of other in-memory
> data. I would argue that this is a much worse result, even though in
> this small test that corruption seems to not trigger a crash.

Agreed. For a guaranteed crash, put assert(c->object.type ==
OBJ_COMMIT); in the macro function "slabname## _at_peek" in
commit-slab.h. That is if I analyzed the crash correctly. I'm not
suggesting to put the assert() in the code permanently though because
I don't know how often this function is called.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] reflog-walk: don't segfault on non-commit sha1's in the reflog

[Dennis Kaarsemaker]

On di, 2016-01-05 at 20:52 -0500, Eric Sunshine wrote:
> On Tue, Jan 5, 2016 at 8:28 PM, Eric Sunshine <
> sunsh...@sunshineco.com> wrote:
> > On Tue, Jan 5, 2016 at 8:20 PM, Dennis Kaarsemaker
> >  wrote:
> > > On di, 2016-01-05 at 20:05 -0500, Eric Sunshine wrote:
> > > > Hmm, this test is successful for me on OS X even without the
> > > > reflog-walk.c changes applied.
> > > > 
> > > > And this test actually fails (inversely) because it's expecting
> > > > a
> > > > failure, but doesn't get one since the command produces the
> > > > expected
> > > > output.
> > > 
> > > That's... surprising to say the least. What's the content of
> > > 'actual',
> > > and which git.git commit are you on?
> > 
> > % cat t/trash\ directory.t1410-reflog/actual
> > b60a214 refs/tests/tree-in-reflog@{0}: Restoring to commit
> > 140c527 refs/tests/tree-in-reflog@{1}: Forcing tree
> > b60a214 refs/tests/tree-in-reflog@{2}: Creating ref
> > %
> > 
> > This is with only the t/t1410-reflog.sh changes from your patch
> > applied atop current 'master' (SHA1 7548842).
>
> By the way, the segfault does occur for me on Linux and FreeBSD.
> 
> And, in all cases, on all tested platforms, with the full patch
> applied, both tests behave sanely (in the expected fashion). So, even
> though the crash doesn't manifest everywhere, the fact that the tests
> are meaningfully testing it on the "affected" platforms may mean that
> it's not worth worrying about why it doesn't segfault on OS X.
> 
> (Of course, practicality aside, one might want to satisfy one's
> intellectual curiosity about why it behaves differently on OS X.)

The only explanation I can think of (and that's with practically no
knowledge of OS X internals) is that OS X's memory allocation strategy
is unlucky. Git is definitely writing to a location it should not write
to. On linux and freebsd this is unallocated memory, so you get a
segfault. On OS X, it happens to be memory actually allocated by git,
resulting not in a segfault but in silent corruption of other in-memory
data. I would argue that this is a much worse result, even though in
this small test that corruption seems to not trigger a crash.

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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/16] init: allow alternate backends to be set for new repos

[Michael Haggerty]

On 01/05/2016 07:03 PM, Junio C Hamano wrote:
> David Turner  writes:
> 
>> I'm working on the rest now, but wanted to comment on this first.  I
>> went ahead and made this change, but I'm not sure I like it.  In the
>> git codebase, the concept will continue to be called "backend"; there
>> are already-accepted patches using that terminology.  Having two
>> separate names for the same thing seems confusing to me.
> 
> We have the option to update whatever "are already-accepted" [*1*].
> That would allow us to uniformly call it "ref storage", if we wanted
> to.

...whereas whatever we name the option, we have to live with forever
because it is user-facing. It's more important to get the option name
correct (though I agree that it would be nice for the nomenclature used
in the code to be reminiscent of the option name).

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git 2.7.0 gitignore behaviour regression

[Mike McQuaid]

> On 6 Jan 2016, at 09:42, Duy Nguyen  wrote:
> 
> Yeah.. I think it's the only relevant commit in 2.7.0 cycle anyway.
> These patterns "/a" followed by "!/a/*" were wrecking my head. But I
> finally decided 2.7 output makes more sense. You asked to un-ignore
> everything inside 'a' so we can't treat 'a' as (entirely) ignored and
> hide it away.
> 
>> I'm sympathetic that in making that use-case work, we might have
>> regressed another one, but it's hard to tell from the small example. Can
>> you elaborate on your use case? Why are you both ignoring and unignoring
>> everything in the directory?
> 
> Also how bad this affects you (widely-used 'wrong' behavior can become
> 'right', and my change a regression as a result)

This doesn’t affect me badly; I was able to work around the original issue 
before the bug report in a way that was consistent between Git 2.6 and Git 2.7 
but wanted to ensure that I filed something upstream just so it was a known 
issue as it was relatively easy to reproduce.

I agree that all the pattern handling stuff like in my example is pretty awful; 
it’s also a big area where libgit2 was inconsistent with Git’s behaviour on 
either of those versions too. I’ve played around and now got a .gitignore file 
that behaves consistently across Git 2.6, Git 2.7 and libgit2 0.23.4 
(https://github.com/Homebrew/homebrew/blob/master/.gitignore) so there’s no 
outstanding issue on my side.

Thanks!

Mike McQuaid
http://mikemcquaid.com

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git 2.7.0 gitignore behaviour regression

[Duy Nguyen]

On Wed, Jan 6, 2016 at 4:50 PM, Mike McQuaid  wrote:
> it’s also a big area where libgit2 was inconsistent with Git’s behaviour on 
> either of those versions too.

Yeah.. it looks like libgit2's gitignore support was written new, not
imported from C Git, so behavior differences (especially in corner
cases) and even lack of some feature ("**" support comes to mind). For
isolated features like gitignore, perhaps we can have an option to
replace C Git code with libgit2 and therefore can test libgit2 against
C Git test suite. It could be a good start for libgit2 to invade C
Git. Not sure if anybody's interested in doing it though.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] commit: ensure correct permissions of the commit message

[Jeff King]

On Wed, Jan 06, 2016 at 09:20:34AM +0100, Johannes Schindelin wrote:

> Hi Junio,
> 
> On Tue, 5 Jan 2016, Junio C Hamano wrote:
> 
> > If we want to follow the X_or_Y() pattern, fopen_or_create() may
> > describe what it does better.  I do not have strong preference either
> > way, but again I am not good at naming things (and I suspect you aren't
> > either), so...
> 
> Heh... You got that right...
> 
> Let's let it simmer for a couple more days, maybe somebody else chimes in
> with a brilliant idea... :-)

I can be the anti-brilliant and just shoot down what has been said. :)

I think fopen_or_create is confusing; it implies to me that we'll open
an existing file or create it if it's not there. But we are always about
truncating/replacing the existing file.

I think fopen_for_writing() is fine, or fopen_truncate().

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Segmentation fault found while fuzzing .pack file under 2.7.0.rc3

[Duy Nguyen]

On Tue, Jan 5, 2016 at 10:24 PM, Jeff King  wrote:
> If you can find a fuzzed packfile that crashes "index-pack", then _that_
> would be a big deal.

I'm sure you know this, but if Jacek moves to break index-pack, then
he/she should also try to break unpack-objects because sometimes we
use that command instead of index-pack.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC v2 0/2] add regex match flags to git describe

[Duy Nguyen]

On Wed, Jan 6, 2016 at 8:08 AM, Mostyn Bramley-Moore  wrote:
> On 01/04/2016 06:46 PM, Junio C Hamano wrote:
 Magic pattern annotation like we do for pathspecs Duy raised may not
 be a bad idea, either, and would probably be easier to teach people.
 Just like in Perl "(?i)$any_pattern" is a way to introduce the case
 insensitive match with $any_pattern, we may be able to pick an
 extensible magic syntax and decorate the pattern you would specify
 for matching refnames to tell Git what kind of pattern it is, e.g.

 $ git mgrep -P -e foo --refs '/?glob/release_*'

 I am not suggesting that we must use /?/ prefix
 as the "extensible magic syntax" here--I am just illustrating what
 I mean by "extensible magic syntax".
>>>
>>>
>>> I hadn't seen the pathspec magic patterns before- interesting.  We
>>> could possibly share syntax with pathspecs, ie
>>> :(?pattern-options...)pattern
>>
>>
>> Even though we have DWIM between revisions and paths on the command
>> line when the user omits "--" for disambiguation, I do not think we
>> look at the shape of the string to DWIM/decide that it is a pattern,
>> so as long as the magic syntax cannot be a valid pattern to match
>> against refs right now (and your ":(?...)"  qualifies as such, as a
>> refname would not contain a component that begins with a colon), it
>> would be possible to introduce it as the magic syntax for matching
>> refs.
>>
>> Or did you mean to use this syntax also for patterns that match
>> strings in contents, e.g.
>>
>>  $ git grep -e ':(?perl-regexp)...'
>
>
> I think it would be nice to support this syntax in every command that does
> pattern matching.
>
> Corner case: what if we want to search for ":(?perl-regexp)", eg in git's
> own source?  I suppose this would do:
> git grep -e ":(?fixed-strings):(?perl-regexp)"

We can also override it with command line options. If you define
--perl-regexp as "if no regex syntax is specified in the pattern, pcre
is used", then you can have something like --force-perl-regexp that
says "all patterns are in pcre, there's no magic whatsoever to choose
a different regex syntax". Though I think --perl-regexp should play
the role of --force-perl-regexp so you don't surprise current scripts,
and have a new option --default-syntax= instead.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/16] init: allow alternate backends to be set for new repos

[Duy Nguyen]

On Wed, Dec 23, 2015 at 8:34 PM, Michael Haggerty  wrote:
> On 12/03/2015 01:35 AM, David Turner wrote:
>> diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
>> index 8174d27..9ea6753 100644
>> --- a/Documentation/git-init.txt
>> +++ b/Documentation/git-init.txt
>> @@ -12,7 +12,7 @@ SYNOPSIS
>>  'git init' [-q | --quiet] [--bare] [--template=]
>> [--separate-git-dir ]
>> [--shared[=]] [directory]
>> -
>> +   [--refs-backend-type=]
>
> ISTM that "backend" (used here in this option name, and in the manpage)
> is not such a meaningful term to users. Could we pick a less obscure
> term? E.g., maybe "--ref-storage="?

>From an (ex-)translator point of view, storage is also easier to
translate than the technical term "backend". I know we do not
translate option names, but whatever term you use usually show up in
some user-facing messages that need translating. But I do prefer
backend in source code, I think it expresses the idea much better than
storage.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Segmentation fault found while fuzzing .pack file under 2.7.0.rc3

[Jacek Wielemborek]

W dniu 06.01.2016 o 10:46, Duy Nguyen pisze:
> On Tue, Jan 5, 2016 at 10:24 PM, Jeff King  wrote:
>> If you can find a fuzzed packfile that crashes "index-pack", then _that_
>> would be a big deal.
> 
> I'm sure you know this, but if Jacek moves to break index-pack, then
> he/she should also try to break unpack-objects because sometimes we
> use that command instead of index-pack.
> 

It sounds that you could use a little explanation on how I found this
crashing case and what would it take to fuzz index-pack, according to
the conversation I had on #git-devel on irc.freenode.net. Should I
assume that you know the basic afl-fuzz in my next post?

BTW @Duy, thanks for CC to me, I'm not subscribed to the ML.



signature.asc
Description: OpenPGP digital signature

[PATCH] interpret-trailers: add option for in-place editing

[Tobias Klauser]

From: Tobias Klauser 

Add a command line option --in-place to support in-place editing akin to
sed -i.  This allows to write commands like the following:

  git interpret-trailers --trailer "X: Y" a.txt > b.txt && mv b.txt a.txt

in a more concise way:

  git interpret-trailers --trailer "X: Y" --in-place a.txt

Also add the corresponding documentation and tests.

Signed-off-by: Tobias Klauser 
---
 Documentation/git-interpret-trailers.txt | 24 +++-
 builtin/interpret-trailers.c | 13 +++
 t/t7513-interpret-trailers.sh| 32 ++
 trailer.c| 39 
 trailer.h|  3 ++-
 5 files changed, 91 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-interpret-trailers.txt 
b/Documentation/git-interpret-trailers.txt
index 0ecd497c4de7..a77b901f1d7b 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -8,7 +8,7 @@ git-interpret-trailers - help add structured information into 
commit messages
 SYNOPSIS
 
 [verse]
-'git interpret-trailers' [--trim-empty] [(--trailer [(=|:)])...] 
[...]
+'git interpret-trailers' [--in-place] [--trim-empty] [(--trailer 
[(=|:)])...] [...]
 
 DESCRIPTION
 ---
@@ -64,6 +64,9 @@ folding rules, the encoding rules and probably many other 
rules.
 
 OPTIONS
 ---
+--in-place::
+   Edit the files in place.
+
 --trim-empty::
If the  part of any trailer contains only whitespace,
the whole trailer will be removed from the resulting message.
@@ -216,6 +219,25 @@ Signed-off-by: Alice 
 Signed-off-by: Bob 
 
 
+* Use the '--in-place' option to edit a message file in place:
++
+
+$ cat msg.txt
+subject
+
+message
+
+Signed-off-by: Bob 
+$ git interpret-trailers --trailer 'Acked-by: Alice ' 
--in-place msg.txt
+$ cat msg.txt
+subject
+
+message
+
+Signed-off-by: Bob 
+Acked-by: Alice 
+
+
 * Extract the last commit as a patch, and add a 'Cc' and a
   'Reviewed-by' trailer to it:
 +
diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 46838d24a90a..b99ae4be8875 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -12,16 +12,18 @@
 #include "trailer.h"
 
 static const char * const git_interpret_trailers_usage[] = {
-   N_("git interpret-trailers [--trim-empty] [(--trailer 
[(=|:)])...] [...]"),
+   N_("git interpret-trailers [--in-place] [--trim-empty] [(--trailer 
[(=|:)])...] [...]"),
NULL
 };
 
 int cmd_interpret_trailers(int argc, const char **argv, const char *prefix)
 {
+   int in_place = 0;
int trim_empty = 0;
struct string_list trailers = STRING_LIST_INIT_DUP;
 
struct option options[] = {
+   OPT_BOOL(0, "in-place", _place, N_("edit files in place")),
OPT_BOOL(0, "trim-empty", _empty, N_("trim empty 
trailers")),
OPT_STRING_LIST(0, "trailer", , N_("trailer"),
N_("trailer(s) to add")),
@@ -34,9 +36,12 @@ int cmd_interpret_trailers(int argc, const char **argv, 
const char *prefix)
if (argc) {
int i;
for (i = 0; i < argc; i++)
-   process_trailers(argv[i], trim_empty, );
-   } else
-   process_trailers(NULL, trim_empty, );
+   process_trailers(argv[i], in_place, trim_empty, 
);
+   } else {
+   if (in_place)
+   die(_("no input file given for in-place editing"));
+   process_trailers(NULL, in_place, trim_empty, );
+   }
 
string_list_clear(, 0);
 
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 322c436a494c..1103a4838b5c 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -326,6 +326,38 @@ test_expect_success 'with complex patch, args and 
--trim-empty' '
test_cmp expected actual
 '
 
+test_expect_success 'in-place editing with basic patch' '
+   cat basic_message >message &&
+   cat basic_patch >>message &&
+   cat basic_message >expected &&
+   echo >>expected &&
+   cat basic_patch >>expected &&
+   git interpret-trailers --in-place message &&
+   test_cmp expected message
+'
+
+test_expect_success 'in-place editing with additional trailer' '
+   cat basic_message >message &&
+   cat basic_patch >>message &&
+   cat basic_message >expected &&
+   echo >>expected &&
+   cat >>expected <<-\EOF &&
+   Reviewed-by: Alice
+   EOF
+   cat basic_patch >>expected &&
+   git interpret-trailers --trailer "Reviewed-by: Alice" --in-place 
message &&
+   test_cmp expected message
+'
+

[PATCH v2 1/2] commit: allow editing the commit message even in shared repos

[Johannes Schindelin]

It was pointed out by Yaroslav Halchenko that the file containing the
commit message is writable only by the owner, which means that we have
to rewrite it from scratch in a shared repository.

Signed-off-by: Johannes Schindelin 
---
 builtin/commit.c  |  2 +-
 git-compat-util.h |  1 +
 wrapper.c | 13 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index d054f84..89bf6ad 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -761,7 +761,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
hook_arg2 = "";
}
 
-   s->fp = fopen(git_path(commit_editmsg), "w");
+   s->fp = fopen_for_writing(git_path(commit_editmsg));
if (s->fp == NULL)
die_errno(_("could not open '%s'"), git_path(commit_editmsg));
 
diff --git a/git-compat-util.h b/git-compat-util.h
index 2da0a75..e8f2867 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -733,6 +733,7 @@ extern int xmkstemp_mode(char *template, int mode);
 extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
 extern int odb_pack_keep(char *name, size_t namesz, const unsigned char *sha1);
 extern char *xgetcwd(void);
+extern FILE *fopen_for_writing(const char *path);
 
 #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x)))
 
diff --git a/wrapper.c b/wrapper.c
index b43d437..29a45d2 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -391,6 +391,19 @@ FILE *xfdopen(int fd, const char *mode)
return stream;
 }
 
+FILE *fopen_for_writing(const char *path)
+{
+   FILE *ret = fopen(path, "w");
+
+   if (!ret && errno == EPERM) {
+   if (!unlink(path))
+   ret = fopen(path, "w");
+   else
+   errno = EPERM;
+   }
+   return ret;
+}
+
 int xmkstemp(char *template)
 {
int fd;
-- 
2.6.3.windows.1.300.g1c25e49


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] Handle more file writes correctly in shared repos

[Johannes Schindelin]

In shared repositories, we have to be careful when writing files whose
permissions do not allow users other than the owner to write them.

In particular, we force the marks file of fast-export and the FETCH_HEAD
when fetching to be rewritten from scratch.

This commit does not touch the following users of fopen() that want to
write files:

- git am, when splitting mails (git-am correctly cleans up its directory
  after finishing, so there is no need to share those files between users)

- git apply, when writing rejected hunks

- git fsck, when writing lost blobs

- git merge-file, when writing merged files (when Git itself calls
  merge-file, the file in question was already there, with shared
  permissions).

- git submodule clone, when writing the .git file, because the file will
  not be overwritten

- git_terminal_prompt() in compat/terminal.c, because it is not writing to
  a file at all

- git diff --output, because the output file is clearly not intended to be
  shared between the users of the current repository

- git fast-import, when writing a crash report

- mailinfo() in mailinfo.c, because the output is clearly not intended to
  be shared between the users of the current repository

- check_or_regenerate_marks() in remote-testsvn.c, because this is only
  used for Git's internal testing

- git rerere, when writing resolved files, because the files in question
  were already written with the correct permissions

Note that this patch does not touch callers of write_file() and
write_file_gently(), which would benefit from the same scrutiny as to
usage in shared repositories. Most notable users: branch, daemon,
submodule & worktree, and a worrisome call in transport.c when updating
one ref (which ignores the shared flag).

Signed-off-by: Johannes Schindelin 
---
 builtin/fast-export.c | 2 +-
 builtin/fetch.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d9ac5d8..2471297 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -880,7 +880,7 @@ static void export_marks(char *file)
FILE *f;
int e = 0;
 
-   f = fopen(file, "w");
+   f = fopen_for_writing(file);
if (!f)
die_errno("Unable to open marks file %s for writing.", file);
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 586840d..33f04c1 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -840,7 +840,7 @@ static void check_not_current_branch(struct ref *ref_map)
 static int truncate_fetch_head(void)
 {
const char *filename = git_path_fetch_head();
-   FILE *fp = fopen(filename, "w");
+   FILE *fp = fopen_for_writing(filename);
 
if (!fp)
return error(_("cannot open %s: %s\n"), filename, 
strerror(errno));
-- 
2.6.3.windows.1.300.g1c25e49
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 0/2] Correctly handle transient files in shared repositories

[Johannes Schindelin]

Transient files, e.g. commit messages, are writable only by the owner,
even in shared repositories, to avoid interference between competing
users working on the same files.

These files are typically not deleted after use. As a consequence, we
have to delete such files before writing when they are owned by someone
else than the current user.

Reported-by: Yaroslav Halchenko 


Johannes Schindelin (2):
  commit: allow editing the commit message even in shared repos
  Handle more file writes correctly in shared repos

 builtin/commit.c  |  2 +-
 builtin/fast-export.c |  2 +-
 builtin/fetch.c   |  2 +-
 git-compat-util.h |  1 +
 wrapper.c | 13 +
 5 files changed, 17 insertions(+), 3 deletions(-)

Interdiff vs v1:

 diff --git a/builtin/commit.c b/builtin/commit.c
 index 3bfd457..89bf6ad 100644
 --- a/builtin/commit.c
 +++ b/builtin/commit.c
 @@ -761,7 +761,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
hook_arg2 = "";
}
  
 -  s->fp = fopen(git_path(commit_editmsg), "w");
 +  s->fp = fopen_for_writing(git_path(commit_editmsg));
if (s->fp == NULL)
die_errno(_("could not open '%s'"), git_path(commit_editmsg));
  
 @@ -905,7 +905,6 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
strbuf_release(_ident);
  
fclose(s->fp);
 -  adjust_shared_perm(git_path(commit_editmsg));
  
/*
 * Reject an attempt to record a non-merge empty commit without
 diff --git a/builtin/fast-export.c b/builtin/fast-export.c
 index d9ac5d8..2471297 100644
 --- a/builtin/fast-export.c
 +++ b/builtin/fast-export.c
 @@ -880,7 +880,7 @@ static void export_marks(char *file)
FILE *f;
int e = 0;
  
 -  f = fopen(file, "w");
 +  f = fopen_for_writing(file);
if (!f)
die_errno("Unable to open marks file %s for writing.", file);
  
 diff --git a/builtin/fetch.c b/builtin/fetch.c
 index 586840d..33f04c1 100644
 --- a/builtin/fetch.c
 +++ b/builtin/fetch.c
 @@ -840,7 +840,7 @@ static void check_not_current_branch(struct ref *ref_map)
  static int truncate_fetch_head(void)
  {
const char *filename = git_path_fetch_head();
 -  FILE *fp = fopen(filename, "w");
 +  FILE *fp = fopen_for_writing(filename);
  
if (!fp)
return error(_("cannot open %s: %s\n"), filename, 
strerror(errno));
 diff --git a/git-compat-util.h b/git-compat-util.h
 index 2da0a75..e8f2867 100644
 --- a/git-compat-util.h
 +++ b/git-compat-util.h
 @@ -733,6 +733,7 @@ extern int xmkstemp_mode(char *template, int mode);
  extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
  extern int odb_pack_keep(char *name, size_t namesz, const unsigned char 
*sha1);
  extern char *xgetcwd(void);
 +extern FILE *fopen_for_writing(const char *path);
  
  #define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), (alloc) * sizeof(*(x)))
  
 diff --git a/wrapper.c b/wrapper.c
 index b43d437..29a45d2 100644
 --- a/wrapper.c
 +++ b/wrapper.c
 @@ -391,6 +391,19 @@ FILE *xfdopen(int fd, const char *mode)
return stream;
  }
  
 +FILE *fopen_for_writing(const char *path)
 +{
 +  FILE *ret = fopen(path, "w");
 +
 +  if (!ret && errno == EPERM) {
 +  if (!unlink(path))
 +  ret = fopen(path, "w");
 +  else
 +  errno = EPERM;
 +  }
 +  return ret;
 +}
 +
  int xmkstemp(char *template)
  {
int fd;

-- 
2.6.3.windows.1.300.g1c25e49

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Questions on passing --depth to git-clone vs. git-fetch

[Sebastian Schuberth]

On 1/6/2016 13:41, Duy Nguyen wrote:


I think the culprit is the "git remote add" line. "git clone --depth"
by default will fetch only one branch (aka --single-branch option in
git-clone). But I suspect when you add a new remote, the default


Now that you mention it I see this being documented as part of 
--single-branch instead of --depth, which I think is confusing. I'll 
send a patch.


--
Sebastian Schuberth


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] docs: clarify that passing --depth to git-clone implies --single-branch

[Sebastian Schuberth]

It is confusing to document how --depth behaves as part of the
--single-branch docs. Better move that part to the --depth docs, saying
that it implies --single-branch by default.

Signed-off-by: Sebastian Schuberth 
---
 Documentation/git-clone.txt | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 6bf000d..943de8b 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -190,15 +190,14 @@ objects from the source repository into a pack in the 
cloned repository.
 
 --depth ::
Create a 'shallow' clone with a history truncated to the
-   specified number of revisions.
+   specified number of revisions. Implies `--single-branch` unless
+   `--no-single-branch` is given to fetch the histories near the
+   tips of all branches.
 
 --[no-]single-branch::
Clone only the history leading to the tip of a single branch,
either specified by the `--branch` option or the primary
-   branch remote's `HEAD` points at. When creating a shallow
-   clone with the `--depth` option, this is the default, unless
-   `--no-single-branch` is given to fetch the histories near the
-   tips of all branches.
+   branch remote's `HEAD` points at.
Further fetches into the resulting repository will only update the
remote-tracking branch for the branch this option was used for the
initial cloning.  If the HEAD at the remote did not point at any
-- 
2.7.0.windows.1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] interpret-trailers: add option for in-place editing

[Matthieu Moy]

Tobias Klauser  writes:

> From: Tobias Klauser 
>
> Add a command line option --in-place to support in-place editing akin to
> sed -i.  This allows to write commands like the following:

Since -i is a common shortcut for --in-place (perl -i, sed -i), it
probably makes sense to have it here too. OTOH, this is meant for
scripting and perhaps it's best to force script writters to be verbose.

> Also add the corresponding documentation and tests.

This sentence does not harm, but I personnally don't think it's really
helpfull, as it's already clear by the diffstat right below, and the
patch itself.

> -static void print_tok_val(const char *tok, const char *val)
> +static void print_tok_val(FILE *fp, const char *tok, const char *val)
>  {
>   char c = last_non_space_char(tok);
>   if (!c)
>   return;
>   if (strchr(separators, c))
> - printf("%s%s\n", tok, val);
> + fprintf(fp, "%s%s\n", tok, val);
>   else
> - printf("%s%c %s\n", tok, separators[0], val);
> + fprintf(fp, "%s%c %s\n", tok, separators[0], val);
>  }

The patch would be even easier to read if split into a preparatory
refactoring patch "turn printf into fprintf" and then the actual one.
But it's already rather clear, so you can probably leave it as-is.

> -static void print_lines(struct strbuf **lines, int start, int end)
> +static void print_lines(FILE *fp, struct strbuf **lines, int start, int end)

Here and below: it would probably be more readable with a more explicit
name for fp, like "outfile". Especially here:

> -static int process_input_file(struct strbuf **lines,
> +static int process_input_file(FILE *fp,
> +   struct strbuf **lines,

Where it's tempting to think that a parameter given to
process_input_file is ... the input file!

Other than these minor details, the patch looks good to me. Thanks for
the patch!

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Questions on passing --depth to git-clone vs. git-fetch

[Sebastian Schuberth]

Hi,

I recently compared the results of doing

$ git clone --depth=1 https://github.com/git/git.git git-clone-depth-1

versus

$ mkdir git-fetch-depth-1
$ cd git-fetch-depth-1
$ git init
$ git remote add origin https://github.com/git/git.git
$ git fetch --depth=1

and noticed a few things:

1. The docs of clone [1] say about --depth "Create a shallow clone with a 
history truncated to the specified number of revisions" while for fetch the 
docs [2] say "[...] to the specified number of commits [...]". As in this 
particular case revision are always commits, I think the clone docs should also 
say "commits".

2. In the fetch docs --depth is described to "Deepen or shorten the history of 
a shallow repository created by git clone". That sounds as if my example from 
above where I initialze a repo manually would not allow fetch to be called with 
--depth as I did not clone before. But in fact my example works fine. I guess 
we need some clarfication in the wording here.

3. When running "git log --all -oneline" in the two working trees I get 
different results, which is not what I'd expect:

$ cd git-clone-depth-1
$ git log --all --oneline
  7548842 Git 2.7

versus

$ cd git-fetch-depth-1
$ git log --all --oneline
  b819526 Merge branch 'jk/notes-merge-from-anywhere' into pu
  e2281f4 What's cooking (2016/01 #01)
  ef7b32d Sync with 2.7
  7548842 Git 2.7
  833e482 Git 2.6.5

So in the clone case only the specified number of commits from the tip of the 
default branch (master in this case) is fetched, not of each remote branch 
history. fetch in the other hand really gets the specified number of commits 
from the tip of each remote branch history. I don't know whether this behavior 
is inded or not as I cannot find any docs on it either way. But it seems 
inconsistent to me that clone with --depth only gets the history for the 
default branch, as clone without --depth would give me the history of all 
branches.

For completeness, I'm using Git for Windows 2.7.

Any comments?

[1] https://git-scm.com/docs/git-clone
[1] https://git-scm.com/docs/git-fetch

-- 
Sebastian Schuberth

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Questions on passing --depth to git-clone vs. git-fetch

[Duy Nguyen]

On Wed, Jan 6, 2016 at 7:30 PM, Sebastian Schuberth
 wrote:
> Hi,
>
> I recently compared the results of doing
>
> $ git clone --depth=1 https://github.com/git/git.git git-clone-depth-1
>
> versus
>
> $ mkdir git-fetch-depth-1
> $ cd git-fetch-depth-1
> $ git init
> $ git remote add origin https://github.com/git/git.git
> $ git fetch --depth=1
>
> and noticed a few things:

I think the culprit is the "git remote add" line. "git clone --depth"
by default will fetch only one branch (aka --single-branch option in
git-clone). But I suspect when you add a new remote, the default
refspec is to get all refs. So "git fetch --depth=1" means fetch _all_
refs, each one has maximum one commit. "git log --graph --decorate"
should show this clearer.

> 1. The docs of clone [1] say about --depth "Create a shallow clone with a 
> history truncated to the specified number of revisions" while for fetch the 
> docs [2] say "[...] to the specified number of commits [...]". As in this 
> particular case revision are always commits, I think the clone docs should 
> also say "commits".
>
> 2. In the fetch docs --depth is described to "Deepen or shorten the history 
> of a shallow repository created by git clone". That sounds as if my example 
> from above where I initialze a repo manually would not allow fetch to be 
> called with --depth as I did not clone before. But in fact my example works 
> fine. I guess we need some clarfication in the wording here.
>
> 3. When running "git log --all -oneline" in the two working trees I get 
> different results, which is not what I'd expect:
>
> $ cd git-clone-depth-1
> $ git log --all --oneline
>   7548842 Git 2.7
>
> versus
>
> $ cd git-fetch-depth-1
> $ git log --all --oneline
>   b819526 Merge branch 'jk/notes-merge-from-anywhere' into pu
>   e2281f4 What's cooking (2016/01 #01)
>   ef7b32d Sync with 2.7
>   7548842 Git 2.7
>   833e482 Git 2.6.5
>
> So in the clone case only the specified number of commits from the tip of the 
> default branch (master in this case) is fetched, not of each remote branch 
> history. fetch in the other hand really gets the specified number of commits 
> from the tip of each remote branch history. I don't know whether this 
> behavior is inded or not as I cannot find any docs on it either way. But it 
> seems inconsistent to me that clone with --depth only gets the history for 
> the default branch, as clone without --depth would give me the history of all 
> branches.
>
> For completeness, I'm using Git for Windows 2.7.
>
> Any comments?
>
> [1] https://git-scm.com/docs/git-clone
> [1] https://git-scm.com/docs/git-fetch
>
> --
> Sebastian Schuberth
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] interpret-trailers: add option for in-place editing

[Tobias Klauser]

Thanks for your feedback Matthieu!

On 2016-01-06 at 15:19:45 +0100, Matthieu Moy  
wrote:
> Tobias Klauser  writes:
> 
> > From: Tobias Klauser 
> >
> > Add a command line option --in-place to support in-place editing akin to
> > sed -i.  This allows to write commands like the following:
> 
> Since -i is a common shortcut for --in-place (perl -i, sed -i), it
> probably makes sense to have it here too. OTOH, this is meant for
> scripting and perhaps it's best to force script writters to be verbose.

Yes, I thought this would mainly be used in scripts and thus omitted the
short option.

> > Also add the corresponding documentation and tests.
> 
> This sentence does not harm, but I personnally don't think it's really
> helpfull, as it's already clear by the diffstat right below, and the
> patch itself.

Ok, I can omit it for v2.

> > -static void print_tok_val(const char *tok, const char *val)
> > +static void print_tok_val(FILE *fp, const char *tok, const char *val)
> >  {
> > char c = last_non_space_char(tok);
> > if (!c)
> > return;
> > if (strchr(separators, c))
> > -   printf("%s%s\n", tok, val);
> > +   fprintf(fp, "%s%s\n", tok, val);
> > else
> > -   printf("%s%c %s\n", tok, separators[0], val);
> > +   fprintf(fp, "%s%c %s\n", tok, separators[0], val);
> >  }
> 
> The patch would be even easier to read if split into a preparatory
> refactoring patch "turn printf into fprintf" and then the actual one.
> But it's already rather clear, so you can probably leave it as-is.

Ok. I have also no problem with splitting it. I'll wait for a feedback
from Junio on what he prefers.

> > -static void print_lines(struct strbuf **lines, int start, int end)
> > +static void print_lines(FILE *fp, struct strbuf **lines, int start, int 
> > end)
> 
> Here and below: it would probably be more readable with a more explicit
> name for fp, like "outfile". Especially here:
> 
> > -static int process_input_file(struct strbuf **lines,
> > +static int process_input_file(FILE *fp,
> > + struct strbuf **lines,
> 
> Where it's tempting to think that a parameter given to
> process_input_file is ... the input file!

Yes, makes sense. I'll change it to a more concise and readable name
I'd also take "outfile" as you suggest, unless anyone objects.

Thanks
Tobias
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] git-candidate: git based patch tracking and review

[Richard Ipsum]

On Tue, Dec 01, 2015 at 04:00:52PM -0500, Dave Borowitz wrote:
> On Tue, Dec 1, 2015 at 3:55 PM, Jonathan Nieder  wrote:
> > Cc-ing dborowitz, who has been working on storing Gerrit's code review
> > information in Git instead of a separate database (e.g., see [1]).
> 
> Thanks, we actually already had a thread going that I realize only in
> retrospect did not include the git mailing list.

Thanks, we did indeed have a thread going,
that probably should have been on-list.
I'm also working on a little library[1] that should eventually allow other
tools to use gerrit's notedb in the hope that this might
eventually lead to the development/adoption of a common format for such tools.

One thing that concerns me about notedb with respect to distributed review
is the storage format for comments. Within a distributed review system comments
may be made in any order, yet the format is designed around the kind of
linearisation that can be assumed by a centralised system.

The problem is that multiple comments in notedb may be stored within
the same blob/note, specifically all comments on a particular commit will
be stored in the same blob. In a distributed system storing multiple comments
in the same blob like this will inevitably lead to merge conflicts.

This problem isn't unsolvable, someone already suggested to me the idea
of writing a custom merge driver for merging different notes stored in notedb.
It would obviously be preferable to have a format that avoided creating
conflicts in the first place, but a custom merge driver doesn't seem like
an unreasonable solution.

If anyone has any thoughts on how else this problem might be solved,
I'd be very interested to hear them.

Thanks again,
Richard Ipsum

[1]: https://bitbucket.org/richardipsum/perl-notedb
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 14/15] ref-filter: introduce contents_atom_parser()

[Karthik Nayak]

On Wed, Jan 6, 2016 at 2:52 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> Introduce contents_atom_parser() which will parse the '%(contents)'
>> atom and store information into the 'used_atom' structure based on the
>> modifiers used along with the atom.
>>
>> Helped-by: Ramsay Jones 
>> Helped-by: Eric Sunshine 
>> Signed-off-by: Karthik Nayak 
>> ---
>>  ref-filter.c | 74 
>> +---
>>  1 file changed, 46 insertions(+), 28 deletions(-)
>>
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 32b4674..9e61530 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -39,6 +39,10 @@ static struct used_atom {
>>   struct align align;
>>   enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
>>   remote_ref;
>> + struct {
>> + enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, 
>> C_SUB } option;
>> + unsigned int nlines;
>> + } contents;
>>   } u;
>>  } *used_atom;
>>  static int used_atom_cnt, need_tagged, need_symref;
>> @@ -96,6 +100,35 @@ static void remote_ref_atom_parser(struct used_atom 
>> *atom)
>>   die(_("unrecognized format: %%(%s)"), atom->name);
>>  }
>>
>> +static void contents_atom_parser(struct used_atom *atom)
>> +{
>> + const char * buf;
>> +
>> + if (match_atom_name(atom->name, "subject", ) && !buf) {
>> + atom->u.contents.option = C_SUB;
>> + return;
>> + } else if (match_atom_name(atom->name, "body", ) && !buf) {
>> + atom->u.contents.option = C_BODY_DEP;
>> + return;
>> + } if (!match_atom_name(atom->name, "contents", ))
>> +   die("BUG: parsing non-'contents'");
>
> Did you really intend to say "if" here, not "else if"?
>

Not that it makes a difference here since both the previous
condition return. I think "else if" would be better.

> I also wonder if the "&& !buf" in the first two are correct.  What
> should happen to "%(subject:foo)", which gives you a non-empty buf?
> It may or may not be an error, but it should not fall thru and cause
> "BUG:parsing non-contents", should it?
>

I think It would be better to add specific messages there

if (match_atom_name(atom->name, "subject", )) {
if (buf)
die("%%(subject) atom does not take arguments");
atom->u.contents.option = C_SUB;
return;
} else if (match_atom_name(atom->name, "body", )) {
if (buf)
die("%%(body) atom does not take arguments");
atom->u.contents.option = C_BODY_DEP;
return;
} else if (!match_atom_name(atom->name, "contents", ))
 die("BUG: parsing non-'contents'");

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 15/15] ref-filter: introduce objectname_atom_parser()

[Karthik Nayak]

On Wed, Jan 6, 2016 at 3:01 AM, Junio C Hamano  wrote:
> Karthik Nayak  writes:
>
>> @@ -467,15 +482,17 @@ static void *get_obj(const unsigned char *sha1, struct 
>> object **obj, unsigned lo
>>  }
>>
>>  static int grab_objectname(const char *name, const unsigned char *sha1,
>> - struct atom_value *v)
>> +struct atom_value *v, struct used_atom *atom)
>>  {
>> - if (!strcmp(name, "objectname")) {
>> - v->s = xstrdup(sha1_to_hex(sha1));
>> - return 1;
>> - }
>> - if (!strcmp(name, "objectname:short")) {
>> - v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV));
>> - return 1;
>> + if (starts_with(name, "objectname")) {
>
> The original used to reject "objectnamefoo", but the updated one is
> more sloppy.  Shouldn't it be doing the same match_atom_name() here?
>
> This comment applies to many remaining strcmp() and starts_with()
> that is reachable from populate_value().

Should we worry about such extensions of atoms? I see that
parse_ref_filter_atom()
takes care of something like that in the beginning itself

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git 2.7.0 gitignore behaviour regression

[Junio C Hamano]

On Wed, Jan 6, 2016 at 2:03 AM, Duy Nguyen  wrote:
>
> On Wed, Jan 6, 2016 at 4:50 PM, Mike McQuaid  wrote:
> > it’s also a big area where libgit2 was inconsistent with Git’s behaviour on 
> > either of those versions too.
>
> Yeah.. it looks like libgit2's gitignore support was written new, not
> imported from C Git, so behavior differences (especially in corner
> cases) and even lack of some feature ("**" support comes to mind). For
> isolated features like gitignore, perhaps we can have an option to
> replace C Git code with libgit2 and therefore can test libgit2 against
> C Git test suite. It could be a good start for libgit2 to invade C
> Git. Not sure if anybody's interested in doing it though.

Yup, an area that is reasonably isolated from the remainder of the system like
this might be a good starting point. But I suspect that the invasion needs to
happen in the opposite direction in this particular case before it happens.
That is, if libgit2's implementation does not behave like how we do, it needs to
be fixed, possibly by discarding what they did and instead importing code from
us. After the behaviour of libgit2 is fixed, we can talk about the
invasion in the
opposite direction.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] interpret-trailers: add option for in-place editing

[Eric Sunshine]

On Wed, Jan 6, 2016 at 8:34 AM, Tobias Klauser
 wrote:
> Add a command line option --in-place to support in-place editing akin to
> sed -i.  This allows to write commands like the following:
>
>   git interpret-trailers --trailer "X: Y" a.txt > b.txt && mv b.txt a.txt
>
> in a more concise way:
>
>   git interpret-trailers --trailer "X: Y" --in-place a.txt
>
> Also add the corresponding documentation and tests.

In addition to Matthieu's comments...

> Signed-off-by: Tobias Klauser 
> ---
> diff --git a/trailer.c b/trailer.c
> @@ -856,19 +858,28 @@ void process_trailers(const char *file, int trim_empty, 
> struct string_list *trai
>
> lines = read_input_file(file);
>
> +   if (in_place) {
> +   fp = fopen(file, "w");
> +   if (!fp)
> +   die_errno(_("could not open file '%s' for writing"), 
> file);
> +   }

The input file should be considered precious, but this implementation
plays too loosely with it. If the user interrupts the program or a
die() somewhere within the "trailers" code aborts the program before
the output file is written, then the original file will be
irrecoverably lost. Users won't be happy about that.

Instead, this code should go through the standard dance of writing the
output to a new file (with some unique temporary name) and then, only
once the output has been successfully written in full, rename the new
file atop the old.

> /* Print the lines before the trailers */
> -   trailer_end = process_input_file(lines, _tok_first, _tok_last);
> +   trailer_end = process_input_file(fp, lines, _tok_first, 
> _tok_last);
>
> arg_tok_first = process_command_line_args(trailers);
>
> process_trailers_lists(_tok_first, _tok_last, _tok_first);
>
> -   print_all(in_tok_first, trim_empty);
> +   print_all(fp, in_tok_first, trim_empty);
>
> free_all(_tok_first);
>
> /* Print the lines after the trailers as is */
> -   print_lines(lines, trailer_end, INT_MAX);
> +   print_lines(fp, lines, trailer_end, INT_MAX);
> +
> +   if (in_place)
> +   fclose(fp);
>
> strbuf_list_free(lines);
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] commit: ensure correct permissions of the commit message

[Johannes Schindelin]

Hi Junio,

On Tue, 5 Jan 2016, Junio C Hamano wrote:

> If we want to follow the X_or_Y() pattern, fopen_or_create() may
> describe what it does better.  I do not have strong preference either
> way, but again I am not good at naming things (and I suspect you aren't
> either), so...

Heh... You got that right...

Let's let it simmer for a couple more days, maybe somebody else chimes in
with a brilliant idea... :-)

Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Advice regarding inherited git repository

[Jeff King]

On Tue, Jan 05, 2016 at 05:42:15PM +, Danielle wrote:

> I inherited a web site and a git repository. the git repository is cloned 
> to the website and a sandbox website (two clones). No commits have been 
> done in more than 6 months. The main site has been updated a lot of times, 
> the sandbox has lots of test and exploratory code. To bring things up to 
> date, I thought of staging and committing all the files in the main site 
> and pushing to the remote repository, and then fetching into the sandbox 
> clone and merging what is needed (which will be sooo not trivial). Does 
> this make sense? Or would it be better to create a sandbox branch somehow? 
> I'm used to working with SVN, no real git experience, so I would 
> appreciate any advice on how to manage this.

You probably should create a sandbox branch, for your own sanity.
Because git is distributed, each separate repository is implicitly a
branch. So if you did something like:

  1. Commit all the changes on the main site to "master". Push the
 result to some common remote.

  2. Commit all the changes in the sandbox to its "master". Do _not_
 push to the remote.

  3. Pull the changes from the remote into the sandbox. If they're worth
 adding to the main site, push them up. If not, leave them there.

That works; "master" in the sandbox has the experimental changes, but
you don't have to push them anywhere.  However, you are one accidental
"git push" away from sending all of those experimental features to your
remote "master". So it's easy enough to make step 1.5 "git checkout -b
sandbox-cruft" in the sandbox repository, so git knows not to ever push
it to "master".

In general, I'd also say you may benefit from splitting the changes in
each repository into as many separate logical commits as you can (and
possibly even to separate topic branches that you can merge
independently). Given that you inherited this, that's _probably_ too
much work. But if you do want to try it, I recommend "git add -p" for
picking out individual hunks.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Initial git clone behaviour

[Junio C Hamano]

On Wed, Jan 6, 2016 at 2:26 PM, Eric Curtin  wrote:
>
> Often I do a standard git clone:
>
> git clone (name of repo)
>
> Followed by a depth=1 clone in parallel, so I can get building and
> working with the code asap:
>
> git clone --depth=1 (name of repo)
>
> Could we change the default behavior of git so that we initially get
> all the current files quickly so that we can start working them and
> then getting the rest of the data? At least a user could get to work
> quicker this way. Any disadvantages of this approach?

It would put more burden on a shared and limited resource (i.e.
the server side).

For example, I just tried a depth=1 clone of Linus's repository from

  git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

which transferred ~150MB pack data to check out 52k files in 90 seconds.

On the other hand, a full clone transferred ~980MB pack data and it took
170 seconds to complete. You can already see that a full clone is highly
optimized--it does not take even twice the time of getting the most recent
checkout to grab 10 years worth of development (562k of commits).

This efficiency comes from some tradeoffs, and one of them is that not
all the data necessary to check out the latest tree contents can be stored
near the beginning of the pack data. So "we'll checkout the tip while the
remainder of the data is still incoming" would not be a workable, unless
you are willing to destroy the full-clone performance.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC v2 2/3] refs: resolve symbolic refs first

[David Turner]

Before committing ref updates, split symbolic ref updates into two
parts: an update to the underlying ref, and a log-only update to the
symbolic ref.  This ensures that both references are locked correctly
while their reflogs are updated.

It is still possible to confuse git by concurrent updates, since the
splitting of symbolic refs does not happen under lock. So a symbolic ref
could be replaced by a plain ref in the middle of this operation, which
would lead to reflog discontinuities and missed old-ref checks.

Signed-off-by: David Turner 
---
 refs.c   |  70 +
 refs/files-backend.c | 123 +--
 refs/refs-internal.h |   8 
 3 files changed, 139 insertions(+), 62 deletions(-)

diff --git a/refs.c b/refs.c
index 38f29b3..2496b91 100644
--- a/refs.c
+++ b/refs.c
@@ -1126,6 +1126,72 @@ int refs_init_db(struct strbuf *err, int shared)
return the_refs_backend->init_db(err, shared);
 }
 
+/*
+ * Special case for symbolic refs when REF_NODEREF is not turned on.
+ * Dereference them here, mark them REF_LOG_ONLY, and add an update
+ * for the underlying ref.
+ */
+static int dereference_symrefs(struct ref_transaction *transaction,
+  struct strbuf *err)
+{
+   int i;
+   int nr = transaction->nr;
+
+   for (i = 0; i < nr; i++) {
+   struct ref_update *update = transaction->updates[i];
+   const char *resolved;
+   unsigned char sha1[20];
+   int resolve_flags = 0;
+   int mustexist = (update->old_sha1 &&
+!is_null_sha1(update->old_sha1));
+   int deleting = (update->flags & REF_HAVE_NEW) &&
+   is_null_sha1(update->new_sha1);
+   struct ref_update *new_update;
+
+   if (mustexist)
+   resolve_flags |= RESOLVE_REF_READING;
+   if (deleting)
+   resolve_flags |= RESOLVE_REF_ALLOW_BAD_NAME |
+   RESOLVE_REF_NO_RECURSE;
+
+   if (strcmp(update->refname, "HEAD"))
+   update->flags |= REF_IS_NOT_HEAD;
+
+   resolved = resolve_ref_unsafe(update->refname, resolve_flags,
+ sha1, >type);
+   if (!resolved) {
+   /*
+* We'll try again to resolve this during
+* commit and give a better error message
+* then, but we know it's not a symbolic ref
+* (or, indeed, any sort of ref).
+*/
+   continue;
+   }
+
+   hashcpy(update->read_sha1, sha1);
+
+   if (update->flags & REF_NODEREF ||
+   !(update->type & REF_ISSYMREF))
+   continue;
+
+   /* Create a new transaction for the underlying ref */
+   if (ref_transaction_update(transaction,
+  resolved,
+  update->new_sha1,
+  (update->flags & REF_HAVE_OLD) ?
+  update->old_sha1 : NULL,
+  update->flags & ~REF_IS_NOT_HEAD,
+  update->msg, err))
+   return -1;
+
+   /* Make the symbolic ref update log-only */
+   update->flags |= REF_LOG_ONLY | REF_NODEREF;
+   }
+
+   return 0;
+}
+
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
@@ -1142,6 +1208,10 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
return 0;
}
 
+   ret = dereference_symrefs(transaction, err);
+   if (ret)
+   goto done;
+
if (get_affected_refnames(transaction, _refnames, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto done;
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0800a57..ce73bc4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -7,7 +7,6 @@
 
 struct ref_lock {
char *ref_name;
-   char *orig_ref_name;
struct lock_file *lk;
struct object_id old_oid;
 };
@@ -1839,7 +1838,6 @@ static void unlock_ref(struct ref_lock *lock)
if (lock->lk)
rollback_lock_file(lock->lk);
free(lock->ref_name);
-   free(lock->orig_ref_name);
free(lock);
 }
 
@@ -1890,6 +1888,7 @@ static int remove_empty_directories(struct strbuf *path)
  */
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
+   const unsigned char *read_sha1,

Re: [PATCH 12/16] refs: always handle non-normal refs in files backend

[David Turner]

On Wed, 2015-12-23 at 09:02 +0100, Michael Haggerty wrote:
> I very much like the idea of introducing special handling for
> symbolic
> reference updates within a transaction. In fact, I think I would go
> even
> farther:
> 
> Let's take the example of an update to HEAD, which currently points
> at
> refs/heads/master. I think it would *always* be a good idea (i.e.,
> even
> when only the files backend is in use) to split that ref_update into
> two
> ref_updates:
> 
> 1. One to update refs/heads/master and add a reflog entry for that
>reference.
> 
> 2. One to add a reflog entry for HEAD (i.e. using the new
> REF_LOG_ONLY
>flag suggested above).

Having now implemented this, I think it's ugly.

While committing a ref_update, we read the ref's old value.  We need
this value for two things: 1. confirming that it matches old_sha1 in
the ref_update and 2. writing to the reflog.  In the case of a symbolic
ref, we need to resolve it in order to get its old value.  But we've
already resolved it during the splitting step (in order to know that it
was a symbolic ref).  So we're duplicating effort.  We can store the
resolved SHA from the split phase (along with the resolved ref type),
but that's just leaking more weird details.  

In addition, this doesn't actually solve the problem that this patch is
intended to solve; I still have to split refs into normal and non
-normal (files-backend).  It's true that this splitting is simplified,
since we don't need to do weird things for the reflogs, but I'm not
sure it's a net win.

I'm going to send just these patches in reply to your mail (I hope),
and see what you think before continuing down this path.  
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] clone: use child_process for recursive checkouts

[David Turner]

On Wed, 2015-12-23 at 12:30 +0100, Michael Haggerty wrote:
> Signed-off-by: Michael Haggerty 
> ---
> David, I think if you insert this patch before your
> 
>   13/16 refs: allow ref backend to be set for clone
> 
> , then the hunk in builtin/clone.c:checkout() of your patch becomes
> trivial:
> 
>   if (refs_backend_type)
>   argv_array_pushf(, "--refs-backend-type=%s",
>refs_backend_type);

Inserted, thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Initial git clone behaviour

[Eric Curtin]

On 6 January 2016 at 23:14, Junio C Hamano  wrote:
> On Wed, Jan 6, 2016 at 2:26 PM, Eric Curtin  wrote:
>>
>> Often I do a standard git clone:
>>
>> git clone (name of repo)
>>
>> Followed by a depth=1 clone in parallel, so I can get building and
>> working with the code asap:
>>
>> git clone --depth=1 (name of repo)
>>
>> Could we change the default behavior of git so that we initially get
>> all the current files quickly so that we can start working them and
>> then getting the rest of the data? At least a user could get to work
>> quicker this way. Any disadvantages of this approach?
>
> It would put more burden on a shared and limited resource (i.e.
> the server side).
>
> For example, I just tried a depth=1 clone of Linus's repository from
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>
> which transferred ~150MB pack data to check out 52k files in 90 seconds.
>
> On the other hand, a full clone transferred ~980MB pack data and it took
> 170 seconds to complete. You can already see that a full clone is highly
> optimized--it does not take even twice the time of getting the most recent
> checkout to grab 10 years worth of development (562k of commits).
>
> This efficiency comes from some tradeoffs, and one of them is that not
> all the data necessary to check out the latest tree contents can be stored
> near the beginning of the pack data. So "we'll checkout the tip while the
> remainder of the data is still incoming" would not be a workable, unless
> you are willing to destroy the full-clone performance.

Ok, my internet connection at home is pretty terrible then! I don't get
nowhere near these timings. It takes over an hour to do a full clone
from my house. And approx 30 mins for the depth=1 (approx, did not time
it).

That all makes sense I guess, probably not a good idea to regress the
full time performance for the sake of this use case. Was just a query
really!
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git 2.7.0 gitignore behaviour regression

[Jeff King]

[+cc Carlos and Shawn for libgit2/JGit talk]

On Wed, Jan 06, 2016 at 10:58:37AM -0800, Junio C Hamano wrote:

> On Wed, Jan 6, 2016 at 2:03 AM, Duy Nguyen  wrote:
> >
> > Yeah.. it looks like libgit2's gitignore support was written new, not
> > imported from C Git, so behavior differences (especially in corner
> > cases) and even lack of some feature ("**" support comes to mind). For
> > isolated features like gitignore, perhaps we can have an option to
> > replace C Git code with libgit2 and therefore can test libgit2 against
> > C Git test suite. It could be a good start for libgit2 to invade C
> > Git. Not sure if anybody's interested in doing it though.

Yeah, libgit2 is in the difficult position of trying to hit a moving
target. There's a good chance that it _was_ the same as git's behavior
when it was written. :)

JGit is in the same boat, and I wouldn't be surprised if they don't
handle "**" either (I didn't check). Note that git inherited that
feature (and probably some others) by importing a GPLv2 version of
wildmatch. That certainly isn't an option for JGit (because it's not
pure Java), and probably not for libgit2 (which would need the wildmatch
authors to grant the linking exception).

> Yup, an area that is reasonably isolated from the remainder of the system like
> this might be a good starting point. But I suspect that the invasion needs to
> happen in the opposite direction in this particular case before it happens.
> That is, if libgit2's implementation does not behave like how we do, it needs 
> to
> be fixed, possibly by discarding what they did and instead importing code from
> us. After the behaviour of libgit2 is fixed, we can talk about the
> invasion in the
> opposite direction.

Unfortunately, "importing code from us" is not so easy. :(

They'll either need to contact the wildmatch authors, or rewrite
wildmatch from scratch.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git 2.7.0 gitignore behaviour regression

[Duy Nguyen]

On Thu, Jan 7, 2016 at 9:04 AM, Jeff King  wrote:
>> Yup, an area that is reasonably isolated from the remainder of the system 
>> like
>> this might be a good starting point. But I suspect that the invasion needs to
>> happen in the opposite direction in this particular case before it happens.
>> That is, if libgit2's implementation does not behave like how we do, it 
>> needs to
>> be fixed, possibly by discarding what they did and instead importing code 
>> from
>> us. After the behaviour of libgit2 is fixed, we can talk about the
>> invasion in the
>> opposite direction.
>
> Unfortunately, "importing code from us" is not so easy. :(
>
> They'll either need to contact the wildmatch authors, or rewrite
> wildmatch from scratch.

wildmatch author relicensed it on request before [1] so he might do it
again (hard to say). I'm not sure if there are other authors though.
But even with wildmatch good to go, there still a lot of work to do.

[1] http://thread.gmane.org/gmane.comp.version-control.git/264312/focus=264328
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Picking up old threads/patches

[Stephen & Linda Smith]

> If Will isn't interested in finishing these two patches I will pick them 
> up [ ($gmane/271213), ($gmane/272180) ]
> 
> After that I will check look at some of the others for which you've 
> asked for help.

I started work on both of this this evening.   Since I do not have the 
original emails I don't have the Message ID's which would make it 
to use with the git send-email command.   Do either of you have the 
message ID's?


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Picking up old threads/patches

[Stephen & Linda Smith]

> If Will isn't interested in finishing these two patches I will pick them 
> up [ ($gmane/271213), ($gmane/272180) ]
> 
> After that I will check look at some of the others for which you've 
> asked for help.
 
I started work on both of these rerolls this evening.   Since I do not have the 
original emails I don't have the Message ID's which  would allow me 
to add to the threads with the git send-email command.   Do either of you have 
the 
message ID's?

 

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[BUG?] worktree setup broken in subdirs with git alias

[Michael J Gruber]

Hi there,

sorry I can't dig deeper now, but the worktree code from next seems to
get confused now as soon as you cd to a subdir of a worktree (other than
the main worktree) and use an alias:

git help ss
`git ss' is aliased to `status -s -b'
[mjg@skimbleshanks Biomath-WS15 (exam $)]✓ git status -s -b
## exam
[mjg@skimbleshanks Biomath-WS15 (exam $)]✓ git ss
fatal: internal error: work tree has already been set
Current worktree: /home/mjg/Teaching/LUK
New worktree: /home/mjg/Teaching/LUK/Biomath-WS15

This is inside the subdir "Biomath-WS15" of the worktree residing in
".../LUK".

It wasn't like that last year ;)

Something about setting GIT_DIR and the like in the environment must
have changed (for aliases), badly interacting with the worktree code.

Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Initial git clone behaviour

[Eric Curtin]

Hi Guys,

I am not a git developer or a git expert but just a change I would love
to see.

When I clone a really large repository (like the linux kernel for
example) initially on a brand new machine, it can take quite a while
before I can start working with the code.

Often I do a standard git clone:

git clone (name of repo)

Followed by a depth=1 clone in parallel, so I can get building and
working with the code asap:

git clone --depth=1 (name of repo)

Could we change the default behavior of git so that we initially get
all the current files quickly so that we can start working them and
then getting the rest of the data? At least a user could get to work
quicker this way. Any disadvantages of this approach? Maybe I am not
the first to suggest something like this.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC v2 1/3] refs: allow log-only updates

[David Turner]

The refs infrastructure learns about log-only ref updates, which only
update the reflog.  Later, we will use this to separate symbolic
reference resolution from ref updating.

Signed-off-by: David Turner 
---
 refs/files-backend.c | 15 ++-
 refs/refs-internal.h |  2 ++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 176cf65..0800a57 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2821,7 +2821,7 @@ static int commit_ref_update(struct ref_lock *lock,
}
}
}
-   if (commit_ref(lock)) {
+   if (!(flags & REF_LOG_ONLY) && commit_ref(lock)) {
error("Couldn't set %s", lock->ref_name);
unlock_ref(lock);
return -1;
@@ -3175,7 +3175,8 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
goto cleanup;
}
if ((update->flags & REF_HAVE_NEW) &&
-   !(update->flags & REF_DELETING)) {
+   !(update->flags & REF_DELETING) &&
+   !(update->flags & REF_LOG_ONLY)) {
int overwriting_symref = ((update->type & REF_ISSYMREF) 
&&
  (update->flags & 
REF_NODEREF));
 
@@ -3205,7 +3206,9 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
update->flags |= REF_NEEDS_COMMIT;
}
}
-   if (!(update->flags & REF_NEEDS_COMMIT)) {
+
+   if (!(update->flags & REF_LOG_ONLY) &&
+   !(update->flags & REF_NEEDS_COMMIT)) {
/*
 * We didn't have to write anything to the lockfile.
 * Close it to free up the file descriptor:
@@ -3222,7 +3225,8 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
for (i = 0; i < n; i++) {
struct ref_update *update = updates[i];
 
-   if (update->flags & REF_NEEDS_COMMIT) {
+   if (update->flags & REF_NEEDS_COMMIT ||
+   update->flags & REF_LOG_ONLY) {
if (commit_ref_update(update->backend_data,
  update->new_sha1, update->msg,
  update->flags, err)) {
@@ -3242,7 +3246,8 @@ static int files_transaction_commit(struct 
ref_transaction *transaction,
struct ref_update *update = updates[i];
struct ref_lock *lock = update->backend_data;
 
-   if (update->flags & REF_DELETING) {
+   if (update->flags & REF_DELETING &&
+   !(update->flags & REF_LOG_ONLY)) {
if (delete_ref_loose(lock, update->type, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto cleanup;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 21ac680..649ba63 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -42,6 +42,8 @@
  * value to ref_update::flags
  */
 
+#define REF_LOG_ONLY 0x80
+
 /* Include broken references in a do_for_each_ref*() iteration */
 #define DO_FOR_EACH_INCLUDE_BROKEN 0x01
 
-- 
2.4.2.749.g730654d-twtrsrc

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH/RFC v2 3/3] refs: always handle non-normal refs in files backend

[David Turner]

Always handle non-normal (per-worktree or pseudo) refs in the files
backend instead of alternate backends.

Sometimes a ref transaction will update both a per-worktree ref and a
normal ref.  For instance, an ordinary commit might update
refs/heads/master and HEAD (or at least HEAD's reflog).

Updates to normal refs continue to go through the chosen backend.

Updates to non-normal refs are moved to a separate files backend
transaction.

Signed-off-by: David Turner 
---
 refs.c   | 82 ++--
 refs/refs-internal.h |  2 ++
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 2496b91..67b0280 100644
--- a/refs.c
+++ b/refs.c
@@ -9,6 +9,11 @@
 #include "object.h"
 #include "tag.h"
 
+static const char split_transaction_fail_warning[] =
+   "A ref transaction was split across two refs backends.  Part of the "
+   "transaction succeeded, but then the update to the per-worktree refs "
+   "failed.  Your repository may be in an inconsistent state.";
+
 /*
  * We always have a files backend and it is the default.
  */
@@ -781,6 +786,13 @@ void ref_transaction_free(struct ref_transaction 
*transaction)
free(transaction);
 }
 
+static void add_update_obj(struct ref_transaction *transaction,
+  struct ref_update *update)
+{
+   ALLOC_GROW(transaction->updates, transaction->nr + 1, 
transaction->alloc);
+   transaction->updates[transaction->nr++] = update;
+}
+
 static struct ref_update *add_update(struct ref_transaction *transaction,
 const char *refname)
 {
@@ -788,8 +800,7 @@ static struct ref_update *add_update(struct ref_transaction 
*transaction,
struct ref_update *update = xcalloc(1, sizeof(*update) + len);
 
memcpy((char *)update->refname, refname, len); /* includes NUL */
-   ALLOC_GROW(transaction->updates, transaction->nr + 1, 
transaction->alloc);
-   transaction->updates[transaction->nr++] = update;
+   add_update_obj(transaction, update);
return update;
 }
 
@@ -1192,11 +1203,39 @@ static int dereference_symrefs(struct ref_transaction 
*transaction,
return 0;
 }
 
+/*
+ * Move all non-normal ref updates into a specially-created
+ * files-backend transaction
+ */
+static int move_abnormal_ref_updates(struct ref_transaction *transaction,
+struct ref_transaction *files_transaction,
+struct strbuf *err)
+{
+   int i;
+
+   for (i = 0; i < transaction->nr; i++) {
+   int last;
+   struct ref_update *update = transaction->updates[i];
+
+   if (ref_type(update->refname) == REF_TYPE_NORMAL)
+   continue;
+
+   last = --transaction->nr;
+   transaction->updates[i] = transaction->updates[last];
+   files_transaction->updates[files_transaction->nr++] = update;
+   }
+
+   return 0;
+}
+
 int ref_transaction_commit(struct ref_transaction *transaction,
   struct strbuf *err)
 {
int ret = -1;
struct string_list affected_refnames = STRING_LIST_INIT_NODUP;
+   struct string_list files_affected_refnames = STRING_LIST_INIT_NODUP;
+   struct string_list_item *item;
+   struct ref_transaction *files_transaction = NULL;
 
assert(err);
 
@@ -1212,6 +1251,26 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
if (ret)
goto done;
 
+   if (the_refs_backend != _be_files) {
+   files_transaction = ref_transaction_begin(err);
+   if (!files_transaction)
+   goto done;
+
+   ret = move_abnormal_ref_updates(transaction, files_transaction,
+   err);
+   if (ret)
+   goto done;
+
+   /* files backend commit */
+   if (get_affected_refnames(files_transaction,
+_affected_refnames,
+err)) {
+   ret = TRANSACTION_GENERIC_ERROR;
+   goto done;
+   }
+   }
+
+   /* main backend commit */
if (get_affected_refnames(transaction, _refnames, err)) {
ret = TRANSACTION_GENERIC_ERROR;
goto done;
@@ -1219,8 +1278,24 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
 
ret = the_refs_backend->transaction_commit(transaction,
   _refnames, err);
+   if (ret)
+   goto done;
+
+   if (files_transaction) {
+   ret = refs_be_files.transaction_commit(files_transaction,
+  _affected_refnames,
+ 

Re: [PATCH 2/2] contrib/git-candidate: Add README

[Sebastian Schuberth]

On 10.11.2015 13:56, Richard Ipsum wrote:


+Existing tools such as Github's pull-requests and Gerrit are already
+in wide use, why bother with something new?
+
+We are concerned that whilst git is a distributed version control
+system the systems used to store comments and reviews for content
+under version control are usually centralised,


I think it's a bit unjust to unconditionally mention Gerrit in this 
context as you seem to imply that Gerrit does not store *any* review 
data in Git.


Even without Dave's upcoming notedb, Gerrit already stores refs/changes 
in Git, and with the reviewnotes plugin [1] also the outcome of a review 
in refs/notes/review.


[1] 
https://gerrit.googlesource.com/plugins/reviewnotes/+/refs/heads/master/src/main/resources/Documentation/refs-notes-review.md


--
Sebastian Schuberth

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 00/15] ref-filter: use parsing functions

[Eric Sunshine]

On Tue, Jan 5, 2016 at 3:02 AM, Karthik Nayak  wrote:
> Eric suggested that I make match_atom_name() not return a value [0]. I
> haven't done that as we use match_atom_name() in [14/15] for matching
> 'subject' and 'body' in contents_atom_parser() and although Eric
> suggested I use strcmp() instead, this would not work as we need to
> check for derefernced 'subject' and 'body' atoms.
> [0]: http://article.gmane.org/gmane.comp.version-control.git/282701

I don't understand the difficulty. It should be easy to manually skip
the 'deref' for this one particular case:

const char *name = atom->name;
if (*name == '*')
name++;

Which would allow this unnecessarily complicated code from patch 14/15:

if (match_atom_name(atom->name, "subject", ) && !buf) {
...
return;
} else if (match_atom_name(atom->name, "body", ) && !buf) {
...
return;
} if (!match_atom_name(atom->name, "contents", ))
die("BUG: parsing non-'contents'");

to be simplified to the more easily understood form suggested during
review[1] of v2:

if (!strcmp(name, "subject")) {
...
return;
} else if (!strcmp(name, "body")) {
...
return;
} else if (!match_atom_name(name,"contents", ))
die("BUG: expected 'contents' or 'contents:'");

You could also just use (!strcmp("body") || !strcmp("*body")) rather
than skipping "*" manually, but the repetition makes that a bit
noisier and uglier.

[1]: http://article.gmane.org/gmane.comp.version-control.git/282645
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html