Re: [PATCH] list: avoid incompatibility with *BSD sys/queue.h

[Eric Sunshine]

On Sat, Jul 16, 2016 at 8:25 PM, Eric Wong  wrote:
> Eric Wong  wrote:
>> I also wonder where we use sys/queue.h, since I use
>> LIST_HEAD from ccan/list/list.h in a different project
>> without conflicts...
>
> Still wondering... Checking sys/mman.h in an old FreeBSD source
> tree I had lying around reveals "#include " is
> guarded by "#if defined(_KERNEL)", so it mman.h wouldn't pull
> it in for userspace builds...

It's pulled in like this:

git-compat-util.h ->
sys/sysctl.h ->
sys/ucred.h ->
sys/queue.h

Very reminiscent of [1].

[1]: 
http://git.661346.n2.nabble.com/PATCH-ewah-bitmap-silence-warning-about-MASK-macro-redefinition-td7632287.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

proposal: allow git clone for http-hosted bundles

[mappu]

Hi list,

Right now it's possible to git clone a repository over http, and git 
clone a bundle from the local filesystem, but it's not possible to git 
clone a bundle hosted on http.


Would it be possible to allow this in the future? Hopefully it's only a 
minor refactor in `builtin/clone.c`.


Regards

mappu


(Back story: I'm stuck with a git frontend that only ever calls `git 
clone ${target}` - that's Golang's `go get` - but bundles are a bit 
better fit for my request patterns than raw repositories).


--
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

format-patch with pager.format-patch=true gets very confused

[Josh Triplett]

git-config(1) documents the ability to enable or disable the pager (or
set a command-specific pager) for any command by setting
pager.=true.  For most commands, this seems to work as expected.
However, setting pager.format-patch=true (or setting it to any specific
pager) breaks badly: the pager spawns, with no output in it, and the
pager doesn't respond to keystrokes (which makes it difficult to quit).

I think this may occur because format-patch's "reopen_stdout" interacts
badly with the pager.

I think it makes sense for "format-patch --stdout" to respect
pager.format-patch, but for format-patch *without* stdout to ignore
pager.* and *never* spawn a pager, given that its only output (the list
of patch files) goes to "realstdout".

- Josh Triplett
--
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] list: avoid incompatibility with *BSD sys/queue.h

[Eric Wong]

Eric Wong  wrote:
> Lars Schneider  wrote:
> > It looks like as if this topic breaks the OS X build because 
> > it defines LIST_HEAD. LIST_HEAD is already defined in 
> > /usr/include/sys/queue.h. 
> 
> Oops, I suppose GIT_LIST_HEAD is an acceptable name?
> (looks a bit like a refname, though...).
> 
> Or maybe CDS_LIST_HEAD (since I originally took it from the cds
> namespace under urcu)

Naming things is hard; I think it's better to just undef an
existing LIST_HEAD, instead, since it's unlikely we'd ever use
sys/queue.h from *BSD.  (sys/queue.h is branchier, and IMHO
sys/queue.h macros are uglier than list_entry (container_of))

> I also wonder where we use sys/queue.h, since I use
> LIST_HEAD from ccan/list/list.h in a different project
> without conflicts...

Still wondering... Checking sys/mman.h in an old FreeBSD source
tree I had lying around reveals "#include " is
guarded by "#if defined(_KERNEL)", so it mman.h wouldn't pull
it in for userspace builds...

-8<--
Subject: [PATCH] list: avoid incompatibility with *BSD sys/queue.h

Somehow, the OS X build pulls in sys/queue.h and causes
conflicts with the LIST_HEAD macro, here.

ref: http://mid.gmane.org/fb76544f-16f7-45ca-9649-fd62ee44b...@gmail.com

Reported-by: Lars Schneider 
Signed-off-by: Eric Wong 
---
 list.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/list.h b/list.h
index f65edce..a226a87 100644
--- a/list.h
+++ b/list.h
@@ -36,6 +36,8 @@ struct list_head {
struct list_head *next, *prev;
 };
 
+/* avoid conflicts with BSD-only sys/queue.h */
+#undef LIST_HEAD
 /* Define a variable with the head and tail of the list. */
 #define LIST_HEAD(name) \
struct list_head name = { &(name), &(name) }
-- 
EW
--
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: What's cooking in git.git (Jul 2016, #05; Wed, 13)

[Lars Schneider]

> On 16 Jul 2016, at 23:04, Eric Wong  wrote:
> 
> Lars Schneider  wrote:
>>> On 13 Jul 2016, at 18:56, Junio C Hamano  wrote:
>>> * ew/http-walker (2016-07-12) 3 commits
>>> - http-walker: reduce O(n) ops with doubly-linked list
>>> - http: avoid disconnecting on 404s for loose objects
>>> - http-walker: remove unused parameter from fetch_object
>>> 
>>> Optimize dumb http transport on the client side.
>> 
>> It looks like as if this topic breaks the OS X build because 
>> it defines LIST_HEAD. LIST_HEAD is already defined in 
>> /usr/include/sys/queue.h. 
> 
> Oops, I suppose GIT_LIST_HEAD is an acceptable name?
> (looks a bit like a refname, though...).
> 
> Or maybe CDS_LIST_HEAD (since I originally took it from the cds
> namespace under urcu)
> 
> I also wonder where we use sys/queue.h, since I use
> LIST_HEAD from ccan/list/list.h in a different project
> without conflicts...
> 
>> See: https://travis-ci.org/git/git/jobs/145121761
> 
> Is there a way to get plain-text logs?  No JavaScript, here.
> Thanks.

Here you go:
https://api.travis-ci.org/jobs/145121761/log.txt?deansi=true

- Lars


--
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 and SHA-1 security (again)

[brian m. carlson]

On Sat, Jul 16, 2016 at 11:46:06PM +0200, Herczeg Zsolt wrote:
> Dear Brian,
> 
> Thank you for your response. It very good to hear that changing the
> hash is on the git project's list. I haven't found any official
> communication on that topic since 2006.

There's been some recent discussion on the list about it.  It is less on
the Git project's list and more on my personal list.  It's my hope that
Junio and other contributors will decide to accept my patches when they
are ready.  Also, the plan is to keep SHA-1 available, probably as the
default, for backwards compatibility.

> I'll look into the contributions guide and the source codes, to check
> if I can contribute to this transition. If you have any documentation
> or other related info, please point me towards it.

The major work at this point is turning instances of unsigned char [20]
into struct object_id, as well as converting hardcoded 20 and 40 (and
derivative values) to GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ.  This work
allows us to make as little code as possible know about the size of the
hash, as well as generally being easier to maintain.

You can look at the bc/cocci branch which was recently merged into next.
(It doesn't exist independently outside of next, so you'll have to
search through the history).  That work is what in my branches is called
object-id-part4.  I'm currently working on getting to the point of
converting get_tree_entry to use struct object_id, which is what will
become my object-id-part5.

I recommend if you're planning on doing some of this work that you try
to avoid areas which are under work by other developers, especially the
refs code, which is undergoing massive changes.  Other people will
appreciate it.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Re: Git and SHA-1 security (again)

[Herczeg Zsolt]

Dear Brian,

Thank you for your response. It very good to hear that changing the
hash is on the git project's list. I haven't found any official
communication on that topic since 2006.
I'll look into the contributions guide and the source codes, to check
if I can contribute to this transition. If you have any documentation
or other related info, please point me towards it.

Thanks,
Zsolt Herczeg


2016-07-16 22:13 GMT+02:00 brian m. carlson :
> On Sat, Jul 16, 2016 at 03:48:49PM +0200, Herczeg Zsolt wrote:
>> But - and that's the main idea i'm writing here - changing the storage
>> keys does not mean you should drop your old hashes out. If you change
>> the git data structure in a way, that it can keep multiple hashes for
>> the same "link" in each objects (trees, commits, etc) you can keep the
>> old ones right next to the new one. If you want to look up the
>> referenced object, you must use the newest hash - which is the key.
>> But if you want to verify some old hash, it's still possible! Just
>> look up the objects by the new key, remove all the newer generation
>> keys, and verify the old hash on that.
>>
>> A storage structure like this would allow a very great flexibility:
>>  - You can change your hash algorithm in the future. If SHA-256
>> becomes broken, it's not a problem. Just re-hash the storage, and
>> append the new hashes the git objects.
>>  - You can still verify your old hashes after a hash change - removing
>> the new hashes from the objects before hashing should give you back
>> the old objects, thus giving you the same hash as before.
>>  - That makes possible for signed tags, and commits to keep their
>> validity after hash change! With a clever-enough new format, you can
>> even keep the validity of current hashes and signs. (To be able to do
>> that, you should be able to calculate back the current format from the
>> new format.)
>>
>> Moving git forward to a format like this would solve the weak-key
>> problem in git forever. You would be able to configure your key algo
>> on a per repository basis, you - and git - can do the daily work on
>> the newest hashes, while still carrying the old hashes and signatures,
>> in case you ever want to verify them. That would allow repositories to
>> gracefully change hashes in case they need to, and to only
>> compatibility limitation is that you must use a new enough git to
>> understand the new storage format.
>>
>> What are your thoughts on this approach? Will git ever reach a release
>> with exchangeable hash algorithm? Or should someone look for
>> alternatives if there's a need for cryptographic security?
>
> I'm working on adding new hash algorithm support in Git.  However, it
> requires a significant refactor of the code base.  My current plan is
> not to implement side-by-side data, for a couple reasons.
>
> One is that it requires significantly more work to implement and
> complicates the code.  It's also incompatible with all the refactoring
> I've done already.
>
> The second is that it requires that Git have the ability to store
> multiple hashes at once, which is very expensive in terms of memory.
> Moving from a 160-bit hash to a 256-bit hash (my current plan is
> SHA3-256) requires 1.6× the memory.  Storing both requires 2.6× the
> memory.  If you add a third hash, it requires even more.  Memory is
> often a constraint with using Git.
>
> The current plan is to use git-fast-import and git-fast-export to handle
> that conversion process, and then maybe provide wrappers to make it more
> transparent.
>
> Currently the process of the refactor is ongoing, but it is a free time
> activity for me.
>
> If you'd like to follow the progress roughly, you can do so by checking
> the output of the following commands:
>
>   git grep 'unsigned char.*20' | wc -l
>   git grep 'struct object_id' | wc -l
>
> You are also welcome to contribute, of course.
> --
> brian m. carlson / brian with sandals: Houston, Texas, US
> +1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
> OpenPGP: https://keybase.io/bk2204
--
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: Question: Getting 'git diff' to generate /usr/bin/diff output

[Perry Hutchison]

n...@dad.org wrote:

> I am trying to learn how to use git, and am having difficulty
> using 'git diff'.
>
> I can't deal with its output very well. What I would really like
> to do is apply /usr/lib/diff.
  ^^^

For starters, when using the full pathname, you'll need to spell it
correctly :)

> Would some kind soul be willing to tell me if there is a way to
> do that, short of making a backup copy of the relevant file, and
> then doing 'git checkout'. Maybe the '--ext-diff' argument to
> 'git diff' will do that, but I can't figure out how to use it.
> ...
> git version: 2.5.1.454.g1616360

'git cat' would be simpler than making a backup and using 'git checkout',
but for this use case there's a better way even than that.

Google is your friend when you can't figure something out from (or
find the right part of) a program's own documentation.  A search for

git "external diff program"

(including the quotes) found several references.

One possible complication:  It's likely that the command parameters
which git passes to an external diff program are not exactly what
/usr/bin/diff requires.  The solution to that sort of problem is to
write a very short executable script that rearranges the parameters
as needed, and specify that script (instead of /usr/bin/diff itself)
as the external diff program.
--
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: What's cooking in git.git (Jul 2016, #05; Wed, 13)

[Eric Wong]

Lars Schneider  wrote:
> > On 13 Jul 2016, at 18:56, Junio C Hamano  wrote:
> > * ew/http-walker (2016-07-12) 3 commits
> > - http-walker: reduce O(n) ops with doubly-linked list
> > - http: avoid disconnecting on 404s for loose objects
> > - http-walker: remove unused parameter from fetch_object
> > 
> > Optimize dumb http transport on the client side.
> 
> It looks like as if this topic breaks the OS X build because 
> it defines LIST_HEAD. LIST_HEAD is already defined in 
> /usr/include/sys/queue.h. 

Oops, I suppose GIT_LIST_HEAD is an acceptable name?
(looks a bit like a refname, though...).

Or maybe CDS_LIST_HEAD (since I originally took it from the cds
namespace under urcu)

I also wonder where we use sys/queue.h, since I use
LIST_HEAD from ccan/list/list.h in a different project
without conflicts...

> See: https://travis-ci.org/git/git/jobs/145121761

Is there a way to get plain-text logs?  No JavaScript, here.
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: What's cooking in git.git (Jul 2016, #05; Wed, 13)

[Lars Schneider]

> On 13 Jul 2016, at 18:56, Junio C Hamano  wrote:
> 
...
> 
> * ew/http-walker (2016-07-12) 3 commits
> - http-walker: reduce O(n) ops with doubly-linked list
> - http: avoid disconnecting on 404s for loose objects
> - http-walker: remove unused parameter from fetch_object
> 
> Optimize dumb http transport on the client side.

It looks like as if this topic breaks the OS X build because 
it defines LIST_HEAD. LIST_HEAD is already defined in 
/usr/include/sys/queue.h. 
See: https://travis-ci.org/git/git/jobs/145121761

- Lars
--
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 Funds Beneficiary,

[Sarah Michael]

Dear Funds Beneficiary,

I'm happy to inform you that your email address has been selected  
among those that have compensation payment of ($2.5 Million USD), The  
INTERNATIONAL MONETARY FUND contacted us for the release of your money  
a couple of hours ago due to your allocated security code. We are  
unable to complete a transfer directed at an email address, so we  
require some more information in order to complete this transfer, But  
for security purpose the money has been loaded in a prepaid Visa Debit  
Card on your behalf, You Withdraw cash at ATMs and make purchases  
anywhere debit cards are accepted, including retail stores,grocery
stores, restaurants and pharmacies, You are advised that a maximum  
withdrawal value of $10,000.00 USD is permitted daily.


Therefore arrangements has been perfectly concluded to effect the  
delivery of your Visa Debit Card via DHL-Express Delivery, Courier &  
Shipping, they offer's the fastest courier services around the world  
daily. DHL-Express is your 24 hours solution for the best courier  
service, as soon as youre-confirm the information's below.


1) Your full name:
2) Mobile number:
3) Delivery address:
4) Zip code:

I am waiting to hear from you soon,

Best Regards,
Sarah 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

Re: Git and SHA-1 security (again)

[brian m. carlson]

On Sat, Jul 16, 2016 at 03:48:49PM +0200, Herczeg Zsolt wrote:
> But - and that's the main idea i'm writing here - changing the storage
> keys does not mean you should drop your old hashes out. If you change
> the git data structure in a way, that it can keep multiple hashes for
> the same "link" in each objects (trees, commits, etc) you can keep the
> old ones right next to the new one. If you want to look up the
> referenced object, you must use the newest hash - which is the key.
> But if you want to verify some old hash, it's still possible! Just
> look up the objects by the new key, remove all the newer generation
> keys, and verify the old hash on that.
> 
> A storage structure like this would allow a very great flexibility:
>  - You can change your hash algorithm in the future. If SHA-256
> becomes broken, it's not a problem. Just re-hash the storage, and
> append the new hashes the git objects.
>  - You can still verify your old hashes after a hash change - removing
> the new hashes from the objects before hashing should give you back
> the old objects, thus giving you the same hash as before.
>  - That makes possible for signed tags, and commits to keep their
> validity after hash change! With a clever-enough new format, you can
> even keep the validity of current hashes and signs. (To be able to do
> that, you should be able to calculate back the current format from the
> new format.)
> 
> Moving git forward to a format like this would solve the weak-key
> problem in git forever. You would be able to configure your key algo
> on a per repository basis, you - and git - can do the daily work on
> the newest hashes, while still carrying the old hashes and signatures,
> in case you ever want to verify them. That would allow repositories to
> gracefully change hashes in case they need to, and to only
> compatibility limitation is that you must use a new enough git to
> understand the new storage format.
> 
> What are your thoughts on this approach? Will git ever reach a release
> with exchangeable hash algorithm? Or should someone look for
> alternatives if there's a need for cryptographic security?

I'm working on adding new hash algorithm support in Git.  However, it
requires a significant refactor of the code base.  My current plan is
not to implement side-by-side data, for a couple reasons.

One is that it requires significantly more work to implement and
complicates the code.  It's also incompatible with all the refactoring
I've done already.

The second is that it requires that Git have the ability to store
multiple hashes at once, which is very expensive in terms of memory.
Moving from a 160-bit hash to a 256-bit hash (my current plan is
SHA3-256) requires 1.6× the memory.  Storing both requires 2.6× the
memory.  If you add a third hash, it requires even more.  Memory is
often a constraint with using Git.

The current plan is to use git-fast-import and git-fast-export to handle
that conversion process, and then maybe provide wrappers to make it more
transparent.

Currently the process of the refactor is ongoing, but it is a free time
activity for me.

If you'd like to follow the progress roughly, you can do so by checking
the output of the following commands:

  git grep 'unsigned char.*20' | wc -l
  git grep 'struct object_id' | wc -l

You are also welcome to contribute, of course.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

Question: Getting 'git diff' to generate /usr/bin/diff output

[norm]

I am trying to learn how to use git, and am having difficulty using 'git diff'.

I can't deal with its output very well. What I would really like to do is apply
/usr/lib/diff. Would some kind soul be willing to tell me if there is a way to
do that, short of making a backup copy of the relevant file, and then doing
'git checkout'. Maybe the '--ext-diff' argument to 'git diff' will do that,
but I can't figure out how to use it.

Thank you, most kindly.

Context:

Operating System: Red Hat Enterprise Linux Workstation release 6.8

git version: 2.5.1.454.g1616360

 Norman Shapiro
--
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] config: add conditional include

[Jeff King]

On Sat, Jul 16, 2016 at 06:36:03PM +0200, Johannes Schindelin wrote:

> > On Sat, Jul 16, 2016 at 03:30:45PM +0200, Johannes Schindelin wrote:
> > 
> > > As an alternative solution to your problem, you could of course avoid all
> > > conditional includes. Simply by adding the include.path settings
> > > explicitly to the configs that require them. Now, that would make 
> > > reasoning
> > > and trouble-shooting simple, wouldn't it?
> > > 
> > > And the most beautiful aspect of it: no patch needed.
> > 
> > And you can just "cat" the included files directly into your
> > .git/config. We don't even need include.path. Or ~/.gitconfig, for that
> > matter. But sometimes dynamic things are convenient.
> 
> Well, apparently you are not convinced by my argument. I thought it was
> pretty sound, but if you disagree, it cannot have been...

Heh. Don't get me wrong, I do think there's room for digging ourselves a
deep hole with conditional includes, because anything dynamic opens up a
question of _when_ it is evaluated, and in what context. So any
condition should be something that we would consider static through the
whole run of the program. Looking at the "gitdir" is right on the
borderline of that, but I think it's OK, because we already have to
invalidate any cached config when we setup the gitdir, because
"$GIT_DIR/config" will have become a new source.

So I agree it's something we need to be thoughtful about, but I think
this particular instance is useful and probably not going to bite us.

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

Re: [PATCH v4] config: add conditional include

[Johannes Schindelin]

Hi Peff,

On Sat, 16 Jul 2016, Jeff King wrote:

> On Sat, Jul 16, 2016 at 03:30:45PM +0200, Johannes Schindelin wrote:
> 
> > As an alternative solution to your problem, you could of course avoid all
> > conditional includes. Simply by adding the include.path settings
> > explicitly to the configs that require them. Now, that would make reasoning
> > and trouble-shooting simple, wouldn't it?
> > 
> > And the most beautiful aspect of it: no patch needed.
> 
> And you can just "cat" the included files directly into your
> .git/config. We don't even need include.path. Or ~/.gitconfig, for that
> matter. But sometimes dynamic things are convenient.

Well, apparently you are not convinced by my argument. I thought it was
pretty sound, but if you disagree, it cannot have been...

So I'll shut up ;-)

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: [PATCH v4] config: add conditional include

[Jeff King]

On Sat, Jul 16, 2016 at 03:30:45PM +0200, Johannes Schindelin wrote:

> > >> + ; include if $GIT_DIR is /path/to/foo/.git
> > >> + [include "gitdir:/path/to/foo/.git"]
> > >> + path = /path/to/foo.inc
> > >
> > > I find this way to specify a conditional unintuitive. Reading
> > > "gitdir:/path/to/foo/.git" suggests to me that the Git dir is *re-set*,
> > > not that this is a condition.
> > 
> > Well.. to me it's no different than [remote "foo"] to apply stuff to "foo".
> 
> Except that "include" is an imperative and "remote" is not.

In the very original version of config includes, I had planned out:

  [include-if "...some condition..."]
  path = ...

Later, since "[include ...]" had no other meaning, I think it got
shortened in discussion. But it would be easy to accept include-if (or
even accept either, for maximum confusion :) ).

> Quite frankly, this conditional business scares me. If you introduce it
> for [include], users will want it for every config setting. And the
> current syntax is just not up to, say, making user.name conditional on
> anything.

They already have it for every config setting with this. The reason to
add it to [include] and not as a general syntax is that you can put
user.name into your included file, and then conditionally include it.

That is not as nice as "if this then that" in a single file, but it is
backwards compatible with the existing syntax, and is probably fine in
practice. Each included file becomes a "profile" of multiple settings
that you apply.

> As an alternative solution to your problem, you could of course avoid all
> conditional includes. Simply by adding the include.path settings
> explicitly to the configs that require them. Now, that would make reasoning
> and trouble-shooting simple, wouldn't it?
> 
> And the most beautiful aspect of it: no patch needed.

And you can just "cat" the included files directly into your
.git/config. We don't even need include.path. Or ~/.gitconfig, for that
matter. But sometimes dynamic things are convenient.

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

Re: [PATCH v4] config: add conditional include

[Duy Nguyen]

On Sat, Jul 16, 2016 at 3:30 PM, Johannes Schindelin
 wrote:
> As an alternative solution to your problem, you could of course avoid all
> conditional includes. Simply by adding the include.path settings
> explicitly to the configs that require them. Now, that would make reasoning
> and trouble-shooting simple, wouldn't it?

I can't. Repos can be created and destroyed often (it's in the
process), and there are many of them. Using a wrong identity (among
other incorrect config settings) is a serious problem and I cannot
guarantee myself that I will never make a mistake, forgetting to
include stuff on new clones.

>> > I still fail to see what use case this would benefit,
>>
>> It allows you to group repos together. The first mail that started all
>> this [1] is one of the use cases: you may want to use one identity in
>> a group of repos, and another identity in some other. I want some
>> more, to use a private key one some repos and another private key on
>> some other. Some more settings may be shareable this way, like coding
>> style-related (trailing spaces or not...)
>>
>> [1] http://thread.gmane.org/gmane.comp.version-control.git/273811
>>
>> > and I already start to suspect that the change would open a can of worms 
>> > that might not be desired.
>>
>> You can choose not to use it, I guess.
>
> Sadly, as the maintainer I am unable to share in that luxury of yours.

I need this. And I post it because people may need it too. But if it's
a bad thing, I guess I'll keep this patch on my tree then.
-- 
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

Git and SHA-1 security (again)

[Herczeg Zsolt]

Dear List Members, Git Developers,

I would like to discuss an old topic from 2006. I understand it was
already discussed. The only reason i'm sending this e-mail is to talk
about a possible solution which didn't show up on this list before.

I think we all understand that SHA-1 is broken. It still works perfect
as a storage key, but it's not cryptographically secure anymore. Git
is not moving away from SHA-1 because it would break too many
projects, and cryptographic security is not needed but git if you have
your own repository.

However I would like to show some big problems caused by SHA-1:
 - Git signed tags and signed commits are cryptographically insecure,
they're useless at the moment.
 - Git Torrent (https://github.com/cjb/GitTorrent) is also
cryptographically broken, however it would be an awesome experiment.
 - Linus said: "You only need to know the SHA-1 of the top of your
tree, and if you know that, you can trust your tree." That's not true
anymore. You have to trust your computer, you servers, your git
provider in a way that no-one can maliciously modify your data.

I understand that git is perfect for a work flow, where you have your
very own repository and you double-check any commits or diffs you
accepting to it. But that's not everybody's work flow. For example: if
I want to blindly trust my college, I could just include all commits
he signed without review. Currently I can't do that. There are
workarounds of course: signing the e-mail he sends me, or signing the
entire git repository's tarball, etc... But that's not the right way
to do things.

As a final thought on this, I would like to say: Git is a great tool,
but it can be so much better with a safe hash.


I would like to propose a solution for changing git's hash algorithm:
It would be a breaking change, bit I think it can be done pretty
painless. (If you read the discussion back in 2006 the problems of
moving are clear.)

In git, every data has to have one and only one key - so a hybrid hash
is a no-go. That means changing hash algo involves re-hashing every
data in a git repository, but it's not that bad. On a git clone, we
actually re-hash everything to check integrity. Changing all the keys
shouldn't be worth than that.

But - and that's the main idea i'm writing here - changing the storage
keys does not mean you should drop your old hashes out. If you change
the git data structure in a way, that it can keep multiple hashes for
the same "link" in each objects (trees, commits, etc) you can keep the
old ones right next to the new one. If you want to look up the
referenced object, you must use the newest hash - which is the key.
But if you want to verify some old hash, it's still possible! Just
look up the objects by the new key, remove all the newer generation
keys, and verify the old hash on that.

A storage structure like this would allow a very great flexibility:
 - You can change your hash algorithm in the future. If SHA-256
becomes broken, it's not a problem. Just re-hash the storage, and
append the new hashes the git objects.
 - You can still verify your old hashes after a hash change - removing
the new hashes from the objects before hashing should give you back
the old objects, thus giving you the same hash as before.
 - That makes possible for signed tags, and commits to keep their
validity after hash change! With a clever-enough new format, you can
even keep the validity of current hashes and signs. (To be able to do
that, you should be able to calculate back the current format from the
new format.)

Moving git forward to a format like this would solve the weak-key
problem in git forever. You would be able to configure your key algo
on a per repository basis, you - and git - can do the daily work on
the newest hashes, while still carrying the old hashes and signatures,
in case you ever want to verify them. That would allow repositories to
gracefully change hashes in case they need to, and to only
compatibility limitation is that you must use a new enough git to
understand the new storage format.

What are your thoughts on this approach? Will git ever reach a release
with exchangeable hash algorithm? Or should someone look for
alternatives if there's a need for cryptographic security?

Thank you for your time reading this.

References:
SHA-256 discussion in 2006:
http://www.gelato.unsw.edu.au/archives/git/0608/26446.html
Discussion about git signatures in 2014
https://www.mail-archive.com/git%40vger.kernel.org/msg61087.html
Linus's talk on git
https://www.youtube.com/watch?v=4XpnKHJAok8=56m20s

Kind regards,
Zsolt Herczeg
--
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] config: add conditional include

[Johannes Schindelin]

Hi Duy,

On Thu, 14 Jul 2016, Duy Nguyen wrote:

> On Thu, Jul 14, 2016 at 5:53 PM, Johannes Schindelin
>  wrote:
> > Hi Duy,
> >
> > On Thu, 14 Jul 2016, Nguyễn Thái Ngọc Duy wrote:
> >
> >> Helped-by: Jeff King 
> >> Signed-off-by: Nguyễn Thái Ngọc Duy 
> >
> > This commit message is quite a bit short on details.
> 
> Because it's fully documented in config.txt.

Surely there is more information left for the commit message, such as:
what motivated the patch, what alternative solutions were considered, was
any thought given to how this might break down, etc

> > I still fail to see what use case this would benefit,
> 
> It allows you to group repos together. The first mail that started all
> this [1] is one of the use cases: you may want to use one identity in
> a group of repos, and another identity in some other. I want some
> more, to use a private key one some repos and another private key on
> some other. Some more settings may be shareable this way, like coding
> style-related (trailing spaces or not...)
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/273811
> 
> > and I already start to suspect that the change would open a can of worms 
> > that might not be desired.
> 
> You can choose not to use it, I guess.

Sadly, as the maintainer I am unable to share in that luxury of yours.

> From what I see it's nothing super tricky. The config hierarchy we have
> now is: system -> per-user -> repo. This could change this to: system ->
> per-user -> per-directory -> repo. You could create a different
> hierarchy, but with git-config now showing where a config var comes
> from, it's not hard to troubleshoot.

The more indirection you allow, the harder it gets. And that problem is
exacerbated when allowing conditional includes.

> >> + ; include if $GIT_DIR is /path/to/foo/.git
> >> + [include "gitdir:/path/to/foo/.git"]
> >> + path = /path/to/foo.inc
> >
> > I find this way to specify a conditional unintuitive. Reading
> > "gitdir:/path/to/foo/.git" suggests to me that the Git dir is *re-set*,
> > not that this is a condition.
> 
> Well.. to me it's no different than [remote "foo"] to apply stuff to "foo".

Except that "include" is an imperative and "remote" is not.

Quite frankly, this conditional business scares me. If you introduce it
for [include], users will want it for every config setting. And the
current syntax is just not up to, say, making user.name conditional on
anything.

As an alternative solution to your problem, you could of course avoid all
conditional includes. Simply by adding the include.path settings
explicitly to the configs that require them. Now, that would make reasoning
and trouble-shooting simple, wouldn't it?

And the most beautiful aspect of it: no patch needed.

Ciao,
Dscho

Re: [PATCH v3 2/2] t/t8003-blame-corner-cases.sh: Use here documents

[Johannes Schindelin]

Hi Mike,

On Sat, 16 Jul 2016, Mike Hommey wrote:

> Somehow, this test was using:
> 
> {
>   echo A
>   echo B
> } > file
> 
> block to feed file contents. This changes those to the form most common
> in git test scripts:
> 
> cat >file <<-\EOF
> A
> B
> EOF
> 
> Signed-off-by: Mike Hommey 

I reviewed both patches and like them.

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

[ANNOUNCE] Git for Windows 2.9.2

[Johannes Schindelin]

Dear Git users,

It is my pleasure to announce that Git for Windows 2.9.2 is available from:

https://git-for-windows.github.io/

Changes since Git for Windows v2.9.0 (June 14th 2016)

New Features

  ??? Comes with Git 2.9.2 (skipping the Windows release of Git 2.9.1 due
to a regression caught by the automated tests).
  ??? Git Credential Manager was updated to v1.5.0.
  ??? The installer will now refuse to downgrade Git for Windows, unless
the user assures that it is intended.
  ??? MinGit, the portable, non-interactive Git intended for third-party
tools, is now also built as part of Git for Windows' official
versions.

Bug Fixes

  ??? When git bundle create is asked to create an empty bundle, it is
supposed to error out and delete the corrupt bundle file. The
deletion no longer fails due to an unreleased lock file.
  ??? When launching git help , the help.browser config setting
is now respected.
  ??? The title bar in Git for Windows' SDK shows the correct prefix
again.
  ??? We no longer throw an assertion when using the git
credential-store.
  ??? When configuring notepad as commit message editor, UTF-8 messages
are now handled correctly.

Filename | SHA-256
 | ---
Git-2.9.2-64-bit.exe | 
006d971bcbe73cc8d841a100a4eb20d22e135142bf5b0f2120722fd420e166e5
Git-2.9.2-32-bit.exe | 
d8b5d67dc4859a05254f36e61dcc4bfcffd7e6201c940bb94fb2b502c5dd8e7c
PortableGit-2.9.2-64-bit.7z.exe | 
4319caf04f6e9b7bc4916660ce918e1d26cfc1c6ae5d19981d7ccab166117aca
PortableGit-2.9.2-32-bit.7z.exe | 
d023e37c5e54d46900af4879e786ad107611ec9373fa348450fe844ab125a0a1
Git-2.9.2-64-bit.tar.bz2 | 
292d897d54d864d4b819e25a9773d067310d7890014567cd8e44d2028c583102
Git-2.9.2-32-bit.tar.bz2 | 
b530185c69d9ae4758087de04da706d887e35b94a12e9c088324833818a06a46

Ciao,
Johannes

Re: [PATCH 2/1] Verify that --cherry-pick avoids looking at full diffs

[Johannes Schindelin]

Hi Junio,

On Fri, 15 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > The entire point of the previous patch was to make sure that we look at
> > abbreviated patch IDs (using the diff *headers* only, but avoiding
> > to load the blobs into memory and diff them) first, and only look at
> > full patch IDs when the abbreviated patch IDs were not for the
> > --cherry-pick test.
> >
> > Let's make sure that we actually avoid looking at the full patch ID,
> > simply by corrupting an object that is needed for the full patch ID, and
> > then seeing that --cherry-pick still works.
> 
> I think "Avoid looking at" merely is the means to an end, and not
> the goal by itself.  By not looking at them, you hopefully run
> faster.
> 
> So I'd think a more useful addition under t/ would be to t/perf
> somewhere, not "now you can rev-list --cherry-pick even inside a
> corrupt repository, as long as corruption happens to be with blobs
> and not the containing trees".

I agree that we need a perf test.

But we *also* need this test that ensures that we avoid loading blobs into
memory, because that is the solution we do not want to see regress in the
future.

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: [PATCH] git-clean: remove fifo, devices, socket files

[Andrew Vagin]

On Fri, Jul 15, 2016 at 10:33:42PM +0200, Johannes Sixt wrote:
> Am 15.07.2016 um 09:46 schrieb Andrey Vagin:
> > On Thu, Jul 14, 2016 at 10:56 PM, Johannes Sixt  wrote:
> > > IOW: These special files are invisible for Git unless it already knows the
> > > names. The latter case is outside 'git clean's domain, and the former case
> > > really means that special files in the working tree are left at the user's
> > > discretion.
> > 
> > I understand your points, but I don't see any reasons to ignore these files.
> > 
> > What will be wrong if 'git status' will reports these files?
> > What will be wrong if 'git add' will returns an error instead of
> > skipping them silently?
> 
> I can buy that 'git add' reports an error for special files. (And I concur
> with Dscho that the behavior should otherwise remain unchanged.) But this is
> not what the commit message sells even if the patch changes the behavior of
> 'git add', too (I haven't tested the patch).

Ok. Thank you for the feedback.

> 
> -- Hannes
> 
--
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: Plugin mechanism(s) for Git?

[Jeff King]

On Sat, Jul 16, 2016 at 07:31:25AM +0200, Duy Nguyen wrote:

> >> > I wouldn’t be too quick to dismiss dynamically loaded libraries as
> >> > there are some distinct advantages over the other patterns especially
> >> > performance and simplicity.  I realize it requires us to version the
> >> > ABI but there are established patterns to manage this.  It also isn’t
> >> > that much different than us having to freeze or version the protocol
> >> > for communicating with a remote-helper.
> >
> > (re-adding dropped CCs)
> >
> > The critical difference is protocols can be tested and debugged
> > in a language/tool-independent way.
> 
> Not using dynamic libraries also makes it possible for other git
> reimplementations to reuse the same plugins.

Good point.

I think for the ref backends (which so far are the main pluggable thing
that people expect to link in), the assumption was that the on-disk
formats would be documented, and implementations would then write their
own plugins to match the on-disk formats.

I think all of the other implementations _except_ core git already have
their own internal pluggable ref formats (I know libgit2 and JGit do).

In theory if Git defines the API for the plugin to meet, then one could
use those plugins with other implementations. In practice, I think ref
plugins will also make use of Git functions (even things as simple as
xmalloc), and the ABI there is anything but stable. So I think ref
backends are "pluggable" in the sense of "configurable", but not in
the sense of dynamically linking third party code that git has never
seen.

Likewise, I don't think licensing issues have really been discussed. I'd
think at this point that any ref backends would need to be GPL, as they
are quite intimately part of git, and not true pluggable modules.

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

Re: [PATCH 12/12] receive-pack: send keepalives during quiet periods

[Jeff King]

On Fri, Jul 15, 2016 at 10:24:16AM -0700, Stefan Beller wrote:

> > @@ -319,10 +331,60 @@ static void rp_error(const char *err, ...)
> >  static int copy_to_sideband(int in, int out, void *arg)
> >  {
> > char data[128];
> 
> While looking at this code, do you think it is feasible to increase the
> size of data[] to 1024 ? (The largest that is possible when
> side-band, but no side-band-64k is given).

I don't see any reason it could not be increased. On the other hand, I
find it unlikely to accomplish anything. We are relaying stderr over
sideband 2 here, so assuming that typically consists of human-readable
lines, they're unlikely to go over 128 (and if they do, they simply get
split into two packets).

I suppose if your hooks dump large ascii art to the user, we could save
a few framing bytes.

So I don't mind it, but it's definitely orthogonal to this series.

> The method was short and concise, this adds a lot of lines.
> Remembering d751dd11 (2016-07-10, hoist out handle_nonblock
> function for xread and xwrite), do you think it would be reasonable to
> put the whole poll handling into a dedicated function, maybe even reuse the
> that function?
> 
> if (keepalive_active) {
> if (wrapper_around_poll(_in) < 0) // handles EINTR internally
> break;
> if (!data_in)
> send_keep_alive();
> }
> 
> I am not sure if that makes this function more legible, just food for thought.

That would skip the EINTR continue, though since we're looping anyway
here, I'm not sure it's a big deal. The biggest simplification is more
like this, I think:

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e41f55f..8d792a2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -328,6 +328,32 @@ static void rp_error(const char *err, ...)
va_end(params);
 }
 
+/* returns 1 if data is available, 0 otherwise */
+static int keepalive_poll(int in, int out)
+{
+   struct pollfd pfd;
+   int ret;
+
+   pfd.fd = fd;
+   pfd.events = POLLIN;
+   ret = poll(, 1, 1000 * keepalive_in_sec);
+
+   if (ret < 0) {
+   if (errno == EINTR)
+   return 0; /* no data, we should try again */
+   else
+   return 1; /* no data, but read should return real error 
*/
+   }
+   if (ret == 0) {
+   /* no data; send a keepalive packet */
+   static const char buf[] = "0005\1";
+   write_or_die(out, buf, sizeof(buf) - 1);
+   return 0;
+   }
+   /* else there is actual data to read */
+   return 1;
+}
+
 static int copy_to_sideband(int in, int out, void *arg)
 {
char data[128];
@@ -341,26 +367,8 @@ static int copy_to_sideband(int in, int out, void *arg)
while (1) {
ssize_t sz;
 
-   if (keepalive_active) {
-   struct pollfd pfd;
-   int ret;
-
-   pfd.fd = in;
-   pfd.events = POLLIN;
-   ret = poll(, 1, 1000 * keepalive_in_sec);
-
-   if (ret < 0) {
-   if (errno == EINTR)
-   continue;
-   else
-   break;
-   } else if (ret == 0) {
-   /* no data; send a keepalive packet */
-   static const char buf[] = "0005\1";
-   write_or_die(1, buf, sizeof(buf) - 1);
-   continue;
-   } /* else there is actual data to read */
-   }
+   if (keepalive_active && !keepalive_poll(in, 1))
+   continue;
 
sz = xread(in, data, sizeof(data));
if (sz <= 0)

Note that there are actually three outcomes to poll (error, timeout, or
data), and this simplifies away the "error" case to just "try to read
some data" and assumes that the read() will cover (which is similar to
what xread() does).

I dunno if it really helps much, though. There's still a bunch of
keepalive logic in the function to handle the NUL-signal case.

I guess your proposal is more to have an "xpoll" that handles the EINTR
looping, and leave that logic in place. I think that still leaves most
of the complexity in the function.

> > +   } else if (ret == 0) {
> > +   /* no data; send a keepalive packet */
> > +   static const char buf[] = "0005\1";
> 
> and the \1 is the first sideband. Why do we choose that sideband?

What other sideband do you propose? :)

More seriously, this is modeled on the similar keepalive in upload-pack.
The only other option is sideband 2, and IIRC, clients do bad things
with empty band-2 packets (like printing "remote: " but never finishing
the line if we never 

Re: [PATCH v2 2/2] archive-tar: huge offset and future timestamps would not work on 32-bit

[Duy Nguyen]

On Fri, Jul 15, 2016 at 12:36 AM, Junio C Hamano  wrote:
> Jeff King  writes:
>
>> On Thu, Jul 14, 2016 at 01:43:57PM -0700, Junio C Hamano wrote:
>>
>>> As we are not yet moving everything to size_t but still using ulong
>>> internally when talking about the size of object, platforms with
>>> 32-bit long will not be able to produce tar archive with 4GB+ file,
>>> and cannot grok 0777UL as a constant.  Disable the extended
>>> header feature and do not test it on them.
>>
>> I tried testing this in a VM with 32-bit Debian. It fixes the build
>> problems, but t5000 still fails.
>
> Thanks for testing.  I need to find some time hunting for (or
> building) an environment to do that myself.

If you are on a distro with multilib, building git with "CFLAGS=-m32
LDFLAGS=-m32" should do it. That's how i tested the 32-bit offset
truncation thing. I may pursue the docker thing soon. At least then
you only need to install docker and just build/test (zero docker
configuration, assuming all the relevant kernel options are enabled).
-- 
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: Bug in index-helper/watchman?

[Duy Nguyen]

On Sat, Jul 16, 2016 at 08:10:15AM +0200, Duy Nguyen wrote:
> But you did spot a problem. What if UC extension is loaded _after_
> watchman one? Then index->untracked_cache would have nothing in there
> and invalidation is no-op when we do it (at watchmain ext loading
> time). We can't control extension loading order (technically we can,
> but other git implementations may do it differently). So, maybe we
> should do the invalidation after _all_ index extensions have been
> loaded.
> 
> Maybe we can do it in validate_untracked_cache? We already store the
> path list in the_index.untracked->invalid_untracked.
> validate_untracked_cache has all info it needs.

The squash for "index-helper: use watchman to avoid refreshing index
with lstat()" looks something like this. I did not test it at all though.

-- 8< --
diff --git a/dir.c b/dir.c
index 024b13c..04bd87c 100644
--- a/dir.c
+++ b/dir.c
@@ -1902,6 +1902,43 @@ void remove_untracked_cache(struct index_state *istate)
}
 }
 
+static struct untracked_cache_dir *find_untracked_cache_dir(
+   struct untracked_cache *uc, struct untracked_cache_dir *ucd,
+   const char *name)
+{
+   int component_len;
+   const char *end;
+   struct untracked_cache_dir *dir;
+
+   if (!*name)
+   return ucd;
+
+   end = strchr(name, '/');
+   if (end)
+   component_len = end - name;
+   else
+   component_len = strlen(name);
+
+   dir = lookup_untracked(uc, ucd, name, component_len);
+   if (dir)
+   return find_untracked_cache_dir(uc, dir, name + component_len + 
1);
+
+   return NULL;
+}
+
+static int mark_untracked_invalid(struct string_list_item *item, void *uc)
+{
+   struct untracked_cache *untracked = uc;
+   struct untracked_cache_dir *dir;
+
+   dir = find_untracked_cache_dir(untracked, untracked->root,
+  item->string);
+   if (dir)
+   dir->valid = 0;
+
+   return 0;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct 
*dir,
  int base_len,
  const struct pathspec 
*pathspec)
@@ -1984,6 +2021,11 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
 
/* Make sure this directory is not dropped out at saving phase */
root->recurse = 1;
+
+   if (dir->untracked->use_watchman) {
+   for_each_string_list(>untracked->invalid_untracked,
+mark_untracked_invalid, dir->untracked);
+   }
return root;
 }
 
diff --git a/dir.h b/dir.h
index ed16746..896b64a 100644
--- a/dir.h
+++ b/dir.h
@@ -312,7 +312,4 @@ struct untracked_cache *read_untracked_extension(const void 
*data, unsigned long
 void write_untracked_extension(struct strbuf *out, struct untracked_cache 
*untracked);
 void add_untracked_cache(struct index_state *istate);
 void remove_untracked_cache(struct index_state *istate);
-struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
-struct untracked_cache_dir *dir,
-const char *name, int len);
 #endif
diff --git a/read-cache.c b/read-cache.c
index ee777c1..b90187c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1388,37 +1388,12 @@ static int verify_hdr(const struct cache_header *hdr, 
unsigned long size)
return 0;
 }
 
-static struct untracked_cache_dir *find_untracked_cache_dir(
-   struct untracked_cache *uc, struct untracked_cache_dir *ucd,
-   const char *name)
-{
-   int component_len;
-   const char *end;
-   struct untracked_cache_dir *dir;
-
-   if (!*name)
-   return ucd;
-
-   end = strchr(name, '/');
-   if (end)
-   component_len = end - name;
-   else
-   component_len = strlen(name);
-
-   dir = lookup_untracked(uc, ucd, name, component_len);
-   if (dir)
-   return find_untracked_cache_dir(uc, dir, name + component_len + 
1);
-
-   return NULL;
-}
-
 static void mark_no_watchman(size_t pos, void *data)
 {
struct index_state *istate = data;
struct cache_entry *ce = istate->cache[pos];
struct strbuf sb = STRBUF_INIT;
char *c;
-   struct untracked_cache_dir *dir;
 
assert(pos < istate->cache_nr);
ce->ce_flags |= CE_WATCHMAN_DIRTY;
@@ -1438,27 +1413,11 @@ static void mark_no_watchman(size_t pos, void *data)
if (c == sb.buf)
strbuf_setlen(, 0);
 
-   dir = find_untracked_cache_dir(istate->untracked,
-  istate->untracked->root, sb.buf);
-   if (dir)
-   dir->valid = 0;
+   string_list_append(>untracked->invalid_untracked, ce->name);
 
strbuf_release();
 }
 
-static int mark_untracked_invalid(struct string_list_item 

Re: Bug in index-helper/watchman?

[Duy Nguyen]

On Thu, Jul 14, 2016 at 5:21 PM, Duy Nguyen  wrote:
>> On Thu, Jul 14, 2016 at 12:11 AM, Ben Peart  wrote:
>>> I’ve been chasing down an issue where it looks like the untracked cache
>>> logic doesn’t work correctly in the index-helper/watchman patch series.
>>> It’s also entirely possible that I’m just missing something so feel free to
>>> correct my misconceptions.
>>>
>>>
>>>
>>> Ultimately, it appears that none of the untracked cache directories are
>>> getting flagged as invalid from the data in the watchman extension.  I
>>> believe this is happening because untracked->root doesn’t get initialized
>>> until validate_untracked_cache is called from read_directory.  This causes
>>> all calls to lookup_untracked to return NULL so the dir->valid flag is never
>>> set to zero in mark_untracked_invalid.

No, validate_untracked_cache does not do anything fancy in there to
make UC initialized. It may invalidate UC further though.

But you did spot a problem. What if UC extension is loaded _after_
watchman one? Then index->untracked_cache would have nothing in there
and invalidation is no-op when we do it (at watchmain ext loading
time). We can't control extension loading order (technically we can,
but other git implementations may do it differently). So, maybe we
should do the invalidation after _all_ index extensions have been
loaded.

Maybe we can do it in validate_untracked_cache? We already store the
path list in the_index.untracked->invalid_untracked.
validate_untracked_cache has all info it needs.
-- 
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