Re: [PATCH v1/RFC 1/1] 'git clone C:\cygwin\home\USER\repo' is working (again)

[Steven Penny]

On Mon, Nov 26, 2018 at 11:23 PM Junio C Hamano wrote:
> Sorry, but I do not see the connection to this question and the
> above example.  The reason why we strip C: is because the letter
> that comes after that colon determines if we are talking about
> absolute path (in other words, the current directory does not play a
> role in determining which directory the path refers to), unlike the
> POSIX codepath where it is always the first letter in the pathname.

while it is true that "C:" and similar do not have a bearing on a path being
absolute versus relative, it does have a bearing on what drive the entry is to
be found.

That is to say "C:\tmp\file.txt" does not equal "D:\tmp\file.txt".

Starting with an absolute path like "C:\tmp\file.txt", after stripping that
would yield "\tmp\file.txt" or "/tmp/file.txt". Starting with a relative path
like "C:tmp\file.txt", after stripping that would yield "tmp\file.txt" or
"tmp/file.txt".

However in all cases we have lost the concept of what drive the file is located
on, and Windows will assume the file exists on the current drive.

So I would expect "git clone URL D:\tmp" to fail if the current directory is on
"C:". Upon testing cross drive clones work fine though with this patch, so maybe
the drive is added back at another place in the code.

Re: [PATCH] pack-protocol.txt: accept error packets in any context

[Junio C Hamano]

Masaya Suzuki  writes:

> In the Git pack protocol definition, an error packet may appear only in
> a certain context. However, servers can face a runtime error (e.g. I/O
> error) at an arbitrary timing. This patch changes the protocol to allow
> an error packet to be sent instead of any packet.

Makes perfect sense.

> Following this protocol spec change, the error packet handling code is
> moved to pkt-line.c.
>
> Signed-off-by: Masaya Suzuki 
> ---
>  Documentation/technical/pack-protocol.txt | 20 +++-
>  builtin/archive.c |  2 --
>  connect.c |  2 --
>  fetch-pack.c  |  2 --
>  pkt-line.c|  4 
>  5 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/technical/pack-protocol.txt 
> b/Documentation/technical/pack-protocol.txt
> index 6ac774d5f..7a2375a55 100644
> --- a/Documentation/technical/pack-protocol.txt
> +++ b/Documentation/technical/pack-protocol.txt
> @@ -22,6 +22,16 @@ protocol-common.txt. When the grammar indicate 
> `PKT-LINE(...)`, unless
>  otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
>  include a LF, but the receiver MUST NOT complain if it is not present.
>  
> +An error packet is a special pkt-line that contains an error string.
> +
> +
> +  error-line =  PKT-LINE("ERR" SP explanation-text)
> +
> +
> +Throughout the protocol, where `PKT-LINE(...)` is expected, an error packet 
> MAY
> +be sent. Once this packet is sent by a client or a server, the data transfer
> +process defined in this protocol is terminated.
> +
>  Transports
>  --
>  There are three transports over which the packfile protocol is
> @@ -89,13 +99,6 @@ process on the server side over the Git protocol is this:
>   "0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
>   nc -v example.com 9418
>  
> -If the server refuses the request for some reasons, it could abort
> -gracefully with an error message.
> -
> -
> -  error-line =  PKT-LINE("ERR" SP explanation-text)
> -
> -
>  
>  SSH Transport
>  -
> @@ -398,12 +401,11 @@ from the client).
>  Then the server will start sending its packfile data.
>  
>  
> -  server-response = *ack_multi ack / nak / error-line
> +  server-response = *ack_multi ack / nak
>ack_multi   = PKT-LINE("ACK" SP obj-id ack_status)
>ack_status  = "continue" / "common" / "ready"
>ack = PKT-LINE("ACK" SP obj-id)
>nak = PKT-LINE("NAK")
> -  error-line =  PKT-LINE("ERR" SP explanation-text)
>  
>  
>  A simple clone may look like this (with no 'have' lines):
> diff --git a/builtin/archive.c b/builtin/archive.c
> index d2455237c..5d179bbd1 100644
> --- a/builtin/archive.c
> +++ b/builtin/archive.c
> @@ -59,8 +59,6 @@ static int run_remote_archiver(int argc, const char **argv,
>   if (strcmp(buf, "ACK")) {
>   if (starts_with(buf, "NACK "))
>   die(_("git archive: NACK %s"), buf + 5);
> - if (starts_with(buf, "ERR "))
> - die(_("remote error: %s"), buf + 4);
>   die(_("git archive: protocol error"));
>   }
>  
> diff --git a/connect.c b/connect.c
> index 24281b608..458906e60 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -306,8 +306,6 @@ struct ref **get_remote_heads(struct packet_reader 
> *reader,
>   die_initial_contact(1);
>   case PACKET_READ_NORMAL:
>   len = reader->pktlen;
> - if (len > 4 && skip_prefix(reader->line, "ERR ", ))
> - die(_("remote error: %s"), arg);
>   break;
>   case PACKET_READ_FLUSH:
>   state = EXPECTING_DONE;
> diff --git a/fetch-pack.c b/fetch-pack.c
> index 9691046e6..e66cd7b71 100644
> --- a/fetch-pack.c
> +++ b/fetch-pack.c
> @@ -178,8 +178,6 @@ static enum ack_type get_ack(int fd, struct object_id 
> *result_oid)
>   return ACK;
>   }
>   }
> - if (skip_prefix(line, "ERR ", ))
> - die(_("remote error: %s"), arg);
>   die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
>  }
>  
> diff --git a/pkt-line.c b/pkt-line.c
> index 04d10bbd0..ce9e42d10 100644
> --- a/pkt-line.c
> +++ b/pkt-line.c
> @@ -346,6 +346,10 @@ enum packet_read_status packet_read_with_status(int fd, 
> char **src_buffer,
>   return PACKET_READ_EOF;
>   }
>  
> + if (starts_with(buffer, "ERR ")) {
> + die(_("remote error: %s"), buffer + 4);
> + }
> +
>   if ((options & PACKET_READ_CHOMP_NEWLINE) &&
>   len && buffer[len-1] == '\n')
>   len--;

Re: [PATCH v1/RFC 1/1] 'git clone C:\cygwin\home\USER\repo' is working (again)

[Junio C Hamano]

Steven Penny  writes:

> If you strip the drive, you can still navigate within the same drive:
>
> $ cd 'C:\Users'
> $ pwd
> /cygdrive/c/Users
>
> $ cd '\Windows'
> $ pwd
> /cygdrive/c/Windows
>
> but you can no longer traverse drives:
>
> $ cd '\Testing'
> sh: cd: \Testing: No such file or directory

Sorry, but I fail to see the point the last example wants to make.
If it were

$ cd /cygdrive/d/Testing
$ cd /cygdrive/c/Users
$ cd ../../d/Testing

and the last step fails, then I would suspect it would make sense to
treat /cygdrive/$X exactly like how we would treat $C:, because

$ cd C:Users
$ cd ../D:Testing

would not make sense, either, which is an indication that these two
are quite similar.  On the other hand, if "cd ../../d/Testing" above
does not fail and does what non-DOS users would expect, then that
strongly argues that treating /cygdrive/$X any specially is a mistake.

> So a good first question for me would be: why are we stripping "C:" or similar
> in the first place?

Sorry, but I do not see the connection to this question and the
above example.  The reason why we strip C: is because the letter
that comes after that colon determines if we are talking about
absolute path (in other words, the current directory does not play a
role in determining which directory the path refers to), unlike the
POSIX codepath where it is always the first letter in the pathname.

C:\Users is a directory whose name is Users at the top level of the
C: drive. C0\Users tells us that in the current directory, there is
a directory whose name is C0 and in it, there is a filesystem entity
whose name is Users.  So the colon that follows an alpha (in this
case, C:) is quite special, compared to other letters (in this
example, I used '0' to contrast its effect with ':').  So it is very
understandable why we want to have has_dos_prefix() and
skip_dos_prefix().

> I would say these could be merged into a "win.h" or similar. Cygwin typically
> leans toward the "/unix/style" while MINGW has been more tolerant of
> "C:\Windows\Style" and "C:/Mixed/Style" paths, i dont see that changing.

I'd defer to Windows folks to decide if a unified win.h is a good
idea.

Thanks.

[PATCH] pack-protocol.txt: accept error packets in any context

[Masaya Suzuki]

In the Git pack protocol definition, an error packet may appear only in
a certain context. However, servers can face a runtime error (e.g. I/O
error) at an arbitrary timing. This patch changes the protocol to allow
an error packet to be sent instead of any packet.

Following this protocol spec change, the error packet handling code is
moved to pkt-line.c.

Signed-off-by: Masaya Suzuki 
---
 Documentation/technical/pack-protocol.txt | 20 +++-
 builtin/archive.c |  2 --
 connect.c |  2 --
 fetch-pack.c  |  2 --
 pkt-line.c|  4 
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index 6ac774d5f..7a2375a55 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -22,6 +22,16 @@ protocol-common.txt. When the grammar indicate 
`PKT-LINE(...)`, unless
 otherwise noted the usual pkt-line LF rules apply: the sender SHOULD
 include a LF, but the receiver MUST NOT complain if it is not present.
 
+An error packet is a special pkt-line that contains an error string.
+
+
+  error-line =  PKT-LINE("ERR" SP explanation-text)
+
+
+Throughout the protocol, where `PKT-LINE(...)` is expected, an error packet MAY
+be sent. Once this packet is sent by a client or a server, the data transfer
+process defined in this protocol is terminated.
+
 Transports
 --
 There are three transports over which the packfile protocol is
@@ -89,13 +99,6 @@ process on the server side over the Git protocol is this:
  "0039git-upload-pack /schacon/gitbook.git\0host=example.com\0" |
  nc -v example.com 9418
 
-If the server refuses the request for some reasons, it could abort
-gracefully with an error message.
-
-
-  error-line =  PKT-LINE("ERR" SP explanation-text)
-
-
 
 SSH Transport
 -
@@ -398,12 +401,11 @@ from the client).
 Then the server will start sending its packfile data.
 
 
-  server-response = *ack_multi ack / nak / error-line
+  server-response = *ack_multi ack / nak
   ack_multi   = PKT-LINE("ACK" SP obj-id ack_status)
   ack_status  = "continue" / "common" / "ready"
   ack = PKT-LINE("ACK" SP obj-id)
   nak = PKT-LINE("NAK")
-  error-line =  PKT-LINE("ERR" SP explanation-text)
 
 
 A simple clone may look like this (with no 'have' lines):
diff --git a/builtin/archive.c b/builtin/archive.c
index d2455237c..5d179bbd1 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -59,8 +59,6 @@ static int run_remote_archiver(int argc, const char **argv,
if (strcmp(buf, "ACK")) {
if (starts_with(buf, "NACK "))
die(_("git archive: NACK %s"), buf + 5);
-   if (starts_with(buf, "ERR "))
-   die(_("remote error: %s"), buf + 4);
die(_("git archive: protocol error"));
}
 
diff --git a/connect.c b/connect.c
index 24281b608..458906e60 100644
--- a/connect.c
+++ b/connect.c
@@ -306,8 +306,6 @@ struct ref **get_remote_heads(struct packet_reader *reader,
die_initial_contact(1);
case PACKET_READ_NORMAL:
len = reader->pktlen;
-   if (len > 4 && skip_prefix(reader->line, "ERR ", ))
-   die(_("remote error: %s"), arg);
break;
case PACKET_READ_FLUSH:
state = EXPECTING_DONE;
diff --git a/fetch-pack.c b/fetch-pack.c
index 9691046e6..e66cd7b71 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -178,8 +178,6 @@ static enum ack_type get_ack(int fd, struct object_id 
*result_oid)
return ACK;
}
}
-   if (skip_prefix(line, "ERR ", ))
-   die(_("remote error: %s"), arg);
die(_("git fetch-pack: expected ACK/NAK, got '%s'"), line);
 }
 
diff --git a/pkt-line.c b/pkt-line.c
index 04d10bbd0..ce9e42d10 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -346,6 +346,10 @@ enum packet_read_status packet_read_with_status(int fd, 
char **src_buffer,
return PACKET_READ_EOF;
}
 
+   if (starts_with(buffer, "ERR ")) {
+   die(_("remote error: %s"), buffer + 4);
+   }
+
if ((options & PACKET_READ_CHOMP_NEWLINE) &&
len && buffer[len-1] == '\n')
len--;
-- 
2.20.0.rc0.387.gc7a69e6b6c-goog

Re: [RFC PATCH v2] Add 'human' date format

[Stephen P. Smith]

On Saturday, July 7, 2018 3:02:35 PM MST Linus Torvalds wrote:
> From: Linus Torvalds 
> 
> This adds --date=human, which skips the timezone if it matches the
> current time-zone, and doesn't print the whole date if that matches (ie
> skip printing year for dates that are "this year", but also skip the
> whole date itself if it's in the last few days and we can just say what
> weekday it was).

In the "What's cooking" email response [1] there was some 
concern about what to call this format.

While the format is more useful to humans by dropping inferred information, 
there is nothing that makes it actually human.  Additionally, what 
one human considers a good format another may not like.  

In part of the V2 patch email[2], Linus notes that the patch reports 
relative dates based on information inferred from the date 
on the machine running the git command at the time the 
command is executed.   

If relative wasn't already taken that would be a reasonable 
choice instead of human.

Therefore keeping 'human' and documenting how the patch formats 
the date to suppress future "infinite" tweaks to the format seems 
like a a reasonable way to approach your concerns. [1]

I will start working the Doc's and test updates.   

On Wednesday, November 21, 2018 6:06:13 PM MST Junio C Hamano wrote:
> Command line completion;

I'm not quite sure what you want for this item.   Could you please describe?

sps

[1] https://public-inbox.org/git/xmqq8t1l6hve@gitster-ct.c.googlers.com/
[2] https://public-inbox.org/git/alpine.lfd.2.21.999.1807071502260.18...@i7.lan/

From Mr.Frank Yacouba

[Mr. Frank Yacouba]

With due respect,

Please forgive me for stressing you with my business as I know that
this letter may come to you as big surprise. Truly, I came across your
contact email address through my personal search, then afterward I
decided to email you directly believing that you will be honest to
help me.

My name is Mr. Frank Yacouba, the Head of file Department, Africa
Develop bank (ADB) Ouagadougou, Burkina Faso, in West Africa. I am
contacting you to seek our honesty and sincere cooperation in
confidential manner to transfer the sum of $15 million U.S.A dollars
to your existing or new bank account. In my department we discovered
an abandoned sum of $15 million U.S.A dollars. In an account that
belongs to one of our foreign customer who died along with all his
family and all the relation died along side with him at the Earth
Quake Disaster leaving nobody behind for the claim.

I want you stand as next of kin to the deceased. I agree that 40% of
this money will be for you as foreign partner, in respect to the
provision of a foreign account and 60% would be for me. Upon receipt
of your reply, I will send to you by email the text of the
application. I will not fail to bring to your notice that this
transaction is hitch free and that you should not entertain any atom
of fear as all required arrangements have been made for the transfer.

You have to keep everything secret as to enable the transfer to move
very smoothly in to the account you will prove to the bank.

FILL THIS FORM BELLOW AND RESEND IT TO ME.
1) Your Full Name
2) Your Age.
3) Marital Status..
4) Your Cell Phone Number...
5) Your Fax Number
6) Your Country...
7) Your Occupation
8) Sex
9) Your Religion..
10) Your Private E-mail Address..

I am waiting for your immediate response as you receive this mail.

Yours faithfully,
Mr.Frank Yacouba
E-mail:bissappa...@yandex.com

Re: [PATCH v1/RFC 1/1] 'git clone C:\cygwin\home\USER\repo' is working (again)

[Steven Penny]

On Mon, Nov 26, 2018 at 7:16 PM Junio C Hamano wrote:
> I wonder if it makes the rest of the code simpler if we stripped
> things like /cygdrive/c here exactly the sam way as we strip C:
> For that, has_dos_drive_prefix() needs to know /cygdrive/[a-z],
> which may not be a bad thing, I guess.  Let's read on.

With full paths, Cygwin can traverse drives:

$ cd 'C:\Users'
$ pwd
/cygdrive/c/Users

$ cd 'D:\Testing'
$ pwd
/cygdrive/d/Testing

If you strip the drive, you can still navigate within the same drive:

$ cd 'C:\Users'
$ pwd
/cygdrive/c/Users

$ cd '\Windows'
$ pwd
/cygdrive/c/Windows

but you can no longer traverse drives:

$ cd '\Testing'
sh: cd: \Testing: No such file or directory

So a good first question for me would be: why are we stripping "C:" or similar
in the first place?

>  - Is there a point in having cygwin specific variant of these, or
>can we just borrow from mingw version (with some refactoring)?
>Is there a point in doing so (e.g. if mingw plans to move to
>reject forward slashes, attempting to share is pointless).

I would say these could be merged into a "win.h" or similar. Cygwin typically
leans toward the "/unix/style" while MINGW has been more tolerant of
"C:\Windows\Style" and "C:/Mixed/Style" paths, i dont see that changing.

Re: [PATCH v1/RFC 1/1] 'git clone C:\cygwin\home\USER\repo' is working (again)

[Junio C Hamano]

tbo...@web.de writes:

> Reported-By: Steven Penny 
> Signed-off-by: Torsten Bögershausen 
> ---
>
> This is the first vesion of a patch.
> Is there a chance that you test it ?
>
> abspath.c   |  2 +-
>  compat/cygwin.c | 18 ++
>  compat/cygwin.h | 32 
>  3 files changed, 47 insertions(+), 5 deletions(-)

I am hoping that the funny indentation above is merely an accidental
touch on the delete key and not a sign of MUA eating the patch to
make it unapplicable (and making it harder for those who want to
test to test it).

> diff --git a/compat/cygwin.c b/compat/cygwin.c
> index b9862d606d..c4a10cb5a1 100644
> --- a/compat/cygwin.c
> +++ b/compat/cygwin.c
> @@ -1,19 +1,29 @@
>  #include "../git-compat-util.h"
>  #include "../cache.h"
>  
> +int cygwin_skip_dos_drive_prefix(char **path)
> +{
> + int ret = has_dos_drive_prefix(*path);
> + *path += ret;
> + return ret;
> +}

Mental note: this is exactly the same as mingw version.

I wonder if it makes the rest of the code simpler if we stripped
things like /cygdrive/c here exactly the sam way as we strip C:
For that, has_dos_drive_prefix() needs to know /cygdrive/[a-z],
which may not be a bad thing, I guess.  Let's read on.

>  int cygwin_offset_1st_component(const char *path)
>  {
> - const char *pos = path;
> + char *pos = (char *)path;
> +
>   /* unc paths */
> - if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
> + if (!skip_dos_drive_prefix() &&
> + is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {

When given C:\foo\bar, this strips prefix to leave \foo\bar in pos and
then realizes that it cannot be unc path (because it has dos prefix)
and goes on.  What is returned from the function is "\foo\bar" + 1 -
path, i.e. the offset in the original "C:\foo\bar" string of the
'f' in "foo", i.e. 3.

When given \foo\bar, pos stays the same as path, and it skips the
first backslash and returns the offset in the original string of the
'f' in "foo", i.e. 1.

Both cases return the moreal equivalent --- the offset of the first
component 'foo'.  So this looks correct for these two cases.

>   /* skip server name */
> - pos = strchr(pos + 2, '/');
> + pos = strpbrk(pos + 2, "\\/");

This is to allow \\server\path in addition to //server/path; the
original looked only for '/' with strchr but we now look for either
'/' or '\', whichever comes earlier.  Both helpers return NULL when
they find no separator, so we should be able to handle the returned
pos from here on the same way as the original code.

>   if (!pos)
>   return 0; /* Error: malformed unc path */
>  
>   do {
>   pos++;
> - } while (*pos && pos[0] != '/');
> + } while (*pos && !is_dir_sep(*pos));

And whenever we looked for '/', we consider '\' its equivalent.

>   }
> +
>   return pos + is_dir_sep(*pos) - path;
>  }

Looks good so far.

Wait, did I just waste time by not looking at mingw.c version?  I
suspect this would be exactly the same ;-)

> diff --git a/compat/cygwin.h b/compat/cygwin.h
> index 8e52de4644..46f29c0a90 100644
> --- a/compat/cygwin.h
> +++ b/compat/cygwin.h
> @@ -1,2 +1,34 @@
> +#define has_dos_drive_prefix(path) \
> + (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)

Metanl note: this also looks the same as mingw version.

> +int cygwin_offset_1st_component(const char *path);
> +#define offset_1st_component cygwin_offset_1st_component
> +

So, my real questions are

 - Is there a point in having cygwin specific variant of these, or
   can we just borrow from mingw version (with some refactoring)?
   Is there a point in doing so (e.g. if mingw plans to move to
   reject forward slashes, attempting to share is pointless).

 - Would it make it better (or worse) to treat the /cygdrive/c thing
   as another way to spell dos-drive-prefix?  If the answer is "it
   is a good idea", then that answers the previous question
   automatically (we cannot gain much by sharing, as mingw side
   won't want to treat /cygdrive/c any differently).

Re: [PATCH v1 1/2] log -G: Ignore binary files

[Junio C Hamano]

Stefan Beller  writes:

> On Wed, Nov 21, 2018 at 1:08 PM Thomas Braun
>  wrote:
>>
>> The -G  option of log looks for the differences whose patch text
>> contains added/removed lines that match regex.
>>
>> The concept of differences only makes sense for text files, therefore
>> we need to ignore binary files when searching with -G  as well.
>
> What about partial text/partial binary files?

Good point. You'd use "-a" (or "--text") to tell the diff machinery
to treat the contents as text, and the new logic must pay attention
to that command line option.

Re: [PATCH 2/5] ieot: default to not writing IEOT section

[Junio C Hamano]

Stefan Beller  writes:

>> > +static int record_ieot(void)
>> > +{
>> > + int val;
>> > +
>>
>> Initialize stack val to zero to ensure proper default.
>
> I don't think that is needed here, as we only use `val` when
> we first write to it via git_config_get_bool.

Yup.

Re: [ANNOUNCE] Git v2.20.0-rc1

[Junio C Hamano]

Elijah Newren  writes:

> If we don't set rebase.useBuiltin to false, then there is also a minor
> regression in the error message printed by the built-in rebase we may
> want to try to address.  I have a patch for it at
> <20181122044841.20993-2-new...@gmail.com>, but I don't know if that
> patch is acceptable as-is this close to a release since that'd not
> give translators much time to update.

For this particular one, I'd rather ship "rebase in C" with known
message glitch, with or without the "mark it experimental and make
it opt-in" last-time change.  Of course, turning it off by default
would let us worry about these message glitches even less, but at
this point, I'd be worried more about bugs that can affect the
actual operation and outcome recorded in the resulting history.

Re: [RFC] Introduce two new commands, switch-branch and restore-paths

[Junio C Hamano]

Stefan Beller  writes:

> Maybe we need to step back and consider what the command does.
> And from that I would name it "rewire-HEAD-and-update-index"
> and then simplify from there. For the beginner user, the concept of
> HEAD might be overwhelming, such that we don't want to have that
> in there.

I'd have to say that it is totally backwards.

Use of HEAD, ref, etc. is merely a means to what the end users want
to achieve, which is to switch to a "branch" (in air quotes, as the
word in this sentence means a bit broader than "a ref that is in
refs/heads/"), to choose which lineage of commits to work on to grow
or reshape the history.  Most of the time, you would be working on a
branch (that is a ref that is in refs/heads/), sometimes you would
be working on an unnamed "branch" (i.e. detached HEAD state, only
difference between being on it and a normal branch is whether it is
named---the history manipulation can happen exactly the same way).

In other words, your initial motivation of stepping back and
thinking about what the command is about is very good.  But in the
context of checking out a branch, the concept the end user works
with are at the level of "branch", not HEAD, ref, index, working
tree (all of which are underlying implementation details that let
you work on manipulating the history, represented by the "branch").

> "content-to-path", maybe(?) ...

Path is not a place.  A path t/Makefile is a shared name of a place
in a tree, the index, or in the working tree.  Renaming "git add" to
"content-to-path" because it adds the content to the index at path
may be equally OK, but it misses the essense (which is that it is to
add to the index and not to anythng else like a tree or the working
tree).

I actually think the easiest-to-understand shorthand for the
operation "checking out the contents at the path to the working
tree" is "checkout".  

These...

>   git tree-to-worktree # git checkout  -- 
>   git index-to-worktree # git checkout -- 

...are interesting and worth learning lessons from.  These would be
something people would suggest when users start making noises about
"restore-to-path" or "content-to-path" overloads three different
operations and is confusing.

I think "restore-to-path" that can take contents for individual
paths out of either a treeish or the index and update the index and
the working tree, depending on what the user tells it to do, is not
confusing to the end users.  At the conceptual level, the users need
to have a mental model that has three places (tree-ish, index and
working tree) that can hold contents to make use of these
operations, so it is only the matter of how to express it clearly
and concisely.

For that matter, "checkout ", "checkout  -- "
and "checkout -- " already is a trio of clear and cocncise way
to spell three distinct operations, so with the right mental model,
it may not be so confusing to the users.  And "checkout-branch",
"checkout-contents-from-tree", and "checkout-contents-from-index"
longhands may be a good way to help new users form the right mental
model, as they are more explicit in their names; once they form the
right mental model (that is, there are three places the contents
live, and there are a few operations to move contents from the two
places to the working tree, i.e. what traditionally is called
"checking things out"), they may gradulate to the shorthand form.

Re: [PATCH v1/RFC 1/1] 'git clone C:\cygwin\home\USER\repo' is working (again)

[Steven Penny]

On Mon, Nov 26, 2018 at 11:32 AM wrote:
> This is the first vesion of a patch.
> Is there a chance that you test it ?

I can confirm that this fixes the issue.

Thank you!

Re: Confusing behavior with ignored submodules and `git commit -a`

[Stefan Beller]

On Thu, Nov 15, 2018 at 4:31 PM Michael Forney  wrote:
>
> On 2018-11-15, Stefan Beller  wrote:
> > On Thu, Nov 15, 2018 at 1:33 PM Michael Forney  wrote:
> >> Well, currently the submodule config can be disabled in diff_flags by
> >> setting override_submodule_config=1. However, I'm thinking it may be
> >> simpler to selectively *enable* the submodule config in diff_flags
> >> where it is needed instead of disabling it everywhere else (i.e.
> >> use_submodule_config instead of override_submodule_config).
> >
> > This sounds like undoing the good(?) part of the series that introduced
> > this regression, as before that we selectively loaded the submodule
> > config, which lead to confusion when you forgot it. Selectively *enabling*
> > the submodule config sounds like that state before?
> >
> > Or do we *only* talk about enabling the ignore flag, while loading the
> > rest of the submodule config automatic?
>
> Yes, that is what I meant. I believe the automatic loading of
> submodule config is the right thing to do, it just uncovered cases
> where the current handling of override_submodule_config is not quite
> sufficient.

That sounds good.

>
> My suggestion of replacing override_submodule_config with
> use_submodule_config is because it seems like there are fewer places
> where we want to apply the ignore config than not. I think it should
> only apply in diffs against the working tree and when staging changes
> to the index (unless specified explicitly). The documentation just
> mentions the "diff family", but all but one of the possible values for
> submodule..ignore ("all") don't make sense unless comparing with
> the working tree. This is also how show/log -p behaved in git <2.15.
> So I think that clarifying that it is about modifications *to the
> working tree* would be a good idea.
>
> >> I'm also starting to see why this is tricky. The only difference that
> >> diff.c:run_diff_files sees between `git add inner` and `git add --all`
> >> is whether the index entry matched the pathspec exactly or not.
> >
> > Unrelated to the trickiness, I think we'd need to document the behavior
> > of the -a flag in git-add and git-commit better as adding the diff below
> > will depart from the "all" rule again, which I thought was a strong
> > motivator for Brandons series (IIRC).
>
> Can you explain what you mean by the "all" rule?

-a as short for --all in git commit, and I presumed it to be
'--all-content', but documentation tells me it's about files
only, so there is no really an "all" rule, but rather me misunderstanding
to what it applies.

Re: [ANNOUNCE] Git v2.20.0-rc1

[Johannes Schindelin]

Hi Ævar,

On Mon, 26 Nov 2018, Johannes Schindelin wrote:

> On Sat, 24 Nov 2018, Ævar Arnfjörð Bjarmason wrote:
> 
> > On Wed, Nov 21 2018, Junio C Hamano wrote:
> > 
> > >  * "git rebase" and "git rebase -i" have been reimplemented in C.
> > 
> > Here's another regression in the C version (and rc1), note: the
> > sha1collisiondetection is just a stand in for "some repo":
> > 
> > (
> > rm -rf /tmp/repo &&
> > git init /tmp/repo &&
> > cd /tmp/repo &&
> > for c in 1 2
> > do
> > touch $c &&
> > git add $c &&
> > git commit -m"add $c"
> > done &&
> > git remote add origin 
> > https://github.com/cr-marcstevens/sha1collisiondetection.git &&
> > git fetch &&
> > git branch --set-upstream-to origin/master &&
> > git rebase -i
> > )
> > 
> > The C version will die with "fatal: unable to read tree
> > ". Running this with
> > rebase.useBuiltin=false does the right thing and rebases as of the merge
> > base of the two (which here is the root of the history).
> 
> Sorry, this bug does not reproduce here:
> 
> $ git rebase -i
> Successfully rebased and updated refs/heads/master.
> 
> > I wasn't trying to stress test rebase. I was just wanting to rebase a
> > history I was about to force-push after cleaning it up, hardly an
> > obscure use-case. So [repeat last transmission in
> > https://public-inbox.org/git/87y39w1wc2@evledraar.gmail.com/ ]
> 
> Maybe you can give me the full details so that I can verify that this is
> indeed a bug in the builtin C and not just a regression caused by some
> random branches being merged together?
> 
> In short: please provide me with the exact URL and branch of your git.git
> fork to test. Then please make sure to specify the precise revision of the
> sha1collisiondetection/master rev, just in case that it matters.
> 
> Ideally, you would reduce the problem to a proper test case, say, for
> t3412 (it seems that you try to rebase onto an unrelated history, so it is
> *vaguely* related to "rebase-root").

So I was getting spooked enough by your half-complete bug report that I
did more digging (it is really quite a bit frustrating to have so little
hard evidence to go on, a wild goose chase is definitely not what I was
looking forward to after a day of fighting other fires, but you know,
built-in rebase is dear to me).

The error message you copied clearly comes from tree-walk.c, from
`fill_tree_descriptor()` (the other "unable to read tree" messages enclose
the hash in parentheses).

There are exactly 3 calls to said function in the built-in rebase/rebase
-i in the current `master`, a1598010f775 (Merge branch
'nd/per-worktree-ref-iteration', 2018-11-26):

$ git grep fill_tree_descriptor -- builtin/rebase*.c sequencer.[ch] 
rebase-interactive.[ch]
builtin/rebase.c:   if (!reset_hard && !fill_tree_descriptor([nr++], 
_oid)) {
builtin/rebase.c:   if (!fill_tree_descriptor([nr++], oid)) {
sequencer.c:if (!fill_tree_descriptor(, )) {

The last one of these is in `do_reset()`, i.e. handling a `reset` command
which you did not ask for, as you passed `-i` to `git rebase`, not `-ir`.

The first two *both* are in `reset_head()`. The first of them uses
`head_oid`, which is read directly via `get_oid("HEAD", _oid)`, so if
this is all zeroes for you, then it's not rebase's fault.

The second one uses the parameter `oid` passed into `reset_head()`. The
only calls to that function that do not pass `NULL` as `oid` (which would
trigger `oid` to be replaced by `_oid`, i.e again not all zeroes
unless your setup is broken) are:

- in the `--abort` code path
- in the `--autostash` code path
- in the fast-forwarding code path
- just after the "First, rewinding head" message in the *non*-interactive
  rebase

None of these apply to your script snippet.

Under the assumption that you might have forgotten to talk about
rebase.autostash=true and some dirty file, I tried to augment the script
snippet accordingly, but the built-in rebase as of current `master` still
works for me, plus: reading the autostash code path, it is hard to imagine
that the `lookup_commit_reference()` would return a pointer to a commit
object whose oid is all zeroes.

In short, even a thorough study of the code (keeping in mind the few
tidbits of information provided by you) leaves me really wondering which
code you run, because it sure does not look like current `master` to me.

And if it is not `master`, then I have to ask why you keep suggesting to
turn off the built-in rebase by default in `master`.

Ciao,
Johannes

P.S.: Maybe you have a hook you forgot to mention?

Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)

[Junio C Hamano]

Stefan Beller  writes:

>> * sb/submodule-recursive-fetch-gets-the-tip (2018-10-31) 11 commits
>> [...]
>>
>>  "git fetch --recurse-submodules" may not fetch the necessary commit
>>  that is bound to the superproject, which is getting corrected.
>>
>>  Is the discussion on this topic over?  What was the outcome?
>
> Please don't merge down the topic in the current state.

Thanks.  It won't happen until the final anyway ;-)

Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF

[Junio C Hamano]

Johannes Sixt  writes:

> Am 26.11.18 um 04:04 schrieb Junio C Hamano:
>>
>> That does not sound right.  I would understand it if both lines
>> showed ^M at the end, and only the one on the postimage line had it
>> highlighted as a trailing-whitespace.
>
> I agree that this is a (small?) weakness. But...
>
>> When we are producing a colored output, we know we are *not* writing
>> for machines, so one way to do it would be to turn CR at the end of
>> the line into two letter "^" "M" sequence on our end, without relying
>> on the terminal and/or the pager.  I dunno.
>
> ... this goes too far, IMO. It is the pager's task to decode control
> characters.

It was tongue-in-cheek suggestion to split a CR into caret-em on our
end, but we'd get essentially the same visual effect if we added a
rule:

When producing a colored output (not limited to whitespace
error coloring of diff output), insert  before a CR
that comes immediately before a LF.

Then, what Frank saw in the troublesome output would become

 -something  CR  LF
 +something_new   CR  LF

and we'll let the existing pager+terminal magic turn that trailing
CR on the preimage line into caret-em, just like the trailing CR on
the postimage line is already shown as caret-em with the current
output.

And a good thing is that I do not think that new rule is doing any
decode of control chars on our end.  We are just producing colored
output normally.

Re: [RFC] Introduce two new commands, switch-branch and restore-paths

[Stefan Beller]

On Mon, Nov 26, 2018 at 8:01 AM Ævar Arnfjörð Bjarmason
 wrote:
>
>
> On Tue, Nov 20 2018, Duy Nguyen wrote:
>
> > On Mon, Nov 19, 2018 at 04:19:53PM +0100, Duy Nguyen wrote:
> >> I promise to come back with something better (at least it still
> >> sounds better in my mind). If that idea does not work out, we can
> >> come back and see if we can improve this.
> >
> > So this is it. The patch isn't pretty, mostly as a proof of
> > concept. Just look at the three functions at the bottom of checkout.c,
> > which is the main thing.
> >
> > This patch tries to split "git checkout" command in two new ones:
> >
> > - git switch-branch is all about switching branches
>
> Isn't this going to also take the other ref arguments 'git checkout'
> takes now? I.e. tags, detached HEADs etc? I'm reminded of the discussion
> about what "range-diff" should be called :)

Heh, good call. :-)
Note that the color of a bikeshed has fewer functional implications
than coming up with proper names in your API exposed to millions
of users, as cognitive associations playing mind tricks, can have a
huge impact on their productivity. ;-)

In a neighboring thread there is discussion of the concept of a
'change' (and evolving the change locally), which is yet another
thing in the refs-space.

'switch-branch' sounds like a good name for a beginner who is just
getting started, but as soon as they discover that there is more than
branches (detached HEAD via commits, tags,
remote tracking "branches"), this name may be confusing.
So it would not be a good choice for the intermediate Git user.
The old time power user would not care as they have 'checkout'
in their muscle memory, aliased to 'co', but maybe they'd find
it nice for explaining to new users.

So I'd be thrilled to find a name that serves users on all levels.

Maybe we need to step back and consider what the command does.
And from that I would name it "rewire-HEAD-and-update-index"
and then simplify from there. For the beginner user, the concept of
HEAD might be overwhelming, such that we don't want to have that
in there.

So I'd be tempted to suggest to call it "switch-to-ref", but that would
be wrong in the corner case as well: When using that with a remote
tracking branch, you don't "switch to it" by putting it into your HEAD,
but you merely checkout the commit that it's pointing at.



>
> > - git restore-paths (maybe restore-file is better) for checking out
> >   paths

"content-to-path", maybe(?) as it moves the content (as given by commit
or implicitly assuming the index when omitted) into that path(, again).
(I am not enthused about this, as you can similarly argue for
content-to-paths, content-to-worktree, which then could split up into
"index-to-worktree [pathspec]" as well as "tree-to-worktree ".
also the notion of X-to-Y seems a novel concept in our naming, so maybe
verb-noun is better, hence restore-path or "fix-paths" may be better)

> > Later on, we could start to add a bit more stuff in, e.g. some form of
> > disambiguation is no longer needed when running as switch-branch, or
> > restore-paths.
> >
> > So, what do you think?

The patch looks interestingly small :-)

> That "git checkout" does too many things is something that keeps coming
> up in online discussions about Git's UI. Two things:
>
> a) It would really help to have some comparison of cases where these
>split commands are much clearer or less ambiguous than
>git-checkout. I can think of some (e.g. branch with the same name as
>a file) but having some overall picture of what the new UI looks like
>with solved / not solved cases would be nice. Also a comparison with
>other SCMs people find less confusing (svn, hg, bzr, ...)

How do other SCMs solve this issue? (What is their design space?
How many commands do they have for what git-checkout does
all-in-one?)

> b) I think we really need to have some end-game where we'd actually
>switch away from "checkout" (which we could still auto-route to new
>commands in perpetuity, but print a warning or error). Otherwise
>we'll just end up with https://xkcd.com/927/ and more UI confusion
>for all.

Heh, that situation is only avoided when the new command has clear
advantages over the old, and ISTM that we can only compete on
UX and better defaults, so maybe I'd push for making it more logical,
maybe so:

  git tree-to-worktree # git checkout  -- 
  git index-to-worktree # git checkout -- 
  git rev-to-ref # git checkout 

Just food for thought, specifically the last one would be
hilarious if we'd end up with it.

Stefan

Re: [ANNOUNCE] Git v2.20.0-rc1

[Johannes Schindelin]

Hi Ævar,

On Sat, 24 Nov 2018, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Nov 21 2018, Junio C Hamano wrote:
> 
> >  * "git rebase" and "git rebase -i" have been reimplemented in C.
> 
> Here's another regression in the C version (and rc1), note: the
> sha1collisiondetection is just a stand in for "some repo":
> 
> (
> rm -rf /tmp/repo &&
> git init /tmp/repo &&
> cd /tmp/repo &&
> for c in 1 2
> do
> touch $c &&
> git add $c &&
> git commit -m"add $c"
> done &&
> git remote add origin 
> https://github.com/cr-marcstevens/sha1collisiondetection.git &&
> git fetch &&
> git branch --set-upstream-to origin/master &&
> git rebase -i
> )
> 
> The C version will die with "fatal: unable to read tree
> ". Running this with
> rebase.useBuiltin=false does the right thing and rebases as of the merge
> base of the two (which here is the root of the history).

Sorry, this bug does not reproduce here:

$ git rebase -i
Successfully rebased and updated refs/heads/master.

> I wasn't trying to stress test rebase. I was just wanting to rebase a
> history I was about to force-push after cleaning it up, hardly an
> obscure use-case. So [repeat last transmission in
> https://public-inbox.org/git/87y39w1wc2@evledraar.gmail.com/ ]

Maybe you can give me the full details so that I can verify that this is
indeed a bug in the builtin C and not just a regression caused by some
random branches being merged together?

In short: please provide me with the exact URL and branch of your git.git
fork to test. Then please make sure to specify the precise revision of the
sha1collisiondetection/master rev, just in case that it matters.

Ideally, you would reduce the problem to a proper test case, say, for
t3412 (it seems that you try to rebase onto an unrelated history, so it is
*vaguely* related to "rebase-root").

Ciao,
Dscho

Re: [PATCH 2/5] ieot: default to not writing IEOT section

[Stefan Beller]

On Mon, Nov 26, 2018 at 1:48 PM Ben Peart  wrote:
>
>
>
> On 11/26/2018 2:59 PM, Stefan Beller wrote:
> >>> +static int record_ieot(void)
> >>> +{
> >>> + int val;
> >>> +
> >>
> >> Initialize stack val to zero to ensure proper default.
> >
> > I don't think that is needed here, as we only use `val` when
> > we first write to it via git_config_get_bool.
> >
> > Did you spot this via code review and thought of
> > defensive programming or is there a tool that
> > has a false positive here?
> >
>
> Code review and defensive programming.  I had to review the code in
> git_config_get_bool() to see if it always initialized the val even if it
> didn't find the requested config variable (esp since we don't pass in a
> default value for this function like we do others).
>

Ah, sorry to have sent out this email, which I found as one of the
earliest discussions in my mailbox. The later patches/discussions
became a lot more heated from my cursory skimming and sorted
out this as well.

It is interesting to notice that, as I also had to lookup how the config
machinery works (once? a couple times?) but now it is so hardcoded
in my brain to assume that if functions like git_config_* take the
branch, we can access the value that the config function was supposed
to read into.

Sorry for the noise,
Stefan

Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)

[Stefan Beller]

>
> * sb/submodule-recursive-fetch-gets-the-tip (2018-10-31) 11 commits
> [...]
>
>  "git fetch --recurse-submodules" may not fetch the necessary commit
>  that is bound to the superproject, which is getting corrected.
>
>  Is the discussion on this topic over?  What was the outcome?

Please don't merge down the topic in the current state.

I have a local updated version sitting on my disk and plan to send
it once I made sure it addressed all comments of various revisions.

Re: [PATCH 2/5] ieot: default to not writing IEOT section

[Ben Peart]

On 11/26/2018 2:59 PM, Stefan Beller wrote:

+static int record_ieot(void)
+{
+ int val;
+


Initialize stack val to zero to ensure proper default.


I don't think that is needed here, as we only use `val` when
we first write to it via git_config_get_bool.

Did you spot this via code review and thought of
defensive programming or is there a tool that
has a false positive here?



Code review and defensive programming.  I had to review the code in 
git_config_get_bool() to see if it always initialized the val even if it 
didn't find the requested config variable (esp since we don't pass in a 
default value for this function like we do others).





+ if (!git_config_get_bool("index.recordoffsettable", ))
+ return val;
+ return 0;
+}

Re: [PATCH v2 0/9] diff --color-moved-ws fixes and enhancment

[Stefan Beller]

On Fri, Nov 23, 2018 at 3:17 AM Phillip Wood  wrote:
>
> From: Phillip Wood 
>
> Thanks to Stefan for his feedback on v1. I've updated patches 2 & 8 in
> response to those comments - see the range-diff below for details (the
> patch numbers are off by one in the range diff, I think because the
> first patch is unchanged and so it was used as the merge base by
> --range-diff=.

`git range-diff` accepts a three dotted "range" OLD...NEW
as an easy abbreviation for the arguments
"COMMON..OLD COMMON..NEW" and the common element is
computed as the last common element. It doesn't have knowledge
about where you started your topic branch.


> For some reason the range-diff also includes
> the notes even though I did not give --notes to format-patch)

This is interesting.
The existence of notes.rewrite. seems to work well
with the range-diff then, as the config would trigger the copy-over
of notes and then range-diff would diff the original notes to the new
notes.

>
> When trying out the new --color-moved-ws=allow-indentation-change I
> was disappointed to discover it did not work if the indentation
> contains a mix of spaces and tabs. This series reworks it so that it
> does.
>

The range-diff looks good to me.

Thanks,
Stefan

Re: [PATCH v1 1/2] log -G: Ignore binary files

[Stefan Beller]

On Wed, Nov 21, 2018 at 1:08 PM Thomas Braun
 wrote:
>
> The -G  option of log looks for the differences whose patch text
> contains added/removed lines that match regex.
>
> The concept of differences only makes sense for text files, therefore
> we need to ignore binary files when searching with -G  as well.

What about partial text/partial binary files?

I recall using text searching tools (not necessarily git machinery,
my memory is fuzzy) to check for strings in pdf files, which are
usually marked binary in context of git, such that we do not
see their diffs in `log -p`.

But I would expect a search with -G or -S to still work...
until I find the exception in the docs, only to wonder if
there is a switch to turn off this optimisation for this
corner case.

Stefan

[PATCH] t/lib-git-daemon: fix signal checking

[SZEDER Gábor]

Test scripts checking 'git daemon' stop the daemon with a TERM signal,
and the 'stop_git_daemon' helper checks the daemon's exit status to
make sure that it indeed died because of that signal.

This check is bogus since 03c39b3458 (t/lib-git-daemon: use
test_match_signal, 2016-06-24), for two reasons:

  - Right after killing 'git daemon', 'stop_git_daemon' saves its exit
status in a variable, but since 03c39b3458 the condition checking
the exit status looks at '$?', which at this point is not the exit
status of 'git daemon', but that of the variable assignment, i.e.
it's always 0.

  - The unexpected exit status should abort the whole test script with
'error', but it doesn't, because 03c39b3458 forgot to negate
'test_match_signal's exit status in the condition.

This patch fixes both issues.

Signed-off-by: SZEDER Gábor 
---
 t/lib-git-daemon.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index edbea2d986..f98de95c15 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -92,7 +92,7 @@ stop_git_daemon() {
kill "$GIT_DAEMON_PID"
wait "$GIT_DAEMON_PID" >&3 2>&4
ret=$?
-   if test_match_signal 15 $?
+   if ! test_match_signal 15 $ret
then
error "git daemon exited with status: $ret"
fi
-- 
2.20.0.rc1.149.g55c2c038c2

Re: [PATCH 2/5] ieot: default to not writing IEOT section

[Stefan Beller]

> > +static int record_ieot(void)
> > +{
> > + int val;
> > +
>
> Initialize stack val to zero to ensure proper default.

I don't think that is needed here, as we only use `val` when
we first write to it via git_config_get_bool.

Did you spot this via code review and thought of
defensive programming or is there a tool that
has a false positive here?

>
> > + if (!git_config_get_bool("index.recordoffsettable", ))
> > + return val;
> > + return 0;
> > +}

[PATCH] transport-helper.c: do not translate a string twice

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

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 My bad.

 transport-helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport-helper.c b/transport-helper.c
index 7213fa0d32..bf225c698f 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -573,7 +573,7 @@ static int run_connect(struct transport *transport, struct 
strbuf *cmdbuf)
fprintf(stderr, "Debug: Falling back to dumb "
"transport.\n");
} else {
-   die(_(_("unknown response to connect: %s")),
+   die(_("unknown response to connect: %s"),
cmdbuf->buf);
}
 
-- 
2.19.1.1327.g328c130451.dirty

Re: [PATCH] grep: use grep_opt->repo instead of explict repo argument

[Stefan Beller]

On Sun, Nov 18, 2018 at 8:38 AM Nguyễn Thái Ngọc Duy  wrote:
>
> This command is probably the first one that operates on a repository
> other than the_repository, in f9ee2fcdfa (grep: recurse in-process
> using 'struct repository' - 2017-08-02). An explicit 'struct
> repository *' was added in that commit to pass around the repository
> that we're supposed to grep from.
>
> Since 38bbc2ea39 (grep.c: remove implicit dependency on the_index -
> 2018-09-21). 'struct grep_opt *' carries in itself a repository
> parameter for grepping. We should now be able to reuse grep_opt to
> hold the submodule repo instead of a separate argument, which is just
> room for mistakes.

That makes sense. Assuming we did not make a mistake yet, the
test suite would not need changes (further assuming we do test for it,
but we do as we have explicit submodule grep tests).

>
> While at there, use the right reference instead of the_repository and
> the_index in this code. I was a bit careless in my attempt to kick
> the_repository / the_index out of library code. It's normally safe to
> just stick the_repository / the_index in bultin/ code, but it's not
> the case for grep.

Reviewed-by: Stefan Beller 

> Signed-off-by: Nguyễn Thái Ngọc Duy 

Re: BUG: CR marker ^M doesn't show up in '-' lines of diffs when the ending of the removed line is CR+LF

[Johannes Sixt]

Am 26.11.18 um 04:04 schrieb Junio C Hamano:

It appears to me that what Frank sees is not "^M highlighted for
whitespace breakage appears only on postimage lines, while ^M is
shown but not highlighted on preimage lines".  My reading of the
above is "Not just without highlight, ^M is just *NOT* shown on the
preimage line".

That does not sound right.  I would understand it if both lines
showed ^M at the end, and only the one on the postimage line had it
highlighted as a trailing-whitespace.


I agree that this is a (small?) weakness. But...


When we are producing a colored output, we know we are *not* writing
for machines, so one way to do it would be to turn CR at the end of
the line into two letter "^" "M" sequence on our end, without relying
on the terminal and/or the pager.  I dunno.


... this goes too far, IMO. It is the pager's task to decode control 
characters.


-- Hannes

[PATCH v2 1/2] Introduce "precious" file concept

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

A new attribute "precious" is added to indicate that certain files
have valuable content and should not be easily discarded even if they
are ignored or untracked.

So far there are two parts of Git that are made aware of precious
files: "git clean" will leave precious files alone and unpack-trees.c
(i.e. merges and branch switches) will not overwrite
ignored-but-precious files.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-clean.txt |  3 ++-
 Documentation/gitattributes.txt | 13 +
 Documentation/gitignore.txt |  4 
 attr.c  | 12 
 attr.h  |  2 ++
 builtin/clean.c | 20 +---
 t/t1004-read-tree-m-u-wf.sh |  6 ++
 t/t7300-clean.sh| 29 +
 unpack-trees.c  |  1 +
 9 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 03056dad0d..a9beadfb12 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -21,7 +21,8 @@ option is specified, ignored files are also removed. This 
can, for
 example, be useful to remove all build products.
 
 If any optional `...` arguments are given, only those paths
-are affected.
+are affected. Ignored or untracked files with `precious` attributes
+are not removed.
 
 OPTIONS
 ---
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index b8392fc330..b027abea4a 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -1188,6 +1188,19 @@ If this attribute is not set or has an invalid value, 
the value of the
 (See linkgit:git-config[1]).
 
 
+Precious files
+~~
+
+`precious`
+^^
+
+This attribute is set on files to indicate that their content is
+valuable. Many commands will behave slightly different on precious
+files. linkgit:git-clean[1] will leave precious files alone. Merging
+and branch switching will not silently overwrite ignored files that
+are marked "precious".
+
+
 USING MACRO ATTRIBUTES
 --
 
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 1c94f08ff4..0e9614289e 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -141,6 +141,10 @@ not tracked by Git remain untracked.
 To stop tracking a file that is currently tracked, use
 'git rm --cached'.
 
+Ignored files are generally considered discardable. See `precious`
+attribute in linkgit:gitattributes[5] to change the behavior regarding
+ignored files.
+
 EXAMPLES
 
 
diff --git a/attr.c b/attr.c
index eaece6658d..b07d8cd835 100644
--- a/attr.c
+++ b/attr.c
@@ -1172,3 +1172,15 @@ void attr_start(void)
pthread_mutex_init(_attr_hashmap.mutex, NULL);
pthread_mutex_init(_vector.mutex, NULL);
 }
+
+int is_precious_file(struct index_state *istate, const char *path)
+{
+   static struct attr_check *check;
+   if (!check)
+   check = attr_check_initl("precious", NULL);
+   if (!check)
+   return 0;
+
+   git_check_attr(istate, path, check);
+   return ATTR_TRUE(check->items[0].value);
+}
diff --git a/attr.h b/attr.h
index b0378bfe5f..b9a9751a66 100644
--- a/attr.h
+++ b/attr.h
@@ -82,4 +82,6 @@ void git_attr_set_direction(enum git_attr_direction 
new_direction);
 
 void attr_start(void);
 
+int is_precious_file(struct index_state *istate, const char *path);
+
 #endif /* ATTR_H */
diff --git a/builtin/clean.c b/builtin/clean.c
index bbcdeb2d9e..42cd040849 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -17,6 +17,7 @@
 #include "color.h"
 #include "pathspec.h"
 #include "help.h"
+#include "attr.h"
 
 static int force = -1; /* unset */
 static int interactive;
@@ -30,6 +31,8 @@ static const char *const builtin_clean_usage[] = {
 
 static const char *msg_remove = N_("Removing %s\n");
 static const char *msg_would_remove = N_("Would remove %s\n");
+static const char *msg_skip_precious = N_("Skipping precious file %s\n");
+static const char *msg_would_skip_precious = N_("Would skip precious file 
%s\n");
 static const char *msg_skip_git_dir = N_("Skipping repository %s\n");
 static const char *msg_would_skip_git_dir = N_("Would skip repository %s\n");
 static const char *msg_warn_remove_failed = N_("failed to remove %s");
@@ -153,6 +156,7 @@ static int remove_dirs(struct strbuf *path, const char 
*prefix, int force_flag,
struct dirent *e;
int res = 0, ret = 0, gone = 1, original_len = path->len, len;
struct string_list dels = STRING_LIST_INIT_DUP;
+   const char *rel_path;
 
*dir_gone = 1;
 
@@ -192,9 +196,16 @@ static int remove_dirs(struct strbuf *path, const char 
*prefix, int force_flag,
 
strbuf_setlen(path, len);
strbuf_addstr(path, e->d_name);
-   if (lstat(path->buf, ))
+   if (lstat(path->buf, )) {
; /* fall thru 

[PATCH v2 2/2] unpack-trees: support core.allIgnoredFilesArePreciousWhenMerging

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

Ignored files can be marked precious to prevent being overwritten
during a merge (or even a branch switch). If you really want to make
sure no ignored files are overwritten, this config variable is for
you.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/config/core.txt |  6 ++
 Documentation/gitignore.txt   |  5 +++--
 unpack-trees.c| 21 -
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index d0e6635fe0..bff5834c13 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -1,3 +1,9 @@
+core.allIgnoredFilesArePreciousWhenMerging::
+   During a merge operation, if "precious" attribute is unset,
+   consider it set. You can explicitly remove "precious"
+   attribute if needed. See linkgit:gitattributes[5] for more
+   information.
+
 core.fileMode::
Tells Git if the executable bit of files in the working tree
is to be honored.
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 0e9614289e..2832df7178 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -142,8 +142,9 @@ To stop tracking a file that is currently tracked, use
 'git rm --cached'.
 
 Ignored files are generally considered discardable. See `precious`
-attribute in linkgit:gitattributes[5] to change the behavior regarding
-ignored files.
+attribute in linkgit:gitattributes[5] and
+`core.allIgnoredFilesArePreciousWhenMerging` in linkgit:git-config[1]
+to change the behavior regarding ignored files.
 
 EXAMPLES
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 9a5aadc084..df3b163e2e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1877,6 +1877,25 @@ static int icase_exists(struct unpack_trees_options *o, 
const char *name, int le
return src && !ie_match_stat(o->src_index, src, st, 
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
 }
 
+static int is_precious_ignored_file(struct index_state *istate, const char 
*path)
+{
+   static struct attr_check *check;
+   int all_precious;
+
+   if (!check)
+   check = attr_check_initl("precious", NULL);
+   if (!check)
+   return 0;
+
+   git_check_attr(istate, path, check);
+   if (ATTR_UNSET(check->items[0].value) &&
+   !git_config_get_bool("core.allignoredfilesarepreciouswhenmerging",
+_precious) &&
+   all_precious)
+   return 1;
+   return ATTR_TRUE(check->items[0].value);
+}
+
 static int check_ok_to_remove(const char *name, int len, int dtype,
  const struct cache_entry *ce, struct stat *st,
  enum unpack_trees_error_types error_type,
@@ -1895,7 +1914,7 @@ static int check_ok_to_remove(const char *name, int len, 
int dtype,
return 0;
 
if (o->dir &&
-   !is_precious_file(o->src_index, name) &&
+   !is_precious_ignored_file(o->src_index, name) &&
is_excluded(o->dir, o->src_index, name, ))
/*
 * ce->name is explicitly excluded, so it is Ok to
-- 
2.19.1.1327.g328c130451.dirty

[PATCH v2 0/2] Precios files round two

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

Let's see if I can make everybody almost happy with this.

Patch 1 is not that much different from round one except now with
tests. It is still about "precious" attributes that prevent content
loss with "git clean", "git checkout " or "git merge".

Patch 2 goes Ævar and Per are pursuing. It adds an opt-in config that
turns all ignored files precious, but only for unpack-trees
operations.

Nguyễn Thái Ngọc Duy (2):
  Introduce "precious" file concept
  unpack-trees: support core.allIgnoredFilesArePreciousWhenMerging

 Documentation/config/core.txt   |  6 ++
 Documentation/git-clean.txt |  3 ++-
 Documentation/gitattributes.txt | 13 +
 Documentation/gitignore.txt |  5 +
 attr.c  | 12 
 attr.h  |  2 ++
 builtin/clean.c | 20 +---
 t/t1004-read-tree-m-u-wf.sh |  6 ++
 t/t7300-clean.sh| 29 +
 unpack-trees.c  | 20 
 10 files changed, 112 insertions(+), 4 deletions(-)

-- 
2.19.1.1327.g328c130451.dirty

Re: Did You Receive My Last Mail?

[Reem Al-Hashimi]

Hello,

My name is ms. Reem Al-Hashimi. The UAE minister of state for international 
cooparation. I got your contact from an email database from your country. I 
have a financial transaction i would like to discuss with you. Please reply to 
reem2...@daum.net, for more details if you are interested.

Regards,

Ms. Reem Al-Hashimi

Re: [PATCH v16 Part II 2/8] bisect--helper: `bisect_write` shell function in C

[Johannes Schindelin]

Hi Martin,

On Fri, 23 Nov 2018, Martin Ågren wrote:

> On Fri, 23 Nov 2018 at 11:13, Johannes Schindelin
>  wrote:
> > On Mon, 30 Oct 2017, Pranit Bauva wrote:
> >
> > > On Fri, Oct 27, 2017 at 10:58 PM, Martin Ågren  
> > > wrote:
> > > > On 27 October 2017 at 17:06, Pranit Bauva  
> > > > wrote:
> > > >> +static void free_terms(struct bisect_terms *terms)
> > > >> +{
> > > >> +   if (!terms->term_good)
> > > >> +   free((void *) terms->term_good);
> > > >> +   if (!terms->term_bad)
> > > >> +   free((void *) terms->term_bad);
> > > >> +}
> 
> > > > You leave the pointers dangling, but you're ok for now since this is the
> > > > last thing that happens in `cmd_bisect__helper()`. Your later patches
> > > > add more users, but they're also ok, since they immediately assign new
> > > > values.
> > > >
> > > > In case you (and others) find it useful, the below is a patch I've been
> > > > sitting on for a while as part of a series to plug various memory-leaks.
> > > > `FREE_AND_NULL_CONST()` would be useful in precisely these situations.
> > >
> > > Honestly, I wouldn't be the best person to judge this.
> >
> > Git's source code implicitly assumes that any `const` pointer refers to
> > memory owned by another code path. It is therefore probably not a good
> > idea to encourage `free()`ing a `const` pointer.
> 
> Yeah, I never submitted that patch as part of a real series. I remember
> having a funky feeling about it, and whatever use-case I had for this
> macro, I managed to solve the memory leak in some other way in a
> rewrite.
> 
> Thanks for a sanity check.

I am glad you agree, and it's just fair that I contribute a sanity check
on this here list when I have benefitted so many times from the same.

Ciao,
Johannes

Re: What's cooking in git.git (Nov 2018, #06; Wed, 21)

[Jeff King]

On Sun, Nov 25, 2018 at 11:02:05AM +0900, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > I do also think in the long run we should be fixing the "unreachable
> > always become loose" issues.
> 
> I think I've seen an idea of collecting them into a garbage pack
> floated for at least a few times here.  What are the downsides?  We
> no longer will know when these unreachable ones were last accessed
> individually so we need to come up with a different policy around
> their expiration?  As the common traits among objects in such a
> garbage pack (iow the way we discover the objects that need to be
> placed in there) does not involve path information and we lose the
> ability to compress them well?

Yes, the main issue is handling the expiration/mtime.

We may lose some input to the delta heuristics, but:

  - the current alternative is non-delta loose objects (so just shoving
those in a pack is no worse for disk space, and probably better
because of less inode/file overhead)

  - if they were already packed we can often just retain the existing
deltas (and we get this basically for free with the existing code)

  - we could still walk unreachable bits of the graph, starting at
dangling commits, to find the path information (we do this to some
degree already to avoid dropping objects depended on by "unreachable
but recent" objects, but I don't know how systematic we are about
making sure to hit walk down from root trees first)

The most thorough discussion I know of in this direction is the one
around:

  
https://public-inbox.org/git/20170610080626.sjujpmgkli4mu...@sigill.intra.peff.net/

-Peff

[PATCH v1/RFC 1/1] 'git clone C:\cygwin\home\USER\repo' is working (again)

[tboegi]

From: Torsten Bögershausen 

A regression for cygwin users was introduced with commit 05b458c,
 "real_path: resolve symlinks by hand".

In the the commit message we read:
  The current implementation of real_path uses chdir() in order to resolve
  symlinks.  Unfortunately this isn't thread-safe as chdir() affects a
  process as a whole...

The old (and non-thread-save) OS calls chdir()/pwd() had been
replaced by a string operation.
The cygwin layer "knows" that "C:\cygwin" is an absolute path,
but the new string operation does not.

"git clone  C:\cygwin\home\USER\repo" fails like this:
fatal: Invalid path '/home/USER/repo/C:\cygwin\home\USER\repo'

The solution is to implement has_dos_drive_prefix(), skip_dos_drive_prefix()
is_dir_sep(), offset_1st_component() and convert_slashes() for cygwin
in the same way as it is done in 'Git for Windows' in compat/mingw.[ch]

Reported-By: Steven Penny 
Signed-off-by: Torsten Bögershausen 
---

This is the first vesion of a patch.
Is there a chance that you test it ?

abspath.c   |  2 +-
 compat/cygwin.c | 18 ++
 compat/cygwin.h | 32 
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/abspath.c b/abspath.c
index 9857985329..77a281f789 100644
--- a/abspath.c
+++ b/abspath.c
@@ -55,7 +55,7 @@ static void get_root_part(struct strbuf *resolved, struct 
strbuf *remaining)
 
strbuf_reset(resolved);
strbuf_add(resolved, remaining->buf, offset);
-#ifdef GIT_WINDOWS_NATIVE
+#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
convert_slashes(resolved->buf);
 #endif
strbuf_remove(remaining, 0, offset);
diff --git a/compat/cygwin.c b/compat/cygwin.c
index b9862d606d..c4a10cb5a1 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,19 +1,29 @@
 #include "../git-compat-util.h"
 #include "../cache.h"
 
+int cygwin_skip_dos_drive_prefix(char **path)
+{
+   int ret = has_dos_drive_prefix(*path);
+   *path += ret;
+   return ret;
+}
+
 int cygwin_offset_1st_component(const char *path)
 {
-   const char *pos = path;
+   char *pos = (char *)path;
+
/* unc paths */
-   if (is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
+   if (!skip_dos_drive_prefix() &&
+   is_dir_sep(pos[0]) && is_dir_sep(pos[1])) {
/* skip server name */
-   pos = strchr(pos + 2, '/');
+   pos = strpbrk(pos + 2, "\\/");
if (!pos)
return 0; /* Error: malformed unc path */
 
do {
pos++;
-   } while (*pos && pos[0] != '/');
+   } while (*pos && !is_dir_sep(*pos));
}
+
return pos + is_dir_sep(*pos) - path;
 }
diff --git a/compat/cygwin.h b/compat/cygwin.h
index 8e52de4644..46f29c0a90 100644
--- a/compat/cygwin.h
+++ b/compat/cygwin.h
@@ -1,2 +1,34 @@
+#define has_dos_drive_prefix(path) \
+   (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
+
+
+int cygwin_offset_1st_component(const char *path);
+#define offset_1st_component cygwin_offset_1st_component
+
+
+#define has_dos_drive_prefix(path) \
+   (isalpha(*(path)) && (path)[1] == ':' ? 2 : 0)
+int cygwin_skip_dos_drive_prefix(char **path);
+#define skip_dos_drive_prefix cygwin_skip_dos_drive_prefix
+static inline int cygwin_is_dir_sep(int c)
+{
+   return c == '/' || c == '\\';
+}
+#define is_dir_sep cygwin_is_dir_sep
+static inline char *cygwin_find_last_dir_sep(const char *path)
+{
+   char *ret = NULL;
+   for (; *path; ++path)
+   if (is_dir_sep(*path))
+   ret = (char *)path;
+   return ret;
+}
+static inline void convert_slashes(char *path)
+{
+   for (; *path; path++)
+   if (*path == '\\')
+   *path = '/';
+}
+#define find_last_dir_sep cygwin_find_last_dir_sep
 int cygwin_offset_1st_component(const char *path);
 #define offset_1st_component cygwin_offset_1st_component
-- 
2.19.0.271.gfe8321ec05

Re: [PATCH v2 0/4] Port git-add--interactive.perl:status_cmd to C

[Slavica Djukic]

Hi Daniel,

On 16-May-17 6:00 AM, Daniel Ferreira wrote:

This is the second version of a patch series to start porting
git-add--interactive from Perl to C.

Series:
v1: 
https://public-inbox.org/git/1494009820-2090-1-git-send-email-bnm...@gmail.com/

Travis CI build: https://travis-ci.org/theiostream/git/builds/232669894

I have applied most of the changes suggested by Johannes and Ævar (thank
you!). The one exception was Johannes' request to make this a WIP patch
series with a sort-of "design" of what's next to come. I did not do
this because I still have no clue...! I'll begin experimenting on
update_cmd() to see if I gain some insight on this issue that I could
bring to this series. I do think this would be a good idea, though! :)



I am Slavica Đukic, Outreachy intern for Git (Dec-March 2018/19 round).

Project I'll be working on is "Turn git add -i to built-in".

Would you mind if I use your work so far as head start?




-- Daniel.

Daniel Ferreira (4):
   diff: export diffstat interface
   add--helper: create builtin helper for interactive add
   add--helper: implement interactive status command
   add--interactive: use add-interactive--helper for status_cmd

  .gitignore|   1 +
  Makefile  |   1 +
  builtin.h |   1 +
  builtin/add--helper.c | 285 ++
  diff.c|  17 +--
  diff.h|  19 +++-
  git-add--interactive.perl |   4 +-
  git.c |   1 +
  8 files changed, 309 insertions(+), 20 deletions(-)
  create mode 100644 builtin/add--helper.c

--
2.7.4 (Apple Git-66)




Thank you,
Slavica

Re: t5570 shaky for anyone ?

[Jeff King]

On Sun, Nov 25, 2018 at 11:22:47PM +0100, SZEDER Gábor wrote:

> > The following fixes it, but I am not sure if this is the ideal
> > solution.
> 
> Currently this is the only test that looks at 'daemon.log', but if we
> ever going to add another one, then that will be prone to the same
> issue.
> 
> I wonder whether it's really that useful to have the daemon log in the
> test script's output...  if the log was sent directly to daemon log,
> then the window for this race would be smaller, but still not
> completely closed.
> 
> Anyway, I added Peff to Cc, since he added that whole fifo-shell-loop
> thing.

Yeah, see my comments on the other part of the thread. If we did write
directly to a file, I think that would be enough here because git-daemon
writes this entry before running the sub-process. So by the time
ls-remote finishes, we know that it talked to upload-pack, and we know
that before upload-pack was run, git-daemon wrote the log entry. That
assumes git-daemon doesn't buffer its logs (but if it does, we should
probably fix that).

-Peff

Re: t5570 shaky for anyone ?

[Jeff King]

On Sun, Nov 25, 2018 at 10:01:38PM +, Thomas Gummerer wrote:

> On 11/25, Torsten Bögershausen wrote:
> > After running the  "Git 2.20-rc1" testsuite here on a raspi,
> > the only TC that failed was t5570.
> > When the "grep" was run on daemon.log, the file was empty (?).
> > When inspecting it later, it was filled, and grep would have found
> > the "extended.attribute" it was looking for.
> 
> I believe this has been reported before in
> https://public-inbox.org/git/1522783990.964448.1325338528.0d49c...@webmail.messagingengine.com/,
> but it seems like the thread never ended with actually fixing it.
> Reading the first reply Peff seemed to be fine with just removing the
> test completely, which would be the easiest solution ;)  Adding him to
> Cc: here.

Yes, I don't think there is a way to make this race-proof without
somehow convincing "cat" to flush (and let us know when it has). Which
really implies killing the daemon, and wait()ing on cat to process the
EOF and exit.  And that makes the tests a lot more expensive if we have
to start the daemon for each snippet.

So I'm still fine with just dropping this test.

> > diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
> > index 7466aad111..e259fee0ed 100755
> > --- a/t/t5570-git-daemon.sh
> > +++ b/t/t5570-git-daemon.sh
> > @@ -192,6 +192,7 @@ test_expect_success 'daemon log records all attributes' 
> > '
> > GIT_OVERRIDE_VIRTUAL_HOST=localhost \
> > git -c protocol.version=1 \
> > ls-remote "$GIT_DAEMON_URL/interp.git" &&
> > +   sleep 1 &&
> > grep -i extended.attribute daemon.log | cut -d" " -f2- >actual &&
> > test_cmp expect actual
> >  '
> > 
> > A slightly better approach may be to use a "sleep on demand":
> > 
> > +   ( grep -i -q extended.attribute daemon.log || sleep 1 ) &&

That doesn't really fix it, but just broadens the race window. I dunno.
Maybe that is enough in practice. We could do something like:

  repeat_with_timeout () {
local i=0
while test $i -lt 10
do
"$@" && return 0
sleep 1
done
# no success even after 10 seconds
return 1
  }

  repeat_with_timeout grep -i extended.attribute daemon.log

to make the pattern a bit more obvious (and make it easy to extend the
window arbitrarily; surely 10s is enough?).

-Peff

Re: [RFC] Introduce two new commands, switch-branch and restore-paths

[Duy Nguyen]

On Mon, Nov 26, 2018 at 5:01 PM Ævar Arnfjörð Bjarmason
 wrote:
> > So, what do you think?
>
> That "git checkout" does too many things is something that keeps coming
> up in online discussions about Git's UI. Two things:
>
> a) It would really help to have some comparison of cases where these
>split commands are much clearer or less ambiguous than
>git-checkout. I can think of some (e.g. branch with the same name as
>a file) but having some overall picture of what the new UI looks like
>with solved / not solved cases would be nice. Also a comparison with
>other SCMs people find less confusing (svn, hg, bzr, ...)

Less ambiguous is indeed one of the reasons I wanted to do this.

> b) I think we really need to have some end-game where we'd actually
>switch away from "checkout" (which we could still auto-route to new
>commands in perpetuity, but print a warning or error). Otherwise
>we'll just end up with https://xkcd.com/927/ and more UI confusion
>for all.

I'm not going to remove "git checkout". Not until the majority of
users are really in favor of the new ones. So my end-game plan is to
just promote the two new commands in man pages, tutorial, advice...
Perhaps at some point "git checkout" is also removed from "git help".
But that's about it. If you want to stick with "git checkout", it's
there (and not nagging you to move to new ones).
-- 
Duy

Re: [RFC PATCH] Introduce "precious" file concept

[Eckhard Maaß]

On Mon, Nov 12, 2018 at 11:22:09PM +, brian m. carlson wrote:
> This is going to totally hose automation.  My last job had files which
> might move from tracked to untracked (a file that had become generated),
> and long-running CI and build systems would need to be able to check out
> one status and switch to the other.  Your proposed change will prevent
> those systems from working, whereas they previously did.

Wouldn't those systems not use -f right now? And shouldn't Git have the
same semantic for -f to clobber everything in the proposed use case?
Like it does right now for untracked files which are not ignored. So to
be save going back and forth I would expect those systems to use -f
anyway. Have I missed something here?

Regards,
Eckhard

Re: [RFC] Introduce two new commands, switch-branch and restore-paths

[Ævar Arnfjörð Bjarmason]

On Tue, Nov 20 2018, Duy Nguyen wrote:

> On Mon, Nov 19, 2018 at 04:19:53PM +0100, Duy Nguyen wrote:
>> I promise to come back with something better (at least it still
>> sounds better in my mind). If that idea does not work out, we can
>> come back and see if we can improve this.
>
> So this is it. The patch isn't pretty, mostly as a proof of
> concept. Just look at the three functions at the bottom of checkout.c,
> which is the main thing.
>
> This patch tries to split "git checkout" command in two new ones:
>
> - git switch-branch is all about switching branches

Isn't this going to also take the other ref arguments 'git checkout'
takes now? I.e. tags, detached HEADs etc? I'm reminded of the discussion
about what "range-diff" should be called :)

> - git restore-paths (maybe restore-file is better) for checking out
>   paths

If it takes globs/dirs then a plural is probably better.

> The main idea is these two commands will co-exist with the good old
> 'git checkout', which will NOT be deprecated. Old timers will still
> use "git checkout". But new people should be introduced to the new two
> instead. And the new ones are just as capable as "git checkout".
>
> Since the three commands will co-exist (with duplicate functionality),
> maintenance cost must be kept to minimum. The way I did this is simply
> split the command line options into three pieces: common,
> switch-branch and checkout-paths. "git checkout" has all three, the
> other two have common and another piece.
>
> With this, a new option added to git checkout will be automatically
> available in either switch-branch or checkout-paths. Bug fixes apply
> to all relevant commands.
>
> Later on, we could start to add a bit more stuff in, e.g. some form of
> disambiguation is no longer needed when running as switch-branch, or
> restore-paths.
>
> So, what do you think?

That "git checkout" does too many things is something that keeps coming
up in online discussions about Git's UI. Two things:

a) It would really help to have some comparison of cases where these
   split commands are much clearer or less ambiguous than
   git-checkout. I can think of some (e.g. branch with the same name as
   a file) but having some overall picture of what the new UI looks like
   with solved / not solved cases would be nice. Also a comparison with
   other SCMs people find less confusing (svn, hg, bzr, ...)

b) I think we really need to have some end-game where we'd actually
   switch away from "checkout" (which we could still auto-route to new
   commands in perpetuity, but print a warning or error). Otherwise
   we'll just end up with https://xkcd.com/927/ and more UI confusion
   for all.

Re: [RFC PATCH] Introduce "precious" file concept

[Duy Nguyen]

On Mon, Nov 26, 2018 at 4:47 PM Ævar Arnfjörð Bjarmason
 wrote:
> >> >> How about something like this:
> >> >>
> >> >> 1. Introduce a concept with "garbage" files, which git is "permitted to
> >> >> delete" without prompting.
> >> >>
> >> >> 2. Retain the current default, i.e. "ignored files are garbage" for now,
> >> >> making the new behavior _opt in_ to avoid breaking automated
> >> >> systems/existing scripts for anyone. Put the setting for this behind a
> >> >> new core.* config flag.
> >> >>
> >> >> 3. In the plan for version 3.0 (a new major version where some breakage
> >> >> can be tolerable, according to Semantic Versioning), change the default
> >> >> so that "only explicit garbage is garbage". Include very clear notices
> >> >> of this in the release notes. The config flag is retained, but its
> >> >> default changes from true->false or vice versa. People who dislike the
> >> >> new behavior can easily change back to the 2.x semantics.
> >> >
> >> > How does this garbage thing interact with "git clean -x"? My
> >> > interpretation of this flag/attribute is that at version 3.0 by
> >> > default all ignored files are _not_ garbage, so "git clean -x" should
> >> > not remove any of them. Which is weird because most of ignored files
> >> > are like *.o that should be removed.
> >> >
> >> > I also need to mark "precious" on untracked or even tracked files (*).
> >> > Not sure how this "garbage" attribute interacts with that.
> >> >
> >> > (*) I was hoping I could get the idea [1] implemented in somewhat good
> >> > shape before presenting here. But I'm a bit slow on that front. So
> >> > yeah this "precious" on untracked/tracked thingy may be even
> >> > irrelevant if the patch series will be rejected.
> >>
> >> I think a garbage (or trashable) flag, if implemented, wouldn't need any
> >> special case in git-clean, i.e. -x would remove all untracked files,
> >> whether ignored or garbage/trashable. That's what my patch to implement
> >> it does:
> >> https://public-inbox.org/git/87zhuf3gs0@evledraar.gmail.com/
> >>
> >> I think that makes sense. Users running "git clean" have "--dry-run" and
> >> unlike "checkout a branch" or "merge this commit" where we'll now shred
> >> data implicitly it's obvious that git-clean is going to shred your data.
> >
> > Then that't not what I want. If I'm going to mark to keep "config.mak"
> > around, I'm not going to carefully move it away before doing "git
> > clean -fdx" then move it back. No "git clean --dry-run" telling me to
> > make a backup of config.mak is no good.
>
> Understood. I mean this in the context of solving the problem users have
> with running otherwise non-data-destroying commands like "checkout" and
> "merge" and getting their data destroyed, which is overwhelmingly why
> this topic gets resurrected.
>
> Some of the solutions overlap with this thing you want, but I think it's
> worth keeping the distinction between the two in mind.

On the other hand all use cases should be considered. It's going to be
a mess to have "trashable" attribute that applies to some commands
while "precious" to some others (and even worse when they overlap,
imagine having to define both in .gitattributes)

> I.e. I can
> imagine us finding some acceptable solution to the data shredding
> problem that doesn't implement this mode for "git-clean", or the other
> way around.
-- 
Duy

Re: [RFC PATCH] Introduce "precious" file concept

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 26 2018, Duy Nguyen wrote:

> On Mon, Nov 26, 2018 at 4:34 PM Ævar Arnfjörð Bjarmason
>  wrote:
>>
>>
>> On Mon, Nov 26 2018, Duy Nguyen wrote:
>>
>> > On Mon, Nov 26, 2018 at 10:30 AM Per Lundberg  
>> > wrote:
>> >>
>> >> On 11/13/18 1:22 AM, brian m. carlson wrote:
>> >> > This is going to totally hose automation.  My last job had files which
>> >> > might move from tracked to untracked (a file that had become generated),
>> >> > and long-running CI and build systems would need to be able to check out
>> >> > one status and switch to the other.  Your proposed change will prevent
>> >> > those systems from working, whereas they previously did.
>> >> >
>> >> > I agree that your proposal would have been a better design originally,
>> >> > but breaking the way automated systems currently work is probably going
>> >> > to be a dealbreaker.
>> >>
>> >> How about something like this:
>> >>
>> >> 1. Introduce a concept with "garbage" files, which git is "permitted to
>> >> delete" without prompting.
>> >>
>> >> 2. Retain the current default, i.e. "ignored files are garbage" for now,
>> >> making the new behavior _opt in_ to avoid breaking automated
>> >> systems/existing scripts for anyone. Put the setting for this behind a
>> >> new core.* config flag.
>> >>
>> >> 3. In the plan for version 3.0 (a new major version where some breakage
>> >> can be tolerable, according to Semantic Versioning), change the default
>> >> so that "only explicit garbage is garbage". Include very clear notices
>> >> of this in the release notes. The config flag is retained, but its
>> >> default changes from true->false or vice versa. People who dislike the
>> >> new behavior can easily change back to the 2.x semantics.
>> >
>> > How does this garbage thing interact with "git clean -x"? My
>> > interpretation of this flag/attribute is that at version 3.0 by
>> > default all ignored files are _not_ garbage, so "git clean -x" should
>> > not remove any of them. Which is weird because most of ignored files
>> > are like *.o that should be removed.
>> >
>> > I also need to mark "precious" on untracked or even tracked files (*).
>> > Not sure how this "garbage" attribute interacts with that.
>> >
>> > (*) I was hoping I could get the idea [1] implemented in somewhat good
>> > shape before presenting here. But I'm a bit slow on that front. So
>> > yeah this "precious" on untracked/tracked thingy may be even
>> > irrelevant if the patch series will be rejected.
>>
>> I think a garbage (or trashable) flag, if implemented, wouldn't need any
>> special case in git-clean, i.e. -x would remove all untracked files,
>> whether ignored or garbage/trashable. That's what my patch to implement
>> it does:
>> https://public-inbox.org/git/87zhuf3gs0@evledraar.gmail.com/
>>
>> I think that makes sense. Users running "git clean" have "--dry-run" and
>> unlike "checkout a branch" or "merge this commit" where we'll now shred
>> data implicitly it's obvious that git-clean is going to shred your data.
>
> Then that't not what I want. If I'm going to mark to keep "config.mak"
> around, I'm not going to carefully move it away before doing "git
> clean -fdx" then move it back. No "git clean --dry-run" telling me to
> make a backup of config.mak is no good.

Understood. I mean this in the context of solving the problem users have
with running otherwise non-data-destroying commands like "checkout" and
"merge" and getting their data destroyed, which is overwhelmingly why
this topic gets resurrected.

Some of the solutions overlap with this thing you want, but I think it's
worth keeping the distinction between the two in mind. I.e. I can
imagine us finding some acceptable solution to the data shredding
problem that doesn't implement this mode for "git-clean", or the other
way around.

Re: [RFC PATCH 0/7] test: NetBSD support

[Ævar Arnfjörð Bjarmason]

On Sun, Nov 25 2018, Carlo Marcelo Arenas Belón wrote:

> Likely still missing changes as it only completes a run with a minimal
> number of dependencies but open for feedback
>
> Requires pkgsrc packages for gmake, perl, bash and curl and completes a run
>
>   $ gmake SHELL_PATH=/usr/pkg/bin/bash NO_PYTHON=1 CURL_DIR=/usr/pkg test

This looks good to me, and fixes most of the issues I've encountered on
NetBSD recently. See
https://gitlab.com/git-vcs/git-gitlab-ci/blob/9337ed6f99e3780d1d8dc087dff688f5a68805a4/ci/gitlab/run-on-gcc-farm.sh#L228-239
and https://gitlab.com/git-vcs/git-ci/-/jobs/125476939

Re: [ANNOUNCE] Git v2.20.0-rc1

[Elijah Newren]

On Sun, Nov 25, 2018 at 11:37 PM Junio C Hamano  wrote:
>
> Unless I hear otherwise in the next 24 hours, I am planning to
> merge the following topics to 'master' before cutting -rc2.  Please
> stop me on any of these topics.
>
>  - jc/postpone-rebase-in-c
>
>This may be the most controversial.  It demotes the C
>reimplementation of "git rebase" to an experimental opt-in
>feature that can only be enabled by setting rebase.useBuiltIn
>configuration that defaults to false.
>
>cf. 

If we don't set rebase.useBuiltin to false, then there is also a minor
regression in the error message printed by the built-in rebase we may
want to try to address.  I have a patch for it at
<20181122044841.20993-2-new...@gmail.com>, but I don't know if that
patch is acceptable as-is this close to a release since that'd not
give translators much time to update.

Re: [RFC PATCH] Introduce "precious" file concept

[Duy Nguyen]

On Mon, Nov 26, 2018 at 4:34 PM Ævar Arnfjörð Bjarmason
 wrote:
>
>
> On Mon, Nov 26 2018, Duy Nguyen wrote:
>
> > On Mon, Nov 26, 2018 at 10:30 AM Per Lundberg  wrote:
> >>
> >> On 11/13/18 1:22 AM, brian m. carlson wrote:
> >> > This is going to totally hose automation.  My last job had files which
> >> > might move from tracked to untracked (a file that had become generated),
> >> > and long-running CI and build systems would need to be able to check out
> >> > one status and switch to the other.  Your proposed change will prevent
> >> > those systems from working, whereas they previously did.
> >> >
> >> > I agree that your proposal would have been a better design originally,
> >> > but breaking the way automated systems currently work is probably going
> >> > to be a dealbreaker.
> >>
> >> How about something like this:
> >>
> >> 1. Introduce a concept with "garbage" files, which git is "permitted to
> >> delete" without prompting.
> >>
> >> 2. Retain the current default, i.e. "ignored files are garbage" for now,
> >> making the new behavior _opt in_ to avoid breaking automated
> >> systems/existing scripts for anyone. Put the setting for this behind a
> >> new core.* config flag.
> >>
> >> 3. In the plan for version 3.0 (a new major version where some breakage
> >> can be tolerable, according to Semantic Versioning), change the default
> >> so that "only explicit garbage is garbage". Include very clear notices
> >> of this in the release notes. The config flag is retained, but its
> >> default changes from true->false or vice versa. People who dislike the
> >> new behavior can easily change back to the 2.x semantics.
> >
> > How does this garbage thing interact with "git clean -x"? My
> > interpretation of this flag/attribute is that at version 3.0 by
> > default all ignored files are _not_ garbage, so "git clean -x" should
> > not remove any of them. Which is weird because most of ignored files
> > are like *.o that should be removed.
> >
> > I also need to mark "precious" on untracked or even tracked files (*).
> > Not sure how this "garbage" attribute interacts with that.
> >
> > (*) I was hoping I could get the idea [1] implemented in somewhat good
> > shape before presenting here. But I'm a bit slow on that front. So
> > yeah this "precious" on untracked/tracked thingy may be even
> > irrelevant if the patch series will be rejected.
>
> I think a garbage (or trashable) flag, if implemented, wouldn't need any
> special case in git-clean, i.e. -x would remove all untracked files,
> whether ignored or garbage/trashable. That's what my patch to implement
> it does:
> https://public-inbox.org/git/87zhuf3gs0@evledraar.gmail.com/
>
> I think that makes sense. Users running "git clean" have "--dry-run" and
> unlike "checkout a branch" or "merge this commit" where we'll now shred
> data implicitly it's obvious that git-clean is going to shred your data.

Then that't not what I want. If I'm going to mark to keep "config.mak"
around, I'm not going to carefully move it away before doing "git
clean -fdx" then move it back. No "git clean --dry-run" telling me to
make a backup of config.mak is no good.
-- 
Duy

Re: [RFC] Introduce two new commands, switch-branch and restore-paths

[Duy Nguyen]

On Mon, Nov 26, 2018 at 4:03 AM Junio C Hamano  wrote:
>
> Thomas Gummerer  writes:
>
> > I like the idea of splitting those commands up, in fact it is
> > something I've been considering working on myself.  I do think we
> > should consider if we want to change the behaviour of those new
> > commands in any way compared to 'git checkout', since we're starting
> > with a clean slate.

Better defaults? Hell yes!

> > One thing in particular that I have in mind is something I'm currently
> > working on, namely adding a --index flag to 'git checkout', which
> > would make 'git checkout' work in non-overlay mode (for more
> > discussion on that see also [*1*].
>
> Ah, thanks for reminding me of that.  That explains why I felt
> uneasy to see "restore" in the proposed command name.

About that name. I didn't want to start the command name with checkout
to avoid completion conflict (the obvious choice was checkout-path,
the function name behind it). And I didn't find any other good name,
so I picked "restore" out of git-checkout.txt's one-line description.
If we end up with a better command name, perhaps reword that line too.
-- 
Duy

Re: [RFC PATCH] Introduce "precious" file concept

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 26 2018, Duy Nguyen wrote:

> On Mon, Nov 26, 2018 at 10:30 AM Per Lundberg  wrote:
>>
>> On 11/13/18 1:22 AM, brian m. carlson wrote:
>> > This is going to totally hose automation.  My last job had files which
>> > might move from tracked to untracked (a file that had become generated),
>> > and long-running CI and build systems would need to be able to check out
>> > one status and switch to the other.  Your proposed change will prevent
>> > those systems from working, whereas they previously did.
>> >
>> > I agree that your proposal would have been a better design originally,
>> > but breaking the way automated systems currently work is probably going
>> > to be a dealbreaker.
>>
>> How about something like this:
>>
>> 1. Introduce a concept with "garbage" files, which git is "permitted to
>> delete" without prompting.
>>
>> 2. Retain the current default, i.e. "ignored files are garbage" for now,
>> making the new behavior _opt in_ to avoid breaking automated
>> systems/existing scripts for anyone. Put the setting for this behind a
>> new core.* config flag.
>>
>> 3. In the plan for version 3.0 (a new major version where some breakage
>> can be tolerable, according to Semantic Versioning), change the default
>> so that "only explicit garbage is garbage". Include very clear notices
>> of this in the release notes. The config flag is retained, but its
>> default changes from true->false or vice versa. People who dislike the
>> new behavior can easily change back to the 2.x semantics.
>
> How does this garbage thing interact with "git clean -x"? My
> interpretation of this flag/attribute is that at version 3.0 by
> default all ignored files are _not_ garbage, so "git clean -x" should
> not remove any of them. Which is weird because most of ignored files
> are like *.o that should be removed.
>
> I also need to mark "precious" on untracked or even tracked files (*).
> Not sure how this "garbage" attribute interacts with that.
>
> (*) I was hoping I could get the idea [1] implemented in somewhat good
> shape before presenting here. But I'm a bit slow on that front. So
> yeah this "precious" on untracked/tracked thingy may be even
> irrelevant if the patch series will be rejected.

I think a garbage (or trashable) flag, if implemented, wouldn't need any
special case in git-clean, i.e. -x would remove all untracked files,
whether ignored or garbage/trashable. That's what my patch to implement
it does:
https://public-inbox.org/git/87zhuf3gs0@evledraar.gmail.com/

I think that makes sense. Users running "git clean" have "--dry-run" and
unlike "checkout a branch" or "merge this commit" where we'll now shred
data implicitly it's obvious that git-clean is going to shred your data.

Re: [RFC PATCH] Introduce "precious" file concept

[Duy Nguyen]

On Mon, Nov 26, 2018 at 10:30 AM Per Lundberg  wrote:
>
> On 11/13/18 1:22 AM, brian m. carlson wrote:
> > This is going to totally hose automation.  My last job had files which
> > might move from tracked to untracked (a file that had become generated),
> > and long-running CI and build systems would need to be able to check out
> > one status and switch to the other.  Your proposed change will prevent
> > those systems from working, whereas they previously did.
> >
> > I agree that your proposal would have been a better design originally,
> > but breaking the way automated systems currently work is probably going
> > to be a dealbreaker.
>
> How about something like this:
>
> 1. Introduce a concept with "garbage" files, which git is "permitted to
> delete" without prompting.
>
> 2. Retain the current default, i.e. "ignored files are garbage" for now,
> making the new behavior _opt in_ to avoid breaking automated
> systems/existing scripts for anyone. Put the setting for this behind a
> new core.* config flag.
>
> 3. In the plan for version 3.0 (a new major version where some breakage
> can be tolerable, according to Semantic Versioning), change the default
> so that "only explicit garbage is garbage". Include very clear notices
> of this in the release notes. The config flag is retained, but its
> default changes from true->false or vice versa. People who dislike the
> new behavior can easily change back to the 2.x semantics.

How does this garbage thing interact with "git clean -x"? My
interpretation of this flag/attribute is that at version 3.0 by
default all ignored files are _not_ garbage, so "git clean -x" should
not remove any of them. Which is weird because most of ignored files
are like *.o that should be removed.

I also need to mark "precious" on untracked or even tracked files (*).
Not sure how this "garbage" attribute interacts with that.

(*) I was hoping I could get the idea [1] implemented in somewhat good
shape before presenting here. But I'm a bit slow on that front. So
yeah this "precious" on untracked/tracked thingy may be even
irrelevant if the patch series will be rejected.

[1] 
https://public-inbox.org/git/cacsjy8c3rofv0kqejrwufqqzbnfu4msxjtpheybgmmrroff...@mail.gmail.com/

> Would this be a reasonable compromise for everybody?
-- 
Duy

Re: git bash lunching error

[Konstantin Khomoutov]

On Mon, Nov 26, 2018 at 08:32:27PM +0530, Chandra Shekhar wrote:

> cess: Resource temporarily unavailable (-1).
> DLL rebasing may be required; see 'rebaseall / rebase --help'.

This  ?

Please do not post screenshots, post text.
Also please copy and paste correctly when you do this.

git bash lunching error

[Chandra Shekhar]

cess: Resource temporarily unavailable (-1).
DLL rebasing may be required; see 'rebaseall / rebase --help'.

Re: [RFC PATCH] Introduce "precious" file concept

[Junio C Hamano]

Per Lundberg  writes:

> How about something like this:
> ...
> Would this be a reasonable compromise for everybody?

I do not think you'd need to introduce such a deliberately breaking
change at all.  Just introduce a new "precious" class, perhaps mark
them with the atttribute mechanism, and that would be the endgame.
Early adopters would start marking ignored but not expendable paths
with the "precious" attribute and they won't be clobbered.  As the
mechanism becomes widely known and mature, more and more people use
it.  And even after that happens, early adopters do not have to change
any attribute setting, and late adopters would have plenty of examples
to imitate.  Those who do not need any "precious" class do not have
to do anything and you won't break any existing automation that way.

Re: [RFC PATCH] Introduce "precious" file concept

[Ævar Arnfjörð Bjarmason]

On Mon, Nov 26 2018, Per Lundberg wrote:

> On 11/13/18 1:22 AM, brian m. carlson wrote:
>> This is going to totally hose automation.  My last job had files which
>> might move from tracked to untracked (a file that had become generated),
>> and long-running CI and build systems would need to be able to check out
>> one status and switch to the other.  Your proposed change will prevent
>> those systems from working, whereas they previously did.
>>
>> I agree that your proposal would have been a better design originally,
>> but breaking the way automated systems currently work is probably going
>> to be a dealbreaker.
>
> How about something like this:
>
> 1. Introduce a concept with "garbage" files, which git is "permitted to
> delete" without prompting.
>
> 2. Retain the current default, i.e. "ignored files are garbage" for now,
> making the new behavior _opt in_ to avoid breaking automated
> systems/existing scripts for anyone. Put the setting for this behind a
> new core.* config flag.
>
> 3. In the plan for version 3.0 (a new major version where some breakage
> can be tolerable, according to Semantic Versioning), change the default
> so that "only explicit garbage is garbage". Include very clear notices
> of this in the release notes. The config flag is retained, but its
> default changes from true->false or vice versa. People who dislike the
> new behavior can easily change back to the 2.x semantics.
>
> Would this be a reasonable compromise for everybody?

Possibly, but I think there's an earlier step zero there for anyone
interested in pursuing this (and currently I can't make time for it),
which is to submit a patch with tests and documentation showing exactly
the sort of scenarios where we clobber or don't clobber existing files.

As my https://public-inbox.org/git/87zhuf3gs0@evledraar.gmail.com/
shows we have tests for this, but they're not explicit, and some want to
test some unrelated thing.

I.e. to test the cases where we clobber foo.c because foo.c now
explicitly exists, or cases where dir/foo.c is clobbered because "dir"
is now a tracked text file etc., are those the only two cases? I vaguely
suspect that there were other interesting cases, but at this point the
information has been paged out of the working set of my wetware. The
thread at
https://public-inbox.org/git/87o9au39s7@evledraar.gmail.com/ has
some notes about this.

Then as noted in
https://public-inbox.org/git/87wopj3661@evledraar.gmail.com/ the
reason we have this behavior seems to be something that grew organically
from a semi-related bugfix.

So I don't think we're at a point where we're all dug into our trenches
and some people want X and others want Y and in the name of backwards
compatibility we're going to stay with X. It may turn out that we just
want to retain 10% of X, and can get 99% of the safety of Y by doing
that.

Re: [RFC PATCH] Introduce "precious" file concept

[Per Lundberg]

On 11/13/18 1:22 AM, brian m. carlson wrote:
> This is going to totally hose automation.  My last job had files which
> might move from tracked to untracked (a file that had become generated),
> and long-running CI and build systems would need to be able to check out
> one status and switch to the other.  Your proposed change will prevent
> those systems from working, whereas they previously did.
> 
> I agree that your proposal would have been a better design originally,
> but breaking the way automated systems currently work is probably going
> to be a dealbreaker.

How about something like this:

1. Introduce a concept with "garbage" files, which git is "permitted to 
delete" without prompting.

2. Retain the current default, i.e. "ignored files are garbage" for now, 
making the new behavior _opt in_ to avoid breaking automated 
systems/existing scripts for anyone. Put the setting for this behind a 
new core.* config flag.

3. In the plan for version 3.0 (a new major version where some breakage 
can be tolerable, according to Semantic Versioning), change the default 
so that "only explicit garbage is garbage". Include very clear notices 
of this in the release notes. The config flag is retained, but its 
default changes from true->false or vice versa. People who dislike the 
new behavior can easily change back to the 2.x semantics.

Would this be a reasonable compromise for everybody?
-- 
Per Lundberg