Inquiry 12.11.18

2018-11-11 Thread Daniel Murray
Hi,friend,
 
This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia.
We are glad to know about your company from the web and we are interested in 
your products.
Could you kindly send us your Latest catalog and price list for our trial order.
 
Best Regards,
 
Daniel Murray
Purchasing Manager




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

2018-11-11 Thread Per Lundberg
On 11/11/18 5:41 PM, Duy Nguyen wrote:
> On Sun, Nov 11, 2018 at 1:33 PM Ævar Arnfjörð Bjarmason
>  wrote:
 >
>> That will lose no data, and in the very rare cases where a checkout of
>> tracked files would overwrite an ignored pattern, we can just error out
>> (as we do with the "Ok to overwrite" branch removed) and tell the user
>> to delete the files to proceed. 
> There's also the other side of the coin. If this refuse to overwrite
> triggers too often, it can become an annoyance.

Sure, but doing "git checkout -f some_ref" instead of "git checkout 
some_ref" isn't really _that_ annoying, is it? I think, people (because 
of not having read/studied the .gitignore semantics well enough) having 
their files being overwritten _without realizing it_ is a bigger danger. 
But obviously there is a bit of treading a thin line here.

If we feel thrashable is stretching it too far (which I don't think it 
is), we could add a "core.ignore_files_are_trashable" setting that 
brings back the old semantics, for those who have a strong feeling about it.

It's also quite possible to do it the other way around - i.e. set 
"core.ignore_files_are_trashable" to true by default, and let the "new" 
behavior be opt-in. However, this might "miss the mark" in that those 
people who would really benefit from the new semantics might miss this 
setting, just like they could risk missing the "precious" setting.

(I also think "trashable" sounds better and is more clear & precise than 
"precious", for whatever that is worth.)
--
Per Lundberg


Re: [PATCH] checkout: disambiguate dwim tracking branches and local files

2018-11-11 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> @@ -1079,9 +1079,15 @@ static int parse_branchname_arg(int argc, const char 
> **argv,
>*/
>   int recover_with_dwim = dwim_new_local_branch_ok;
>  
> - if (!has_dash_dash &&
> - (check_filename(opts->prefix, arg) || !no_wildcard(arg)))
> - recover_with_dwim = 0;
> + /*
> +  * If both refs/remotes/origin/master and the file
> +  * 'master'. Don't blindly go for 'master' file
> +  * because it's ambiguous. Leave it for the user to
> +  * decide.
> +  */
> + int disambi_local_file = !has_dash_dash &&
> + (check_filename(opts->prefix, arg) || 
> !no_wildcard(arg));

What you are computing on the right hand side is if the arg is
ambiguous.  And the code that looks at this variable does not
disambiguate, but it just punts and makes it responsibility to the
user (which is by the way the correct thing to do).

When a file with exact name is in the working tree, we do not
declare it is a pathspec and not a rev, as we may be allowed to dwim
and create a rev with that name and at that point we'd be in an
ambigous situation.  If the arg _has_ wildcard, however, it is a
strong sign that it *is* a pathspec, isn't it?  It is iffy that a
single variable that cannot be used to distinguish these two cases
is introduced---one of these cases will be mishandled.

Also how does the patch ensures that this new logic does not kick in
for those who refuse to let the command dwim to create a new branch?

>   /*
>* Accept "git checkout foo" and "git checkout foo --"
>* as candidates for dwim.
> @@ -1094,6 +1100,9 @@ static int parse_branchname_arg(int argc, const char 
> **argv,
>   const char *remote = unique_tracking_name(arg, rev,
> 
> dwim_remotes_matched);
>   if (remote) {
> + if (disambi_local_file)
> + die(_("'%s' could be both a local file 
> and a tracking branch.\n"
> +   "Please use -- to disambiguate"), 
> arg);

Ah, the only user of this variable triggers when recover_with_dwim
is true, so that is how dwim-less case is handled, OK.

That still leaves the question if it is OK to handle these two cases
the same way in a repository without 'next' branch whose origin has
one:

$ >next; git checkout --guess next
$ >next; git checkout --guess 'n??t'

Perhaps the variable should be named "local_file_crashes_with_rev"
and its the scope narrowed to the same block as "remote" is
computed.  Or just remove the variable and check the condition right
there where you need to.  I.e.

if (remote) {
if (!has_dash_dash &&
check_filename(opts->prefix, arg))
die("did you mean a branch or path?: '%s'", arg);
...



Re: [PATCH/RFC] checkout: print something when checking out paths

2018-11-11 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> Since the purpose of printing this is to help disambiguate. Only do it
> when "--" is missing (the actual reason though is many tests check
> empty stderr to see that no error is raised and I'm too lazy to fix
> all the test cases).

Heh, honesty is good but in this particular case the official reason
alone would make perfect sense, too ;-)

As with progress output, shouldn't this automatically be turned off
when the standard error stream goes to non tty, as the real purpose
of printing is to help the user sitting in front of the terminal and
interacting with the command?

And by this, I do not mean to say that "When -- is missing" can go
away.  I agree that "--" is a clear sign that the user knows what
s/he is doing---to overwrite the paths in the working tree that
match the pathspec.

> @@ -371,17 +374,27 @@ static int checkout_paths(const struct checkout_opts 
> *opts,
>   struct cache_entry *ce = active_cache[pos];
>   if (ce->ce_flags & CE_MATCHED) {
>   if (!ce_stage(ce)) {
> - errs |= checkout_entry(ce, , NULL);
> + errs |= checkout_entry(ce, ,
> +NULL, _checkouts);
>   continue;

As we count inside checkout_entry(), we might not actually write
this out when we know that the working tree file is up to date
already but we do not increment in that case either, so we keep
track of the accurate count of "updates", not path matches (i.e. we
are not reporting "we made sure this many paths are up to date in
the working tree"; instead we are reporting how many paths we needed
to overwrite in the working tree).

>   }
>   if (opts->writeout_stage)
> - errs |= checkout_stage(opts->writeout_stage, 
> ce, pos, );
> + errs |= checkout_stage(opts->writeout_stage,
> +ce, pos,
> +, _checkouts);

Likewike.

>   else if (opts->merge)
> - errs |= checkout_merged(pos, );
> + errs |= checkout_merged(pos, ,
> + _checkouts);

Likewise.

>   pos = skip_same_name(ce, pos) - 1;
>   }
>   }
> - errs |= finish_delayed_checkout();
> + errs |= finish_delayed_checkout(, _checkouts);
> +
> + if (opts->count_checkout_paths)
> + fprintf_ln(stderr, Q_("%d path has been updated",
> +   "%d paths have been updated",
> +   nr_checkouts),
> +nr_checkouts);

This one may want to be protected behind isatty(2).  Or the default
value of count_checkout_paths could be tweakedbased on isatty(2).

> @@ -1064,6 +1077,7 @@ static int parse_branchname_arg(int argc, const char 
> **argv,
>   has_dash_dash = 1; /* case (3) or (1) */
>   else if (dash_dash_pos >= 2)
>   die(_("only one reference expected, %d given."), dash_dash_pos);
> + opts->count_checkout_paths = !opts->quiet && !has_dash_dash;

i.e. "&& isatty(2)" as well.

Of course, "--[no-]count-paths" could be added to override this.


Re: [PATCH v2 00/22] Kill the_index part 5

2018-11-11 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> I did start pushing the_repository out of library code [1] in the bottom
> half of this series. There aren't big surprises except that cache-tree
> API now take _both_ struct repository and struct index_state. The
> reason is cache-tree can work on temporary indexes and repo->index is
> about $GIT_DIR/index.

Yes, this is very sensible, and I expect many more functions would
(and probably should) follow suit, as we'd want to be able to work
with temporary index (not in the sense that we'll write out on-disk
temporary index, but multiple in-core instances of istate that is
not necessarily tied to the current or the next state of
$GIT_DIR/index) from more places.



Dear,

2018-11-11 Thread Sylvester Fred
Dear,

Please accept my apologies I do not intend to invade your privacy, I
wrote to you earlier, but no answer, in my first post I told you about
my late client  who bears the same surname with you, I received
several letters from the bank, where he made a deposit of 10.4 million
dollars  before his death, the bank asked me to provide his next of
kin or any of his relatives, who  will stand for this claim, otherwise
it will be confiscated by the bank due to lack of claims from his
relatives hence I contacted you to present you as is beneficiary since
you have the same last name with him. After your reply I shall give
you the details and procedures of this transaction, waiting your
reply.

Regards,
Sylvester Fred


Re: [RFC PATCH 5/5] ref-filter: add docs for new options

2018-11-11 Thread Junio C Hamano
Olga Telezhnaya  writes:

>  objectname::
>   The object name (aka SHA-1).
>   For a non-ambiguous abbreviation of the object name append `:short`.
>   For an abbreviation of the object name with desired length append
>   `:short=`, where the minimum length is MINIMUM_ABBREV. The
>   length may be exceeded to ensure unique object names.
> +deltabase::
> + If the object is stored as a delta on-disk, this expands to the 40-hex
> + sha1 of the delta base object. Otherwise, expands to the null sha1
> + (40 zeroes). See `CAVEATS` section below.

I know existing description for other things nearby still talk about
SHA-1, but we can prepare ourselves better with something like:

This expands to the object name of the delta base for the
given object, if it is stored as a delta.  Otherwise it
expands to the null object name (all zeroes).

> +Note that the sizes of objects on disk are reported accurately, but care
> +should be taken in drawing conclusions about which refs or objects are
> +responsible for disk usage. The size of a packed non-delta object may be
> +much larger than the size of objects which delta against it, but the
> +choice of which object is the base and which is the delta is arbitrary
> +and is subject to change during a repack.
> +
> +Note also that multiple copies of an object may be present in the object
> +database; in this case, it is undefined which copy's size or delta base
> +will be reported.

OK.


>  SEE ALSO
>  
>  linkgit:git-show-ref[1]
>
> --
> https://github.com/git/git/pull/552


Re: [PATCH v2 1/1] remote-curl.c: xcurl_off_t is not portable (on 32 bit platfoms)

2018-11-11 Thread Torsten Bögershausen
On Mon, Nov 12, 2018 at 12:50:30PM +0900, Junio C Hamano wrote:
> tbo...@web.de writes:
> 
> >
> > This is a re-semd, the orignal patch was part of a 2
> > patch-series.
> > This patch needed some rework, and here should be
> > the polished version.
> 
> Will queue.

Thanks, is there a chance to kill a typo here ?
s/comopared/compared/
- A temporally variable "size" is used, promoted int uintmax_t and the comopared


> Next time, please refrain from saying "re-send", if you
> changed anything in the patch (or the log message), as the phrase
> implies you are sending the same thing as before (just in case the
> earlier one was not seen, for example).  Marking it as vN+1 like you
> did for this patch and having a reference to the original would make
> it clear, though ;-)

Sorry for the confusion.
The next time I will not send unrelated patches as a series,
so that we have a better "Message-ID:" and "In-Reply-To:"
flow (which should make live 3% easier).

https://public-inbox.org/git/20181029165914.2677-1-tbo...@web.de/
https://public-inbox.org/git/20181109174110.27630-1-tbo...@web.de/



Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

2018-11-11 Thread Junio C Hamano
Olga Telezhnaya  writes:

> @@ -876,11 +882,13 @@ static void grab_common_values(struct atom_value *val, 
> int deref, struct expand_
>   name++;
>   if (!strcmp(name, "objecttype"))
>   v->s = xstrdup(type_name(oi->type));
> - else if (!strcmp(name, "objectsize")) {
> + else if (!strcmp(name, "objectsize:disk")) {
> + v->value = oi->disk_size;
> + v->s = xstrfmt("%lld", (long long)oi->disk_size);

oi.disk_size is off_t; do we know "long long" 

   (1) is available widely enough (I think it is from c99)?

   (2) is sufficiently wide so that we can safely cast off_t to?

   (3) will stay to be sufficiently wide as machines get larger
   together with standard types like off_t in the future?

I'd rather use intmax_t (as off_t is a signed integral type) so that
we do not have to worry about these questions in the first place.

> + } else if (!strcmp(name, "objectsize")) {
>   v->value = oi->size;
>   v->s = xstrfmt("%lu", oi->size);

This is not a suggestion but is a genuine question, but I wonder if
two years down the road somebody who meets this API for the first
time find the asymmetry between "objectsize" and "objectsize:disk" a
tad strange and suggest the former to have "objectsize:real" or some
synonym.  Or we can consider "objectsize" the primary thing (hence
needing no colon-plus-modifier to clarify what kind of size we are
asking) and leave these two deliberatly asymmetric.  I am leaning
towards the latter myself.

> - }
> - else if (deref)
> + } else if (deref)
>   grab_objectname(name, >oid, v, _atom[i]);
>   }
>  }
>
> --
> https://github.com/git/git/pull/552


Price Inquiry

2018-11-11 Thread Daniel Murray
Hi,friend,
 
This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia.
We are glad to know about your company from the web and we are interested in 
your products.
Could you kindly send us your Latest catalog and price list for our trial order.
 
Best Regards,
 
Daniel Murray
Purchasing Manager




Re: [PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate

2018-11-11 Thread Junio C Hamano
Johannes Schindelin  writes:

>> >  static int reset_head(struct object_id *oid, const char *action,
>> > -const char *switch_to_branch, int detach_head,
>> > +const char *switch_to_branch,
>> > +int detach_head, int reset_hard,
>> 
>> It might be worth switching to a single flag variable here. It would
>> make calls like this:
>> 
>> > -  if (reset_head(>object.oid, "checkout", NULL, 1,
>> > +  if (reset_head(>object.oid, "checkout", NULL, 1, 0,
>> >NULL, msg.buf))
>> 
>> a little more self-documenting (if a little more verbose).
>
> I thought about that, but for just two flags? Well, let me try it and see
> whether I like the result ;-)

My rule of thumb is that repeating three times is definitely when we
should consolidate separate ones into a single flag word, and twice
is a borderline.

For two-time repetition, it is often worth fixing when somebody
notices it during the review, as that is a sign that repetition,
even a minor one, disturbed a reader enough to point out.  On the
other hand, for a file-scope static that is likely to stay as a
non-API function but a local helper, it may not be worth it.

So I am OK either way, slightly in favor of fixing it while we
remember.


>> This one could actually turn into:
>> 
>>   ret = error(...);
>>   goto leave_reset_head;
>> 
>> now. We don't have to worry about an uninitialized desc.buffer anymore
>> (as I mentioned in the previous email), because "nr" would be 0.
>> 
>> It doesn't save any lines, though (but maybe having a single
>> cleanup/exit point would make things easier to read; I dunno).
>
> But you're right, of course. Consistency makes for easier-to-read code.

Yup, that does sound good.

Thanks.


Re: [PATCH v2 1/1] Update .mailmap

2018-11-11 Thread Junio C Hamano
"Johannes Schindelin via GitGitGadget" 
writes:

> From: Johannes Schindelin 
>
> This patch makes the output of `git shortlog -nse v2.10.0..master`
> duplicate-free.
>
> Signed-off-by: Johannes Schindelin 
> ---

Thanks, will queue.


Re: [PATCH v2 1/1] remote-curl.c: xcurl_off_t is not portable (on 32 bit platfoms)

2018-11-11 Thread Junio C Hamano
tbo...@web.de writes:

>
> This is a re-semd, the orignal patch was part of a 2
> patch-series.
> This patch needed some rework, and here should be
> the polished version.

Will queue.  Next time, please refrain from saying "re-send", if you
changed anything in the patch (or the log message), as the phrase
implies you are sending the same thing as before (just in case the
earlier one was not seen, for example).  Marking it as vN+1 like you
did for this patch and having a reference to the original would make
it clear, though ;-)

Thanks.

> remote-curl.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/remote-curl.c b/remote-curl.c
> index 762a55a75f..1220dffcdc 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -617,10 +617,11 @@ static int probe_rpc(struct rpc_state *rpc, struct 
> slot_results *results)
>   return err;
>  }
>  
> -static curl_off_t xcurl_off_t(ssize_t len) {
> - if (len > maximum_signed_value_of_type(curl_off_t))
> +static curl_off_t xcurl_off_t(size_t len) {
> + uintmax_t size = len;
> + if (size > maximum_signed_value_of_type(curl_off_t))
>   die("cannot handle pushes this big");
> - return (curl_off_t) len;
> + return (curl_off_t)size;
>  }
>  
>  static int post_rpc(struct rpc_state *rpc)


Re: [PATCH v4 3/3] range-diff: make diff option behavior (e.g. --stat) consistent

2018-11-11 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason   writes:

> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> index ab44e085d5..9352f65280 100755
> --- a/t/t3206-range-diff.sh
> +++ b/t/t3206-range-diff.sh
> @@ -140,37 +140,15 @@ test_expect_success 'changed commit with --stat diff 
> option' '
>   1:  4de457d = 1:  a4b s/5/A/
>a => b | 0
>1 file changed, 0 insertions(+), 0 deletions(-)
> - $four_spaces

This removes all references to four_spaces=" " assigned at the very
beginning of this test piece, so that assignment should also go, no?

>   2:  fccce22 = 2:  f51d370 s/4/A/
>a => b | 0
>1 file changed, 0 insertions(+), 0 deletions(-)
> - $four_spaces
>   3:  147e64e ! 3:  0559556 s/11/B/
>a => b | 0
>1 file changed, 0 insertions(+), 0 deletions(-)
> - $four_spaces
> - @@ -10,7 +10,7 @@
> -   9
> -   10
> -  -11
> - -+B
> - ++BB
> -   12
> -   13
> -   14
>   4:  a63e992 ! 4:  d966c5c s/12/B/
>a => b | 0
>1 file changed, 0 insertions(+), 0 deletions(-)
> - $four_spaces
> - @@ -8,7 +8,7 @@
> -  @@
> -   9
> -   10
> - - B
> - + BB
> -  -12
> -  +B
> -   13
>   EOF
>   test_cmp expected actual
>  '


Re: [PATCH v4 3/3] range-diff: make diff option behavior (e.g. --stat) consistent

2018-11-11 Thread Junio C Hamano
Eric Sunshine  writes:

> On Fri, Nov 9, 2018 at 5:18 AM Ævar Arnfjörð Bjarmason  
> wrote:
>> Make the behavior when diff options (e.g. "--stat") are passed
>> consistent with how "diff" behaves.
>> [...]
>> Signed-off-by: Ævar Arnfjörð Bjarmason 
>> ---
>> diff --git a/range-diff.c b/range-diff.c
>> @@ -453,7 +453,8 @@ int show_range_diff(const char *range1, const char 
>> *range2,
>> -   opts.output_format |= DIFF_FORMAT_PATCH;
>> +   if (!opts.output_format)
>> +   opts.output_format |= DIFF_FORMAT_PATCH;
>
> I think this can just be '=' now instead of '|=' (to avoid confusing
> the reader, even if it's functionally equivalent).

Hmph, could the condition in the future change to

-   if (!opts.output_format)
+   if (! (opts.output_format & DIFF_FORMAT_MASK))
opts.output_format |= DIFF_FORMAT_PATCH

if we ever gain a new "output_format" bit that does not affect how
we show the diff in a major way, and that is on by default?  If so,
I think "|=" is more future-proof.  Otherwise, "=" is indeed more
clear way to spell the intention.











Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()

2018-11-11 Thread Junio C Hamano
Duy Nguyen  writes:

> FWIW I don't have any preference, as long as the variable can still
> have a name (that is not a symbol).

Same here.

> A side question regardless of syntax. What do we do with
> %(unrecognized name)/foo? I see three options
>
>  - expand to empty, so "/foo"
>  - keep it and try the literal path "%(unrecognized name)/foo"

Neither of these is good for future proofing purposes.

>  - somehow tell the caller that the path is invalid and treat it like
> non-existing path, even if there is some real thing at "%(unrecognized
> name)/foo"

Another thing to worry about is how to spell the real thing that has
such a funny name, perhaps by escaping like "%%(unrecognized
name)/foo".

And from that point of view, "~runtime-prefix~/foo" (i.e. what J6t
started) may make the most sense, as I do not think we need to
support a username that ends with a tilde by introducing a quoting
convention.


Re: Add issue management within git

2018-11-11 Thread yan ke
   Very very agree, now it is very difficult to find a solution when
has some problem such build problem an so on! The mail-list is good to
send patch es, but is it not suitable for problem track or problem
solution search!
   Now the Github or Gitlab is good to track issues, suggest to open
the git issue track!
Martin Delille  于2018年11月12日周一 上午6:52写道:
>
> Hi,
>
> This would be awesome to handle issue directly with git:
> Having an offline version of the issues synced to the gitlab/github issues.
> A lot of work is done on the issues and it is lost when migrating from one 
> service to the other.
> Beside we don’t always have a good internet connection.
> There is already a kind of integration between commit message fixing issue 
> automatically when merged in the master branch (with “fix #143’).
>
> Kind regards,
>
> Martin
>


[PATCH v2 2/2] branch: Mark and colorize a branch differently if it is checked out in a linked worktree

2018-11-11 Thread nbelakovski
From: Nickolai Belakovski 

In order to more clearly display which branches are active, the output
of git branch is modified to mark branches checkout out in a linked
worktree with a "+" and color them in a faint light green (in contrast
to the current branch, which will still be denoted with a "*" and
colored in green)

This is meant to simplify workflows related to worktree, particularly
due to the limitations of not being able to check out the same branch in
two worktrees and the inability to delete a branch checked out in a
worktree. When performing branch operations like checkout and delete, it
would be useful to know more readily if the branches in which the user
is interested are already checked out in a worktree.

The git worktree list command contains the relevant information, however
this is a much less frquently used command than git branch.

Signed-off-by: Nickolai Belakovski 
---
 builtin/branch.c | 22 +-
 color.h  | 18 ++
 t/t3200-branch.sh|  8 
 t/t3203-branch-output.sh | 21 +
 t/test-lib-functions.sh  |  6 ++
 5 files changed, 62 insertions(+), 13 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 0c55f7f065..34f44c82d7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -42,11 +42,12 @@ static struct object_id head_oid;
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
-   GIT_COLOR_NORMAL,   /* PLAIN */
-   GIT_COLOR_RED,  /* REMOTE */
-   GIT_COLOR_NORMAL,   /* LOCAL */
-   GIT_COLOR_GREEN,/* CURRENT */
-   GIT_COLOR_BLUE, /* UPSTREAM */
+   GIT_COLOR_NORMAL, /* PLAIN */
+   GIT_COLOR_RED,/* REMOTE */
+   GIT_COLOR_NORMAL, /* LOCAL */
+   GIT_COLOR_GREEN,  /* CURRENT */
+   GIT_COLOR_BLUE,   /* UPSTREAM */
+   GIT_COLOR_FAINT_LIGHT_GREEN,  /* WORKTREE */
 };
 enum color_branch {
BRANCH_COLOR_RESET = 0,
@@ -54,7 +55,8 @@ enum color_branch {
BRANCH_COLOR_REMOTE = 2,
BRANCH_COLOR_LOCAL = 3,
BRANCH_COLOR_CURRENT = 4,
-   BRANCH_COLOR_UPSTREAM = 5
+   BRANCH_COLOR_UPSTREAM = 5,
+   BRANCH_COLOR_WORKTREE = 6
 };
 
 static const char *color_branch_slots[] = {
@@ -64,6 +66,7 @@ static const char *color_branch_slots[] = {
[BRANCH_COLOR_LOCAL]= "local",
[BRANCH_COLOR_CURRENT]  = "current",
[BRANCH_COLOR_UPSTREAM] = "upstream",
+   [BRANCH_COLOR_WORKTREE] = "worktree",
 };
 
 static struct string_list output = STRING_LIST_INIT_DUP;
@@ -342,9 +345,10 @@ static char *build_format(struct ref_filter *filter, int 
maxwidth, const char *r
struct strbuf local = STRBUF_INIT;
struct strbuf remote = STRBUF_INIT;
 
-   strbuf_addf(, "%%(if)%%(HEAD)%%(then)* %s%%(else)  %s%%(end)",
-   branch_get_color(BRANCH_COLOR_CURRENT),
-   branch_get_color(BRANCH_COLOR_LOCAL));
+   strbuf_addf(, "%%(if)%%(HEAD)%%(then)* 
%s%%(else)%%(if)%%(worktree)%%(then)+ %s%%(else)  %s%%(end)%%(end)",
+   branch_get_color(BRANCH_COLOR_CURRENT),
+   branch_get_color(BRANCH_COLOR_WORKTREE),
+   branch_get_color(BRANCH_COLOR_LOCAL));
strbuf_addf(, "  %s",
branch_get_color(BRANCH_COLOR_REMOTE));
 
diff --git a/color.h b/color.h
index 98894d6a17..857653df73 100644
--- a/color.h
+++ b/color.h
@@ -42,6 +42,24 @@ struct strbuf;
 #define GIT_COLOR_FAINT_BLUE   "\033[2;34m"
 #define GIT_COLOR_FAINT_MAGENTA"\033[2;35m"
 #define GIT_COLOR_FAINT_CYAN   "\033[2;36m"
+#define GIT_COLOR_LIGHT_RED"\033[91m"
+#define GIT_COLOR_LIGHT_GREEN  "\033[92m"
+#define GIT_COLOR_LIGHT_YELLOW "\033[93m"
+#define GIT_COLOR_LIGHT_BLUE   "\033[94m"
+#define GIT_COLOR_LIGHT_MAGENTA"\033[95m"
+#define GIT_COLOR_LIGHT_CYAN   "\033[96m"
+#define GIT_COLOR_BOLD_LIGHT_RED   "\033[1;91m"
+#define GIT_COLOR_BOLD_LIGHT_GREEN "\033[1;92m"
+#define GIT_COLOR_BOLD_LIGHT_YELLOW"\033[1;93m"
+#define GIT_COLOR_BOLD_LIGHT_BLUE  "\033[1;94m"
+#define GIT_COLOR_BOLD_LIGHT_MAGENTA   "\033[1;95m"
+#define GIT_COLOR_BOLD_LIGHT_CYAN  "\033[1;96m"
+#define GIT_COLOR_FAINT_LIGHT_RED  "\033[2;91m"
+#define GIT_COLOR_FAINT_LIGHT_GREEN"\033[2;92m"
+#define GIT_COLOR_FAINT_LIGHT_YELLOW   "\033[2;93m"
+#define GIT_COLOR_FAINT_LIGHT_BLUE "\033[2;94m"
+#define GIT_COLOR_FAINT_LIGHT_MAGENTA  "\033[2;95m"
+#define GIT_COLOR_FAINT_LIGHT_CYAN "\033[2;96m"
 #define GIT_COLOR_BG_RED   "\033[41m"
 #define GIT_COLOR_BG_GREEN "\033[42m"
 #define GIT_COLOR_BG_YELLOW"\033[43m"
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 478b82cf9b..e404f6e23c 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -292,7 +292,7 @@ test_expect_success 'git branch --list -v with --abbrev' '
 test_expect_success 

[PATCH v2 1/2] ref-filter: add worktree atom

2018-11-11 Thread nbelakovski
From: Nickolai Belakovski 

Add an atom expressing whether the particular ref is checked out in a
linked worktree.

Signed-off-by: Nickolai Belakovski 
---
 ref-filter.c   | 31 +++
 t/t6302-for-each-ref-filter.sh | 15 +++
 2 files changed, 46 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index 0c45ed9d94..53e2504f5d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -20,6 +20,7 @@
 #include "commit-slab.h"
 #include "commit-graph.h"
 #include "commit-reach.h"
+#include "worktree.h"
 
 static struct ref_msg {
const char *gone;
@@ -114,6 +115,7 @@ static struct used_atom {
} objectname;
struct refname_atom refname;
char *head;
+   struct string_list worktree_heads;
} u;
 } *used_atom;
 static int used_atom_cnt, need_tagged, need_symref;
@@ -420,6 +422,28 @@ static int head_atom_parser(const struct ref_format 
*format, struct used_atom *a
return 0;
 }
 
+static int worktree_head_atom_parser(const struct ref_format *format,
+struct 
used_atom *atom,
+const 
char *arg,
+struct 
strbuf *unused_err)
+{
+   struct worktree **worktrees = get_worktrees(0);
+   int i;
+
+   string_list_init(>u.worktree_heads, 1);
+
+   for (i = 0; worktrees[i]; i++) {
+   if (worktrees[i]->head_ref)
+   string_list_append(>u.worktree_heads,
+  
worktrees[i]->head_ref);
+   }
+
+   string_list_sort(>u.worktree_heads);
+
+   free_worktrees(worktrees);
+   return 0;
+}
+
 static struct {
const char *name;
info_source source;
@@ -461,6 +485,7 @@ static struct {
{ "flag", SOURCE_NONE },
{ "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser },
{ "color", SOURCE_NONE, FIELD_STR, color_atom_parser },
+   { "worktree", SOURCE_NONE, FIELD_STR, worktree_head_atom_parser },
{ "align", SOURCE_NONE, FIELD_STR, align_atom_parser },
{ "end", SOURCE_NONE },
{ "if", SOURCE_NONE, FIELD_STR, if_atom_parser },
@@ -1594,6 +1619,12 @@ static int populate_value(struct ref_array_item *ref, 
struct strbuf *err)
else
v->s = xstrdup(" ");
continue;
+   } else if (!strcmp(name, "worktree")) {
+   if (string_list_has_string(>u.worktree_heads, 
ref->refname))
+   v->s = xstrdup("+");
+   else
+   v->s = xstrdup(" ");
+   continue;
} else if (starts_with(name, "align")) {
v->handler = align_atom_handler;
v->s = xstrdup("");
diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh
index fc067ed672..5e6d249d4c 100755
--- a/t/t6302-for-each-ref-filter.sh
+++ b/t/t6302-for-each-ref-filter.sh
@@ -441,4 +441,19 @@ test_expect_success '--merged is incompatible with 
--no-merged' '
test_must_fail git for-each-ref --merged HEAD --no-merged HEAD
 '
 
+test_expect_success '"add" a worktree' '
+   mkdir worktree_dir &&
+   git worktree add -b master_worktree worktree_dir master
+'
+
+test_expect_success 'validate worktree atom' '
+   cat >expect <<-\EOF &&
+   master: checked out in a worktree
+   master_worktree: checked out in a worktree
+   side: not checked out in a worktree
+EOF
+git for-each-ref --format="%(refname:short): 
%(if)%(worktree)%(then)checked out in a worktree%(else)not checked out in a 
worktree%(end)" refs/heads/ >actual &&
+   test_cmp expect actual
+'
+
 test_done
-- 
2.14.2



[PATCH v2 0/2] refactoring branch colorization to ref-filter

2018-11-11 Thread nbelakovski
From: Nickolai Belakovski 

Finally found some time to follow up on this :)

I decided to take the approach suggested by Peff for simplicity. I have
another version which uses a hash map to store *all* of the information
returned by get_worktrees, information which can then be accessed in
the fashion %(workree:path) or %(worktree:is_detached), but it seemed
like a lot of code to add and there was no use case to justify the
addition of a hash map at this time. If there's interest though, I can
make a separate patch after this one to introduce those changes. They
build directly off of the changes introduced here.

I've split this work into two commits since the items are logically
separate.

CI results: https://travis-ci.org/nbelakovski/git/builds/453723727

Nickolai Belakovski (2):
  ref-filter: add worktree atom
  branch: Mark and colorize a branch differently if it is checked out in
a linked worktree

 builtin/branch.c   | 22 +-
 color.h| 18 ++
 ref-filter.c   | 31 +++
 t/t3200-branch.sh  |  8 
 t/t3203-branch-output.sh   | 21 +
 t/t6302-for-each-ref-filter.sh | 15 +++
 t/test-lib-functions.sh|  6 ++
 7 files changed, 108 insertions(+), 13 deletions(-)

-- 
2.14.2



Migration to Git LFS inflates repository multiple times

2018-11-11 Thread Mateusz Loskot
Hi,

I'm posting here for the first time and I hope it's the right place to ask
questions about Git LFS.

TL;TR: Is this normal a repository migrated to Git LFS inflates multiple times
and how to deal with it?

I'm migrating a big SVN repository to Git.
In SVN, a collection of third-party SDKs is maintained along with codebase.
Many of the third-party libraries come in binary form.
So, I'm migrating binary files of those to Git LFS.

I'm following the Git LFS tutorial,
section "Migrating existing repository data to LFS"
https://github.com/git-lfs/git-lfs/wiki/Tutorial

First, I run initial translation of the SVN reoi into Git..
The new repository is a Git bare repository.
There are 5 branches and 10+ tags in the proj.git repo.

It is quite large:

proj.git (BARE:master) $ du -sh
19G

Next, I performed the following sequence of steps to optimise it
and migrate to Git LFS:

1. Optimise the repo

proj.git (BARE:master) $ git gc
Enumerating objects: 1432599, done.
Counting objects: 100% (1432599/1432599), done.
Delta compression using up to 48 threads
Compressing objects: 100% (864524/864524), done.
Writing objects: 100% (1432599/1432599), done.
Total 1432599 (delta 541698), reused 1405922 (delta 525738)
Removing duplicate objects: 100% (256/256), done.
Checking connectivity: 1432599, done.

proj.git (BARE:master) $ du -sh
11G

2. List the file types taking up the most space in the repo

proj.git (BARE:master) $ git lfs migrate info --everything
migrate: Sorting commits: ..., done
migrate: Examining commits: 100% (29412/29412), done
*.lib   27 GB   3524/3524 files(s)  100%
*.pdb   5.6 GB  1412/1412 files(s)  100%
*.cpp   4.8 GB  131848/131854 files(s)  100%
*.exe   2.3 GB798/798 files(s)  100%
*.dll   2.0 GB  1000/1000 files(s)  100%

3. Migrate the repo to Git LFS

proj.git (BARE:master) $ git lfs migrate import
--include="*.exe,*.dll,*.lib,*.pdb,*.zip" --everything

4. Check size of the repo after migration to Git LFS

proj.git (BARE:master) $ du -sh
47G

5. Cleaning up the `.git` directory after migration to Git LFS

proj.git (BARE:master) $ git reflog expire --expire-unreachable=now --all

proj.git (BARE:master) $ git gc --prune=now --aggressive
Enumerating objects: 1462310, done.
Counting objects: 100% (1462310/1462310), done.
Delta compression using up to 48 threads
Compressing objects: 100% (1422322/1422322), done.
Writing objects: 100% (1462310/1462310), done.
Total 1462310 (delta 577640), reused 845097 (delta 0)
Removing duplicate objects: 100% (256/256), done.
Checking connectivity: 1462310, done.

6. Check final disk size of the repo

proj.git (BARE:master) $ du -sh
39G

7. List the file types taking up the most space in the repository
after migration to Git LFS

proj.git (BARE:master) $ git lfs migrate info --everything
migrate: Sorting commits: ..., done
migrate: Examining commits: 100% (29412/29412), done
*.cpp   4.8 GB  131848/131854 files(s)  100%
*.png   1.1 GB  696499/696499 files(s)  100%
*.h 828 MB86386/86471 files(s)  100%
*.csv   820 MB939/939 files(s)  100%
*.html  686 MB34126/34126 files(s)  100%


Now, I'm looking for anaswers to the following questions:

1. Is the procedure presented above correct to migrate (SVN ->) Git -> Git LFS?

2. Given the initial translation to Git generated 19 GB repo
(optimised to 11 GB)
is this normal Git LFS migration inflates the repository
to 47 GB (optimised ot 39 GB)?

3. Why the inflation happens? Is this a function of number of branches?
   How to understand the jump from 11 GB to 39 GB?

4. How to optimise the repository to cut the size down further?

My next step is to somehow push the fat pig into GitHub, Bitbucket or
Azure DevOps ;-)

I've used Git for a few years, but I'm pretty newbie regarding low-level
or administration tasks, so I might have made basic errors.
I'll be thankful for any feedback.

Best regards,
-- 
Mateusz Loskot, http://mateusz.loskot.net


Add issue management within git

2018-11-11 Thread Martin Delille
Hi,

This would be awesome to handle issue directly with git:
Having an offline version of the issues synced to the gitlab/github issues.
A lot of work is done on the issues and it is lost when migrating from one 
service to the other.
Beside we don’t always have a good internet connection.
There is already a kind of integration between commit message fixing issue 
automatically when merged in the master branch (with “fix #143’).

Kind regards,

Martin



BUSSINESS OFFER

2018-11-11 Thread nlamia701
Am lamia kindly reply me back so i can introduce my self to you.

Thanks
Lamia


Re: [PATCH 03/10] fast-export: use value from correct enum

2018-11-11 Thread Ævar Arnfjörð Bjarmason


On Sun, Nov 11 2018, Jeff King wrote:

> On Sat, Nov 10, 2018 at 10:23:05PM -0800, Elijah Newren wrote:
>
>> ABORT and ERROR happen to have the same value, but come from differnt
>> enums.  Use the one from the correct enum.
>
> Yikes. :)
>
> This is a good argument for naming these SIGNED_TAG_ABORT, etc. But this
> is obviously an improvement in the meantime.

In C enum values aren't the types of the enum, but I'd thought someone
would have added a warning for this:

#include 

enum { A, B } foo = A;
enum { C, D } bar = C;

int main(void)
{
switch (foo) {
  case C:
puts("A");
break;
  case B:
puts("B");
break;
}
}

But none of the 4 C compilers (gcc, clang, suncc & xlc) I have warn
about it. Good to know.


Re: BUG REPORT: git clone of non-existent repository results in request for credentials

2018-11-11 Thread Federico Lucifredi
I was afraid that was the reason. Oh well, at least we know why :-)

Thanks Ævar!

Best-F

> On Nov 11, 2018, at 9:00 AM, Ævar Arnfjörð Bjarmason  wrote:
> 
> 
>> On Sun, Nov 11 2018, Federico Lucifredi wrote:
>> 
>> git clone of non-existent repository results in request for credentials
>> 
>> REPRODUCING:
>> sudo apt install git
>> git clone https://github.com/xorbit/LiFePo4owered-Pi.git#this repo does 
>> not exist
>> 
>> Git will then prompt for username and password on Github.
>> 
>> I can see a valid data-leak concern (one could probe for private repository 
>> names in a brute-force fashion), but then again the UX impact is appalling. 
>> Chances of someone typing an invalid repo name are pretty high, and this 
>> error message has nothing to do with the actual error.
>> 
>> RESOLUTION:
>> The error message should indicate that the repository name does not exist.
> 
> This is a legitimate thing to complain about, but it has nothing to do
> with git itself maintained on this mailing list, but the response codes
> of specific git hosting websites. E.g. here's two issues for fixing this
> on GitLab:
> 
> https://gitlab.com/gitlab-org/gitlab-ce/issues/50201
> https://gitlab.com/gitlab-org/gitlab-ce/issues/50660
> 
> These hosting platforms are intentionally producing bad error messages
> to not leak information, as you note.
> 
> So I doubt it's something they'll ever change, the bug I have open with
> this on GitLab is to make this configurable for privately run instances.
> 



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

2018-11-11 Thread Ævar Arnfjörð Bjarmason


On Sun, Nov 11 2018, Duy Nguyen wrote:

> On Sun, Nov 11, 2018 at 1:33 PM Ævar Arnfjörð Bjarmason
>  wrote:
>> The users who need protection against git deleting their files the most
>> are exactly the sort of users who aren't expert-level enough to
>> understand the nuances of how the semantics of .gitignore and "precious"
>> are going to interact before git eats their data.
>>
>> This is pretty apparent from the bug reports we're getting about
>> this. None of them are:
>>
>> "Hey, I 100% understood .gitignore semantics including this one part
>> of the docs where you say you'll do this, but just forgot one day
>> and deleted my work. Can we get some more safety?"
>>
>> But rather (with some hyperbole for effect):
>>
>> "ZOMG git deleted my file! Is this a bug??"
>>
>> So I think we should have the inverse of this "precious"
>> attribute". Just a change to the docs to say that .gitignore doesn't
>> imply these eager deletion semantics on tree unpacking anymore, and if
>> users want it back they can define a "garbage" attribute
>> (s/precious/garbage/).
>>
>> That will lose no data, and in the very rare cases where a checkout of
>> tracked files would overwrite an ignored pattern, we can just error out
>> (as we do with the "Ok to overwrite" branch removed) and tell the user
>> to delete the files to proceed.
>
> There's also the other side of the coin. If this refuse to overwrite
> triggers too often, it can become an annoyance. So far I've seen two
> reports of accident overwriting which make me think turning precious
> to trashable may be too extreme. Plus ignored files are trashable by
> default (or at least by design so far), adding trashable attribute
> changes how we handle ignored files quite significantly.

Yeah I'm not trying to make the argument that we should just go with
these user bug reports, clearly that's just going to give us selection
bias and we could continue to flip between the two behaviors with that
approach. Just that an advanced opt-in feature to prevent dataloss will
not prevent it in practice.

Is taking my patch the right thing? I don't know. I'm leaning in that
direction, but more making a devil's advocate argument to see if anyone
finds good cases that'll demonstrate how it's bad. I haven't read/seen
them so far, and the test suite didn't have any.

I did go through the list archives as Junio suggested in
https://public-inbox.org/git/7viq39avay@alter.siamese.dyndns.org/
and found these two:
https://public-inbox.org/git/?q=d%3A20070301..20070331+verify_absent

It seems to me that the reason we ended up with this behavior is a bug
report from Shawn that was describing a similar but not quite the same
problem:

"[...]a bug in read-tree -m that prevents him from switching
branches when the type of a path changes between a directory and a
file.[...]"

That's not the same as when a now-tracked file clobbers a .gitignored
file. As far as I can tell (but may not have read carefully enough) that
wasn't a problem anyone reported, but was changed while fixing another
bug in c81935348b ("Fix switching to a branch with D/F when current
branch has file D.", 2007-03-15).


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

2018-11-11 Thread Duy Nguyen
On Sun, Nov 11, 2018 at 1:33 PM Ævar Arnfjörð Bjarmason
 wrote:
> The users who need protection against git deleting their files the most
> are exactly the sort of users who aren't expert-level enough to
> understand the nuances of how the semantics of .gitignore and "precious"
> are going to interact before git eats their data.
>
> This is pretty apparent from the bug reports we're getting about
> this. None of them are:
>
> "Hey, I 100% understood .gitignore semantics including this one part
> of the docs where you say you'll do this, but just forgot one day
> and deleted my work. Can we get some more safety?"
>
> But rather (with some hyperbole for effect):
>
> "ZOMG git deleted my file! Is this a bug??"
>
> So I think we should have the inverse of this "precious"
> attribute". Just a change to the docs to say that .gitignore doesn't
> imply these eager deletion semantics on tree unpacking anymore, and if
> users want it back they can define a "garbage" attribute
> (s/precious/garbage/).
>
> That will lose no data, and in the very rare cases where a checkout of
> tracked files would overwrite an ignored pattern, we can just error out
> (as we do with the "Ok to overwrite" branch removed) and tell the user
> to delete the files to proceed.

There's also the other side of the coin. If this refuse to overwrite
triggers too often, it can become an annoyance. So far I've seen two
reports of accident overwriting which make me think turning precious
to trashable may be too extreme. Plus ignored files are trashable by
default (or at least by design so far), adding trashable attribute
changes how we handle ignored files quite significantly.
-- 
Duy


Re: BUG REPORT: git clone of non-existent repository results in request for credentials

2018-11-11 Thread Ævar Arnfjörð Bjarmason


On Sun, Nov 11 2018, Federico Lucifredi wrote:

> git clone of non-existent repository results in request for credentials
>
> REPRODUCING:
> sudo apt install git
> git clone https://github.com/xorbit/LiFePo4owered-Pi.git#this repo does 
> not exist
>
> Git will then prompt for username and password on Github.
>
> I can see a valid data-leak concern (one could probe for private repository 
> names in a brute-force fashion), but then again the UX impact is appalling. 
> Chances of someone typing an invalid repo name are pretty high, and this 
> error message has nothing to do with the actual error.
>
> RESOLUTION:
> The error message should indicate that the repository name does not exist.

This is a legitimate thing to complain about, but it has nothing to do
with git itself maintained on this mailing list, but the response codes
of specific git hosting websites. E.g. here's two issues for fixing this
on GitLab:

https://gitlab.com/gitlab-org/gitlab-ce/issues/50201
https://gitlab.com/gitlab-org/gitlab-ce/issues/50660

These hosting platforms are intentionally producing bad error messages
to not leak information, as you note.

So I doubt it's something they'll ever change, the bug I have open with
this on GitLab is to make this configurable for privately run instances.


grep issues

2018-11-11 Thread Orgad Shaneh
Hi,

I found 2 bugs in grep, using Git for Windows 2.19.1 (but noticed
these several versions ago):

1. git grep --recursive on a worktree (without rev) always matches
against the submodule's HEAD, not its worktree, as it should.
2. When core.autocrlf (or eol=crlf) is used, and a file in the
worktree has CRLF, git grep fails to match $ against EOL.

For example:
git init
echo 'file eol=crlf' > .gitattributes
echo ABCD > file
git add file
git commit -m 'CRLF'
rm file
git checkout -f file
git grep 'D$' file # Nothing
git grep 'D$' HEAD -- file # Found!

- Orgad


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

2018-11-11 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> Also while "precious" is a fun name, but it does not sound serious.
> Any suggestions? Perhaps "valuable"?

FWIW, I am reasonably sure that I was the first in Git circle who
used the term "precious" in discussions wrt .gitignore, i.e. "Git
has ignored but not precious category".  Since it was not my
invention but was a borrowed term from tla (aka GNU arch), I'd
suggest to keep using that term, unless there is a strong reason not
to follow longstanding precedent.


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

2018-11-11 Thread Ævar Arnfjörð Bjarmason


On Sun, Nov 11 2018, Ævar Arnfjörð Bjarmason wrote:

> [CC-ing some of the people involved in recent threads about this]
>
> On Sun, Nov 11 2018, Nguyễn Thái Ngọc Duy wrote:
>
>> Since this topic has come up twice recently, I revisited this
>> "precious" thingy that I started four years ago and tried to see if I
>> could finally finish it. There are a couple things to be sorted out...
>>
>> 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.
>>
>> Is there any other parts of Git that should be made aware of this
>> "precious" attribute?
>>
>> Also while "precious" is a fun name, but it does not sound serious.
>> Any suggestions? Perhaps "valuable"?
>>
>> Very lightly tested. The patch is more to have something to discuss
>> than is bug free and ready to use.
>>
>> (*) Note that tracked files could be marked "precious" in the future
>> too although the exact semantics is not very clear since tracked
>> files are by default precious.
>>
>> But something like "index log" could use this to record all
>> changes to precious files instead of just "git add -p" changes,
>> for example. So these files are in a sense more precious than
>> other tracked files.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>>  Documentation/git-clean.txt |  3 ++-
>>  Documentation/gitattributes.txt | 13 +
>>  attr.c  |  9 +
>>  attr.h  |  2 ++
>>  builtin/clean.c | 19 ---
>>  unpack-trees.c  |  3 ++-
>>  6 files changed, 44 insertions(+), 5 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..c722479bdc 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/attr.c b/attr.c
>> index 60d284796d..d06ca0ae4b 100644
>> --- a/attr.c
>> +++ b/attr.c
>> @@ -1186,3 +1186,12 @@ void attr_start(void)
>>  pthread_mutex_init(_vector.mutex, NULL);
>>  #endif
>>  }
>> +
>> +int is_precious_file(struct index_state *istate, const char *path)
>> +{
>> +static struct attr_check *check;
>> +if (!check)
>> +check = attr_check_initl("precious", NULL);
>> +git_check_attr(istate, path, check);
>> +return check && ATTR_TRUE(check->items[0].value);
>> +}
>
> If we merge two branches is this using the merged post-image of
> .gitattributes as a source?
>
>>  if (o->dir &&
>> -is_excluded(o->dir, o->src_index, name, ))
>> +is_excluded(o->dir, o->src_index, name, ) &&
>> +!is_precious_file(o->src_index, name))
>>  /*
>>   * ce->name is explicitly excluded, so it is Ok to
>>   * overwrite it.
>
> I wonder if instead we should just be reverting c81935348b ("Fix
> switching to a branch with D/F when current branch has file D.",
> 2007-03-15), which these days (haven't dug deeply) would just be this,
> right?:
>
>>diff --git a/unpack-trees.c b/unpack-trees.c
> index 7570df481b..b3efaddd4f 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1894,13 +1894,6 @@ static int check_ok_to_remove(const char *name, 
> int len, int dtype,
>   if (ignore_case && icase_exists(o, name, len, st))
>   return 0;
>
> - if (o->dir &&
> - is_excluded(o->dir, o->src_index, name, ))
> - /*
> -  * ce->name is explicitly excluded, so it is Ok to
> -  * overwrite it.
> -  

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

2018-11-11 Thread Ævar Arnfjörð Bjarmason


[CC-ing some of the people involved in recent threads about this]

On Sun, Nov 11 2018, Nguyễn Thái Ngọc Duy wrote:

> Since this topic has come up twice recently, I revisited this
> "precious" thingy that I started four years ago and tried to see if I
> could finally finish it. There are a couple things to be sorted out...
>
> 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.
>
> Is there any other parts of Git that should be made aware of this
> "precious" attribute?
>
> Also while "precious" is a fun name, but it does not sound serious.
> Any suggestions? Perhaps "valuable"?
>
> Very lightly tested. The patch is more to have something to discuss
> than is bug free and ready to use.
>
> (*) Note that tracked files could be marked "precious" in the future
> too although the exact semantics is not very clear since tracked
> files are by default precious.
>
> But something like "index log" could use this to record all
> changes to precious files instead of just "git add -p" changes,
> for example. So these files are in a sense more precious than
> other tracked files.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  Documentation/git-clean.txt |  3 ++-
>  Documentation/gitattributes.txt | 13 +
>  attr.c  |  9 +
>  attr.h  |  2 ++
>  builtin/clean.c | 19 ---
>  unpack-trees.c  |  3 ++-
>  6 files changed, 44 insertions(+), 5 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..c722479bdc 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/attr.c b/attr.c
> index 60d284796d..d06ca0ae4b 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -1186,3 +1186,12 @@ void attr_start(void)
>   pthread_mutex_init(_vector.mutex, NULL);
>  #endif
>  }
> +
> +int is_precious_file(struct index_state *istate, const char *path)
> +{
> + static struct attr_check *check;
> + if (!check)
> + check = attr_check_initl("precious", NULL);
> + git_check_attr(istate, path, check);
> + return check && ATTR_TRUE(check->items[0].value);
> +}

If we merge two branches is this using the merged post-image of
.gitattributes as a source?

>   if (o->dir &&
> - is_excluded(o->dir, o->src_index, name, ))
> + is_excluded(o->dir, o->src_index, name, ) &&
> + !is_precious_file(o->src_index, name))
>   /*
>* ce->name is explicitly excluded, so it is Ok to
>* overwrite it.

I wonder if instead we should just be reverting c81935348b ("Fix
switching to a branch with D/F when current branch has file D.",
2007-03-15), which these days (haven't dug deeply) would just be this,
right?:

>diff --git a/unpack-trees.c b/unpack-trees.c
index 7570df481b..b3efaddd4f 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1894,13 +1894,6 @@ static int check_ok_to_remove(const char *name, int 
len, int dtype,
if (ignore_case && icase_exists(o, name, len, st))
return 0;

-   if (o->dir &&
-   is_excluded(o->dir, o->src_index, name, ))
-   /*
-* ce->name is explicitly excluded, so it is Ok to
-* overwrite it.
-*/
-   return 0;
if (S_ISDIR(st->st_mode)) {
/*
 * We are checking out path "foo" and

Something like the approach you're 

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

2018-11-11 Thread Bert Wesarg
On Sun, Nov 11, 2018 at 10:53 AM Nguyễn Thái Ngọc Duy  wrote:
>
> Also while "precious" is a fun name, but it does not sound serious.
> Any suggestions? Perhaps "valuable"?

"precious" is also used by POSIX make:

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/make.html

Bert


Price enquiry

2018-11-11 Thread Daniel Murray
Hi,friend,
 
This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia.
We are glad to know about your company from the web and we are interested in 
your products.
Could you kindly send us your Latest catalog and price list for our trial order.
 
Best Regards,
 
Daniel Murray
Purchasing Manager




Re: [PATCH] builtin/notes: remove unnecessary free

2018-11-11 Thread Johan Herland
On Sun, Nov 11, 2018 at 10:49 AM Carlo Marcelo Arenas Belón
 wrote:
>
> 511726e4b1 ("builtin/notes: fix premature failure when trying to add
> the empty blob", 2014-11-09) removed the check for !len but left a
> call to free the buffer that will be otherwise NULL
>
> Signed-off-by: Carlo Marcelo Arenas Belón 

Signed-off-by: Johan Herland 

> ---
>  builtin/notes.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/builtin/notes.c b/builtin/notes.c
> index c05cd004ab..68062f7475 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -255,10 +255,8 @@ static int parse_reuse_arg(const struct option *opt, 
> const char *arg, int unset)
>
> if (get_oid(arg, ))
> die(_("failed to resolve '%s' as a valid ref."), arg);
> -   if (!(buf = read_object_file(, , ))) {
> -   free(buf);
> +   if (!(buf = read_object_file(, , )))
> die(_("failed to read object '%s'."), arg);
> -   }
> if (type != OBJ_BLOB) {
> free(buf);
> die(_("cannot read note data from non-blob object '%s'."), 
> arg);
> --
> 2.19.1.856.g8858448bb
>


-- 
Johan Herland, 
www.herland.net


Re: [PATCH v2 1/1] Upcast size_t variables to uintmax_t when printing

2018-11-11 Thread Torsten Bögershausen
On Sun, Nov 11, 2018 at 02:28:35AM -0500, Jeff King wrote:
> On Sun, Nov 11, 2018 at 08:05:04AM +0100, tbo...@web.de wrote:
> 
> > From: Torsten Bögershausen 
> > 
> > When printing variables which contain a size, today "unsigned long"
> > is used at many places.
> > In order to be able to change the type from "unsigned long" into size_t
> > some day in the future, we need to have a way to print 64 bit variables
> > on a system that has "unsigned long" defined to be 32 bit, like Win64.
> > 
> > Upcast all those variables into uintmax_t before they are printed.
> > This is to prepare for a bigger change, when "unsigned long"
> > will be converted into size_t for variables which may be > 4Gib.
> 
> I like the overall direction. I feel a little funny doing this step now,
> and not as part of a series to convert individual variables. But I
> cannot offhand think of any reason that it would behave badly even if
> the other part does not materialize
> 

Hej all,
There may be some background information missing:
- I did a 2-patch series based on this commit in pu:
commit 37c59c3e8fac8bae7ccc5baa148b0e9bae0c8d65
Author: Junio C Hamano 
Date:   Sat Oct 27 16:42:25 2018 +0900

treewide: apply cocci patch

(that patch was never send out, see below)

The week later, I tried to apply it on pu, but that was nearly hopeless,
as too much things had changed on pu.
I had the chance to compile & test it, but decided to take "part2" before
"part1", so to say:
Fix all the printing, and wait for the master branch to settle,
and then do the "unsigned long" -> size_t conversion.
That will probably happen after 2.20.

At the moment, the big "unsigned long" -> size_t conversion is dependend on
 - mk/use-size-t-in-zlib
 - sb/more-repo-in-api,
 - jk/xdiff-interface'
 (and probably more stuff from Duy and others)

How should we handle the big makeover ?

> -Peff

And thanks for reading my stuff


[RFC PATCH] Introduce "precious" file concept

2018-11-11 Thread Nguyễn Thái Ngọc Duy
Since this topic has come up twice recently, I revisited this
"precious" thingy that I started four years ago and tried to see if I
could finally finish it. There are a couple things to be sorted out...

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.

Is there any other parts of Git that should be made aware of this
"precious" attribute?

Also while "precious" is a fun name, but it does not sound serious.
Any suggestions? Perhaps "valuable"?

Very lightly tested. The patch is more to have something to discuss
than is bug free and ready to use.

(*) Note that tracked files could be marked "precious" in the future
too although the exact semantics is not very clear since tracked
files are by default precious.

But something like "index log" could use this to record all
changes to precious files instead of just "git add -p" changes,
for example. So these files are in a sense more precious than
other tracked files.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-clean.txt |  3 ++-
 Documentation/gitattributes.txt | 13 +
 attr.c  |  9 +
 attr.h  |  2 ++
 builtin/clean.c | 19 ---
 unpack-trees.c  |  3 ++-
 6 files changed, 44 insertions(+), 5 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..c722479bdc 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/attr.c b/attr.c
index 60d284796d..d06ca0ae4b 100644
--- a/attr.c
+++ b/attr.c
@@ -1186,3 +1186,12 @@ void attr_start(void)
pthread_mutex_init(_vector.mutex, NULL);
 #endif
 }
+
+int is_precious_file(struct index_state *istate, const char *path)
+{
+   static struct attr_check *check;
+   if (!check)
+   check = attr_check_initl("precious", NULL);
+   git_check_attr(istate, path, check);
+   return check && 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 8d9a7dc206..9e554448a6 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");
@@ -152,6 +155,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;
 
@@ -191,9 +195,15 @@ static int remove_dirs(struct strbuf *path, const char 
*prefix, int force_flag,
 

[PATCH] builtin/notes: remove unnecessary free

2018-11-11 Thread Carlo Marcelo Arenas Belón
511726e4b1 ("builtin/notes: fix premature failure when trying to add
the empty blob", 2014-11-09) removed the check for !len but left a
call to free the buffer that will be otherwise NULL

Signed-off-by: Carlo Marcelo Arenas Belón 
---
 builtin/notes.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index c05cd004ab..68062f7475 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -255,10 +255,8 @@ static int parse_reuse_arg(const struct option *opt, const 
char *arg, int unset)
 
if (get_oid(arg, ))
die(_("failed to resolve '%s' as a valid ref."), arg);
-   if (!(buf = read_object_file(, , ))) {
-   free(buf);
+   if (!(buf = read_object_file(, , )))
die(_("failed to read object '%s'."), arg);
-   }
if (type != OBJ_BLOB) {
free(buf);
die(_("cannot read note data from non-blob object '%s'."), arg);
-- 
2.19.1.856.g8858448bb



Price Inquiry

2018-11-11 Thread Daniel Murray
Hi,friend,
 
This is Daniel Murray and i am from Sinara Group Co.Ltd Group Co.,LTD in Russia.
We are glad to know about your company from the web and we are interested in 
your products.
Could you kindly send us your Latest catalog and price list for our trial order.
 
Best Regards,
 
Daniel Murray
Purchasing Manager




BUG REPORT: git clone of non-existent repository results in request for credentials

2018-11-11 Thread Federico Lucifredi
git clone of non-existent repository results in request for credentials

REPRODUCING:
sudo apt install git
git clone https://github.com/xorbit/LiFePo4owered-Pi.git#this repo does not 
exist

Git will then prompt for username and password on Github.

I can see a valid data-leak concern (one could probe for private repository 
names in a brute-force fashion), but then again the UX impact is appalling. 
Chances of someone typing an invalid repo name are pretty high, and this error 
message has nothing to do with the actual error.

RESOLUTION:
The error message should indicate that the repository name does not exist. 


Best -F



_
-- "'Problem' is a bleak word for challenge" - Richard Fish
(Federico L. Lucifredi) - flucifredi at acm.org - GnuPG 0x4A73884C



Re: [PATCH 00/10] fast export and import fixes and features

2018-11-11 Thread Elijah Newren
On Sat, Nov 10, 2018 at 11:27 PM Jeff King  wrote:
>
> On Sat, Nov 10, 2018 at 10:23:02PM -0800, Elijah Newren wrote:
>
> > This is a series of ten patches representing two doc corrections, one
> > pedantic fix, three real bug fixes, one micro code refactor, and three
> > new features.  Each of these ten changes is relatively small in size.
> > These changes predominantly affect fast-export, but there's a couple
> > small changes for fast-import as well.
> >
> > I could potentially split these patches up, but I'd just end up
> > chaining them sequentially since otherwise there'd be lots of
> > conflicts; having 10 different single patch series with lots of
> > dependencies sounded like a bigger pain to me, but let me know if you
> > would prefer I split them up and how you suggest doing so.
>
> I think it's fine to put them in sequence when there's a textual
> dependency.  If it turns out that one of them needs more discussion and
> we don't want it to hold later patches hostage, we can always re-roll at
> that point.
>
> (I also think it's fine to lump together thematically similar patches
> even when they aren't strictly dependent, even textually. It's less work
> for the maintainer to consider 1 group of 10 than 10 groups of 1).
>
> > These patches were driven by the needs of git-repo-filter[1], but most
> > if not all of them should be independently useful.
>
> I left lots of comments. Some of the earlier ones may just be showing my
> confusion about fast-export works (some of which was cleared up by your
> later patches). But I like the overall direction for sure.

Thanks for taking the time to read over the series and providing lots
of feedback!  And, whoops, looks like it's gotten kinda late, so I'll
check any further feedback on Monday.


Re: [PATCH v4 3/3] range-diff: make diff option behavior (e.g. --stat) consistent

2018-11-11 Thread Eric Sunshine
On Fri, Nov 9, 2018 at 5:18 AM Ævar Arnfjörð Bjarmason  wrote:
> Make the behavior when diff options (e.g. "--stat") are passed
> consistent with how "diff" behaves.
> [...]
> Signed-off-by: Ævar Arnfjörð Bjarmason 
> ---
> diff --git a/range-diff.c b/range-diff.c
> @@ -453,7 +453,8 @@ int show_range_diff(const char *range1, const char 
> *range2,
> -   opts.output_format |= DIFF_FORMAT_PATCH;
> +   if (!opts.output_format)
> +   opts.output_format |= DIFF_FORMAT_PATCH;

I think this can just be '=' now instead of '|=' (to avoid confusing
the reader, even if it's functionally equivalent).


Re: [PATCH 10/10] fast-export: add --always-show-modify-after-rename

2018-11-11 Thread Elijah Newren
On Sat, Nov 10, 2018 at 11:23 PM Jeff King  wrote:
>
> On Sat, Nov 10, 2018 at 10:23:12PM -0800, Elijah Newren wrote:
>
> > fast-export output is traditionally used as an input to a fast-import
> > program, but it is also useful to help gather statistics about the
> > history of a repository (particularly when --no-data is also passed).
> > For example, two of the types of information we may want to collect
> > could include:
> >   1) general information about renames that have occurred
> >   2) what the biggest objects in a repository are and what names
> >  they appear under.
> >
> > The first bit of information can be gathered by just passing -M to
> > fast-export.  The second piece of information can partially be gotten
> > from running
> > git cat-file --batch-check --batch-all-objects
> > However, that only shows what the biggest objects in the repository are
> > and their sizes, not what names those objects appear as or what commits
> > they were introduced in.  We can get that information from fast-export,
> > but when we only see
> > R oldname newname
> > instead of
> > R oldname newname
> > M 100644 $SHA1 newname
> > then it makes the job more difficult.  Add an option which allows us to
> > force the latter output even when commits have exact renames of files.
>
> fast-export seems like a funny tool to look up paths. What about "git
> log --find-object=$SHA1" ?

Eek, and give me O(N*M) behavior, where N is the number of commits in
the repository and M is the number of renames that occur in its
history?  Also, that's the inverse of the lookup I need anyway (I have
the commit and filename, but am missing the SHA).

One of the problems with filter-branch that people often run into is
they know what they want at a high-level (e.g. extract the history of
this directory for a new repository, or rewrite the history of this
repo to appear at a subdirectory so it can be merged into a bigger
repo and people passing filenames to log will still get the history of
those files, or I want to remove some of the big stuff in my history),
but often times that's not quite enough.  They need help finding big
objects, or may be unaware that the subset of files they want used to
be known by alternative names.

I want a simple --analyze mode that can report on all files that have
been renamed (so users don't just say "all I care about is these N
files, give me a rewritten history just including those" -- we can
point out to them whether those N files used to be known by other
names), as well as reporting on all big files and if they've been
deleted, and aggregations of the "big files" information across
directories and file extensions.


Re: [PATCH 09/10] fast-export: add a --show-original-ids option to show original names

2018-11-11 Thread Elijah Newren
On Sat, Nov 10, 2018 at 11:20 PM Jeff King  wrote:
>
> On Sat, Nov 10, 2018 at 10:23:11PM -0800, Elijah Newren wrote:
>
> > Knowing the original names (hashes) of commits, blobs, and tags can
> > sometimes enable post-filtering that would otherwise be difficult or
> > impossible.  In particular, the desire to rewrite commit messages which
> > refer to other prior commits (on top of whatever other filtering is
> > being done) is very difficult without knowing the original names of each
> > commit.
> >
> > This commit teaches a new --show-original-ids option to fast-export
> > which will make it add a 'originally ' line to blob, commits, and
> > tags.  It also teaches fast-import to parse (and ignore) such lines.
>
> Makes sense as a feature; I think filter-branch can make its mappings
> available, too.
>
> Do we need to worry about compatibility with other fast-import programs?
> I think no, because this is not enabled by default (so if sending the
> extra lines to another importer hurts, the answer is "don't do that").
>
> I have a vague feeling that there might be some way to combine this with
> --export-marks or --no-data, but I can't really think of a way. They
> seem related, but not quite.
>
> > ---
> >  Documentation/git-fast-export.txt |  7 +++
> >  builtin/fast-export.c | 20 +++-
> >  fast-import.c | 17 +
> >  t/t9350-fast-export.sh| 17 +
> >  4 files changed, 56 insertions(+), 5 deletions(-)
>
> The fast-import format is documented in Documentation/git-fast-import.txt.
> It might need an update to cover the new format.

We document the format in both fast-import.c and
Documentation/git-fast-import.txt?  Maybe we should delete the long
comments in fast-import.c so this isn't duplicated?

> > --- a/Documentation/git-fast-export.txt
> > +++ b/Documentation/git-fast-export.txt
> > @@ -121,6 +121,13 @@ marks the same across runs.
> >   used by a repository which already contains the necessary
> >   parent commits.
> >
> > +--show-original-ids::
> > + Add an extra directive to the output for commits and blobs,
> > + `originally `.  While such directives will likely be
> > + ignored by importers such as git-fast-import, it may be useful
> > + for intermediary filters (e.g. for rewriting commit messages
> > + which refer to older commits, or for stripping blobs by id).
>
> I'm not quite sure how a blob ends up being rewritten by fast-export (I
> get that commits may change due to dropping parents).

It doesn't get rewritten by fast-export; it gets rewritten by other
intermediary filters, e.g. in something like this:

   git fast-export --show-original-ids --all | intermediary_filter |
git fast-import

The intermediary_filter program may want to strip out blobs by id, or
remove filemodify and filedelete directives unless they touch certain
paths, etc.

> The name "originally" doesn't seem great to me. Probably because I would
> continually wonder if it has one "l" or two. ;) Perhaps something like
> "original-oid" might be better. That's well into bikeshed territory,
> though.

I wasn't a huge fan of "originally" either, but I just couldn't come
up with anything else that wasn't really long.  I'd be happy to switch
to original-oid.


Re: [PATCH 07/10] fast-export: ensure we export requested refs

2018-11-11 Thread Elijah Newren
On Sat, Nov 10, 2018 at 11:02 PM Jeff King  wrote:
>
> On Sat, Nov 10, 2018 at 10:23:09PM -0800, Elijah Newren wrote:
>
> > If file paths are specified to fast-export and a ref points to a commit
> > that does not touch any of the relevant paths, then that ref would
> > sometimes fail to be exported.  (This depends on whether any ancestors
> > of the commit which do touch the relevant paths would be exported with
> > that same ref name or a different ref name.)  To avoid this problem,
> > put *all* specified refs into extra_refs to start, and then as we export
> > each commit, remove the refname used in the 'commit $REFNAME' directive
> > from extra_refs.  Then, in handle_tags_and_duplicates() we know which
> > refs actually do need a manual reset directive in order to be included.
> >
> > This means that we do need some special handling for excluded refs; e.g.
> > if someone runs
> >git fast-export ^master master
> > then they've asked for master to be exported, but they have also asked
> > for the commit which master points to and all of its history to be
> > excluded.  That logically means ref deletion.  Previously, such refs
> > were just silently omitted from being exported despite having been
> > explicitly requested for export.
>
> Hmm. Reading this it makes sense to me, but I remember from discussion
> long ago that there were a lot of funny corner cases around "which refs
> to include" and possibly even some ambiguous cases. Maybe that is all
> sorted these days, with --refspec.

Oh yeah, there definitely were some funny corner cases around "which
refs to include" (though I don't think --refspec affects this, either
before or after my patch.)  Before this commit, fast-export would
often emit unnecessary reset directives at the end, AND fail to export
some other refs that had been explicitly requested for export.  It had
some simple logic to attempt to cover the cases, but it was just
wrong.  As far as I can tell, this patch fixes all of those.

...well, almost all.  We still fail on tags of tags of commits (or
higher level nestings), but that's a multi-pronged issue that feels
like a different beast. (We rewrite tags of tags of commits to just be
tags of commits, even without any special request from the user
somewhat contrary to otherwise requiring --signed-tags and
--tag-of-filtered-object options.  As far as I can tell, this isn't
documented for fast-export but I saw somewhere in the filter-branch
docs where it said it does this kind of thing on purpose.  However, to
make it even weirder, if the user requests
--tag-of-filtered-object=rewrite instead of the default of "abort"
then we actually abort on tags-of-tags-of-commits instead of
rewriting.  I don't think it was intentional, but
tags-of-tags-of-commits inverts the meaning of the
--tag-of-filtered-object={rewrite vs. abort} flag -- it's very weird).
I put more time into attempting to fix the nested tags issue than I
feel like it was worth.  git.git is the only repo I know of that seems
to have such tags, so I just gave up on them for now.

> > ---
> > NOTE: I was hoping the strmap API proposal would materialize, but I either
> > missed it or it hasn't shown up.  The usage of string_list in this patch
> > would be better replaced by what Peff suggested.
>
> You didn't miss it. Junio did some manual conversions using hashmap,
> which weren't too bad.  It's not entirely clear to me how often we'd be
> able to use strmap instead of a full-on hashmap, so I haven't really
> pursued it.
>
> It looks like you generate the list here via append, and then sort at
> the end. That's at least not quadratic. I think the string_list_remove()
> is, though.

I think it would have been useful in multiple places in
merge-recursive.c, in addition to here.  Maybe that just means I need
to add strmap to my list of things to do.


Re: [PATCH 06/10] fast-export: when using paths, avoid corrupt stream with non-existent mark

2018-11-11 Thread Elijah Newren
On Sat, Nov 10, 2018 at 10:53 PM Jeff King  wrote:
>
> On Sat, Nov 10, 2018 at 10:23:08PM -0800, Elijah Newren wrote:
>
> > If file paths are specified to fast-export and multiple refs point to a
> > commit that does not touch any of the relevant file paths, then
> > fast-export can hit problems.  fast-export has a list of additional refs
> > that it needs to explicitly set after exporting all blobs and commits,
> > and when it tries to get_object_mark() on the relevant commit, it can
> > get a mark of 0, i.e. "not found", because the commit in question did
> > not touch the relevant paths and thus was not exported.  Trying to
> > import a stream with a mark corresponding to an unexported object will
> > cause fast-import to crash.
> >
> > Avoid this problem by taking the commit the ref points to and finding an
> > ancestor of it that was exported, and make the ref point to that commit
> > instead.
>
> As with the earlier tag commit, I wonder if this might depend on the
> context in which you're using fast-export. I suppose that if you did not
> feed the ref on the command line that we would not be dealing with it at
> all (and maybe that is the answer to my question about the tag thing,
> too).

Right, if you didn't feed the ref on the command line, we're not
dealing with the ref at all, so the code here doesn't affect any such
ref.

> It does seem funny that the behavior for the earlier case (bounded
> commits) and this case (skipping some commits) are different. Would you
> ever want to keep walking backwards to find an ancestor in the earlier
> case? Or vice versa, would you ever want to simply delete a tag in a
> case like this one?
>
> I'm not sure sure, but I suspect you may have thought about it a lot
> harder than I have. :)

I'm not sure why you thought the behavior for the two cases was
different?  For both patches, my testcases used path limiting; it was
you who suggested employing a negative revision to bound the commits.

Anyway, for both patches assuming you haven't bounded the commits, you
can attempt to keep walking backwards to find an earlier ancestor, but
the fundamental fact is you aren't guaranteed that you can find one
(i.e. some tag or branch points to a commit that didn't modify any of
the specified paths, and nor did any of its ancestors back to any root
commits).  I hit that case lots of times.  If the user explicitly
requested a tag or branch for export (and requested tag rewriting),
and limited to certain paths that had never existed in the repository
as of the time of the tag or branch, then you hit the cases these
patches worry about.  Patch 4 was about (annotated and signed) tags,
this patch is about unannotated tags and branches and other refs.

If you think about using negative revisions, for both cases, then
again you can keep walking back history to try to find a commit that
your tag or branch or ref can point to, but if you get back to the
negative revisions, then you are in the range the user requested to be
omitted from the resulting repository.  Sounds like tag/ref deletion
to me.

>
> > diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> > index a3c044b0af..5648a8ce9c 100644
> > --- a/builtin/fast-export.c
> > +++ b/builtin/fast-export.c
> > @@ -900,7 +900,18 @@ static void handle_tags_and_duplicates(void)
> >   if (anonymize)
> >   name = anonymize_refname(name);
> >   /* create refs pointing to already seen commits */
> > - commit = (struct commit *)object;
> > + commit = rewrite_commit((struct commit *)object);
> > + if (!commit) {
> > + /*
> > +  * Neither this object nor any of its
> > +  * ancestors touch any relevant paths, so
> > +  * it has been filtered to nothing.  Delete
> > +  * it.
> > +  */
> > + printf("reset %s\nfrom %s\n\n",
> > +name, sha1_to_hex(null_sha1));
> > + continue;
> > + }
>
> This hunk makes sense.

Cool, this was the entirety of the code...so does this mean that the
code makes more sense than my commit message summary did?  ...and
perhaps that my attempts to answer your questions in this email
weren't necessary anymore?

> > --- a/t/t9350-fast-export.sh
> > +++ b/t/t9350-fast-export.sh
> > @@ -386,6 +386,30 @@ test_expect_success 'path limiting with import-marks 
> > does not lose unmodified fi
> >   grep file0 actual
> >  '
> >
> > +test_expect_success 'avoid corrupt stream with non-existent mark' '
> > + test_create_repo avoid_non_existent_mark &&
> > + (
> > + cd avoid_non_existent_mark &&
> > +
> > + touch important-path &&
> > + git add important-path &&
> > +