Re: [PATCH v2] for-each-ref: add %(upstream:gone) to mark missing refs

[Karthik Nayak]

On Thu, Aug 25, 2016 at 12:03 AM, Jeff King  wrote:
> On Wed, Aug 24, 2016 at 08:26:26PM +0200, Øystein Walle wrote:
>
>> In the mean time, however, I have discovered that this conflicts with
>> kn/ref-filter-branch-list in pu. In that topic this specific feature is
>> implemented as well. They incorporate it into %(upstream:track) instead
>> of having a separate "sub-atom" (what's the correct nomenclature, by the
>> way?) more in line with with branch -vv and your idea.
>

I'm thinking more on the lines of `%(upstream)` being an atom and the
`:track` being
an option under that atom. I like sub-atom though ;)

> Ah, right. I was feeling like this was all vaguely familiar. I think it
> would be better to push forward kn/ref-filter-branch-list. According to
> the last "what's cooking", I think that topic is waiting on more review.
> If you're willing and able to do so, that would be a big help.
>

It's been waiting for review for a _long_ time now.

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

Re: [L10N] Kickoff of translation for Git 2.10.0 round 1

[Alex Henrie]

2016-08-20 10:01 GMT-06:00 Jean-Noël AVILA :
> 2.  in sequencer.c, there is a mistake in the original string to translate
> "Cannot revert during a another revert"

There's also "In both case" in git-rebase--interactive.sh.

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

Re: [PATCHv5 8/8] clone: recursive and reference option triggers submodule alternates

[Jacob Keller]

On Wed, Aug 24, 2016 at 3:52 PM, Stefan Beller  wrote:
> On Tue, Aug 23, 2016 at 11:29 PM, Jacob Keller  wrote:
>> On Tue, Aug 23, 2016 at 4:03 PM, Stefan Beller  wrote:
> +
> +   if (option_recursive) {
> +   if (option_required_reference.nr &&
> +   option_optional_reference.nr)
> +   die(_("clone --recursive is not compatible with "
> + "both --reference and 
> --reference-if-able"));

 So if you have multiple references that don't all match we basically
 just refuse to allow recursive?

 Would it be better to simply assume that we want to die on missing
 references instead of failing the clone here?
>>>
>>> The new config options are per repo (or even set globally), and not
>>> per alternate. And as we communicate the [if-able] part via the config
>>> options to the submodules it is not feasible to transport both
>>> kinds of (reference-or-die and reference-but-ignore-misses).
>>>
>>> That is why I introduced this check in the first place. If we'd go back
>>> to the drawing board and come up with a solution that is on a
>>> "per alternate" basis we could allow such things.
>>>
 That is, treat it so
 that multiple reference and reference-if-able will die, and only info
 if we got only reference-if-able?

 Probably what's here is fine, and mixing reference and
 reference-if-able doesn't make much sense.
>>>
>>> I think the reference-if-able doesn't make sense for one project alone
>>> as you can easily script around that, but is only useful if you have
>>> submodules in a partially checked out superproject that you want
>>> to reference to.
>>>
>>> Thanks,
>>> Stefan
>>
>> I'm not sure there is a better design.  How are alternates stored? In
>> a config section?
>
> Alternates are stored in .git/objects/info/alternates
> with each alternate in a new line. On that file (from
> (man gitrepository-layout):
>
> objects/info/alternates
>
> This file records paths to alternate object stores that this object store
> borrows objects from, one pathname per line. Note that not only native
> Git tools use it locally, but the HTTP fetcher also tries to use it remotely;
> this will usually work if you have relative paths (relative to the object
> database, not to the repository!) in your alternates file, but it will not 
> work
> if you use absolute paths unless the absolute path in filesystem and web
> URL is the same. See also objects/info/http-alternates.
>
> So changing that file is out of question.
> Ideally we would have a flag for each path here, though.
>
>> Or is there some way we can store the is-able per
>> alternate and look it up when adding them to submodule?
>
> I guess we could invent a file as alternate-flags that is matches
> line by line to the alternates file.
>
> I don't think we'd want to go that way for now as it would really only
> help in an edge case?
>
> If we later find out we need the flag on a per-alternate basis we can
> still come up with a solution and just not set these config variables,
> so I think we'll be fine for now with this approach.
>

Yes that seems reasonable.

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

Re: Git for Windows documentation, was Re: [git-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

[Philip Oakley]

From: "Dakota Hawkins" 

On Wed, Aug 24, 2016 at 11:41 AM, Johannes Schindelin
 wrote:

Hi Dakota,

On Tue, 23 Aug 2016, Dakota Hawkins wrote:


I use GFW almost exclusively, but I pretty much always consult the
upstream documentation anyway (because I find it easier).


Oh... I thought that typing "git help git-commit" opening a nice HTML 
page

in your favorite browser was good enough.

Do you have any suggestion how to improve the user experience?


Just a small one, and that's that I'd prefer the option to have help
display in my terminal (that option might exist and I don't know how
to turn it on). That would be very convenient for me.

Opening a nice HTML page is probably nice for a lot of users. What
frustrates me about it is that I don't know which browser window on
which monitor (of 3) it's going to open in, so it's a context-switch
to a different window somewhere that I don't have much control over.

The thing I find easier about using the upstream documentation is just
that I can pick the browser window I want it to come up in, and it's
usually faster enough for me to just google "git-whatever" or
re-purpose an open doc tab. I don't prefer the _content_ of the
upstream documentation, it's just less frustrating for me to use, if
that makes sense.

If you would like to use the man pages, then one option is to install the 
SDK, which then allows you to install the man package, (setting the manpath 
as required) to allow your choice of viewer. You may need to set the minnty 
config BoldAsFont=yes if you want the bold for the headings in the man 
pages.


With the SDK you can also create a personal release version of GfW that 
includes the man viewer if you like.

--
Philip

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

Re: [PATCHv2] push: change submodule default to check

[Jeff King]

On Wed, Aug 24, 2016 at 03:37:29PM -0700, Stefan Beller wrote:

> > That sounds massively ... broken.  So before even thinking about
> > flipping it to default, this needs to be fixed first.
> 
> I agree. That sounds bad.
> 
> However having the --auto-check feels like papering over the
> actual problem which to me sounds like a design problem.
> However this may be a viable short term solution.

Sort of...

I may not have been clear, but there are really a few things going on.

One is that the design of find_unpushed_submodules() is just brain-dead.
It does one traversal per updated ref, which means a from-scratch mirror
is O(nr_of_refs * nr_of_commits). That's just silly, and can easily be
fixed behind the scenes to be O(nr_of_commits).

And I _suspect_ it is what made Junio's earlier push so awful; he
probably pushed up the same commits as part of many different branches,
so he did the same diffs over and over.

So clearing that up seems like an obvious first step, and dulls the pain
to "if submodule recursion is on, the worst case is that you walk all
the new objects you are sending". That's still _a_ traversal, but it's
one we have to do anyway in pack-objects, so it's the same order of
magnitude as the rest of the push[1].

Then you've got two cases: the repo is using submodules at all, or they
are not. The former is an easy case, if we can identify it; we can avoid
the traversal at all, and people who do not use submodules are not
regressed at all.

That leaves people who _are_ using submodules with paying the extra
traversal cost. Not great, but only really a pain when you have a really
big chunk of history to push. It may be lost in the noise for such a
push in more normal circumstances (where bandwidth to push up the
objects dominates, though it is unfortunate that we do not even start
utilizing the bandwidth, the critical resource, until we are done with
the submodule check).

[1] Of course with reachability bitmaps the pack-objects traversal goes
away, but the same cannot be accomplished here (because they do not
store the gitlink sha1s at all, because they do not imply
reachability).

> We need to answer the question: Which submodule commits
> are referenced by a given set of superproject commits.
> 
> This question is advancing a very similar question that we'd
> have to ask in git-gc. In gc we would end up without having to
> worry about a specific set, but rather the all reachable commits
> of the superproject are in the given set.
> 
> So we could solve two issues at the same time if we had a quick
> way to answer this question quickly.
> [...]

I snipped here because your solutions sound complicated (which isn't to
say they're wrong, but that I am not willing to give them a lot of
thought at this time in the evening ;) ).

One opposite approach which appeals to me is not to remove the need for
the traversal, but to make it much faster. E.g., by storing commits in a
form that can be traversed more quickly, and possibly keeping a bitmap
cache of which paths are touched in each commit (I have posted patches
to the list for the former, but have only been considering experimenting
with the latter).

That's _also_ complicated, but it applies to way more things. Including
normal log traversals, path-limiting for diffs, the "counting" traversal
done by pack-object, etc.

And while it is complicated in some ways, it's conceptually simple at
the git data model layer. It's returning the same old answers about
commits and trees, just faster.

Anyway, food for thought.

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

Re: [PATCHv5 8/8] clone: recursive and reference option triggers submodule alternates

[Stefan Beller]

On Tue, Aug 23, 2016 at 11:29 PM, Jacob Keller  wrote:
> On Tue, Aug 23, 2016 at 4:03 PM, Stefan Beller  wrote:
 +
 +   if (option_recursive) {
 +   if (option_required_reference.nr &&
 +   option_optional_reference.nr)
 +   die(_("clone --recursive is not compatible with "
 + "both --reference and --reference-if-able"));
>>>
>>> So if you have multiple references that don't all match we basically
>>> just refuse to allow recursive?
>>>
>>> Would it be better to simply assume that we want to die on missing
>>> references instead of failing the clone here?
>>
>> The new config options are per repo (or even set globally), and not
>> per alternate. And as we communicate the [if-able] part via the config
>> options to the submodules it is not feasible to transport both
>> kinds of (reference-or-die and reference-but-ignore-misses).
>>
>> That is why I introduced this check in the first place. If we'd go back
>> to the drawing board and come up with a solution that is on a
>> "per alternate" basis we could allow such things.
>>
>>> That is, treat it so
>>> that multiple reference and reference-if-able will die, and only info
>>> if we got only reference-if-able?
>>>
>>> Probably what's here is fine, and mixing reference and
>>> reference-if-able doesn't make much sense.
>>
>> I think the reference-if-able doesn't make sense for one project alone
>> as you can easily script around that, but is only useful if you have
>> submodules in a partially checked out superproject that you want
>> to reference to.
>>
>> Thanks,
>> Stefan
>
> I'm not sure there is a better design.  How are alternates stored? In
> a config section?

Alternates are stored in .git/objects/info/alternates
with each alternate in a new line. On that file (from
(man gitrepository-layout):

objects/info/alternates

This file records paths to alternate object stores that this object store
borrows objects from, one pathname per line. Note that not only native
Git tools use it locally, but the HTTP fetcher also tries to use it remotely;
this will usually work if you have relative paths (relative to the object
database, not to the repository!) in your alternates file, but it will not work
if you use absolute paths unless the absolute path in filesystem and web
URL is the same. See also objects/info/http-alternates.

So changing that file is out of question.
Ideally we would have a flag for each path here, though.

> Or is there some way we can store the is-able per
> alternate and look it up when adding them to submodule?

I guess we could invent a file as alternate-flags that is matches
line by line to the alternates file.

I don't think we'd want to go that way for now as it would really only
help in an edge case?

If we later find out we need the flag on a per-alternate basis we can
still come up with a solution and just not set these config variables,
so I think we'll be fine for now with this approach.

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

Re: [PATCH v14 09/27] bisect--helper: `bisect_write` shell function in C

[Junio C Hamano]

Pranit Bauva  writes:

> +struct bisect_terms {
> + struct strbuf term_good;
> + struct strbuf term_bad;
> +};

I think "struct strbuf" is overrated.  For things like this, where
these fields will never change once it is set (and setting it is
done atomically, not incrementally), there is no good reason to use
define the fields as strbuf.

Only because you chose to use strbuf for these two fields, you have
to make unnecessarily copies of argv[] in the command parser, and
you have to remember to discard these copies later.

I think you can just say "const char *" in this case.  

> +static int bisect_write(const char *state, const char *rev,
> + const struct bisect_terms *terms, int nolog)
> +{
> + struct strbuf tag = STRBUF_INIT;
> + struct strbuf commit_name = STRBUF_INIT;
> + struct object_id oid;
> + struct commit *commit;
> + struct pretty_print_context pp = {0};
> + FILE *fp;
> +
> + if (!strcmp(state, terms->term_bad.buf))
> + strbuf_addf(, "refs/bisect/%s", state);
> + else if (one_of(state, terms->term_good.buf, "skip", NULL))
> + strbuf_addf(, "refs/bisect/%s-%s", state, rev);
> + else
> + return error(_("Bad bisect_write argument: %s"), state);

OK.

> + if (get_oid(rev, )) {
> + strbuf_release();
> + return error(_("couldn't get the oid of the rev '%s'"), rev);
> + }
> +
> + if (update_ref(NULL, tag.buf, oid.hash, NULL, 0,
> +UPDATE_REFS_MSG_ON_ERR)) {
> + strbuf_release();
> + return -1;
> + }
> + strbuf_release();
> +
> + fp = fopen(git_path_bisect_log(), "a");
> + if (!fp)
> + return error_errno(_("couldn't open the file '%s'"), 
> git_path_bisect_log());
> +
> + commit = lookup_commit_reference(oid.hash);
> + format_commit_message(commit, "%s", _name, );
> + fprintf(fp, "# %s: [%s] %s\n", state, sha1_to_hex(oid.hash),
> + commit_name.buf);
> + strbuf_release(_name);
> +
> + if (!nolog)
> + fprintf(fp, "git bisect %s %s\n", state, rev);
> +
> + fclose(fp);
> + return 0;

You seem to be _release()ing tag all over the place.

Would it make sense to have a single clean-up label at the end of
function, introduce a "int retval" variable and set it to -1 (or
whatever) when an error is detected and "goto" to the label?  It may
make it harder to make such a leak.  That is, to end the function
more like:

finish:
if (fp)
fclose(fp);
strbuf_release();
strbuf_release(_name);
return retval;
}

and have sites with potential errors do something like this:

if (update_ref(...)) {
retval = -1;
goto finish;
}

> + struct bisect_terms terms;
> + bisect_terms_init();

With the type of "struct bisect_terms" members corrected, you do not
even need the _init() function.

> @@ -182,24 +251,38 @@ int cmd_bisect__helper(int argc, const char **argv, 
> const char *prefix)
>   usage_with_options(git_bisect_helper_usage, options);
>  
>   switch (cmdmode) {
> + int nolog;
>   case NEXT_ALL:
>   return bisect_next_all(prefix, no_checkout);
>   case WRITE_TERMS:
>   if (argc != 2)
>   die(_("--write-terms requires two arguments"));
> - return write_terms(argv[0], argv[1]);
> + res = write_terms(argv[0], argv[1]);
> + break;
>   case BISECT_CLEAN_STATE:
>   if (argc != 0)
>   die(_("--bisect-clean-state requires no arguments"));
> - return bisect_clean_state();
> + res = bisect_clean_state();
> + break;
>   case BISECT_RESET:
>   if (argc > 1)
>   die(_("--bisect-reset requires either zero or one 
> arguments"));
> - return bisect_reset(argc ? argv[0] : NULL);
> + res = bisect_reset(argc ? argv[0] : NULL);
> + break;
>   case CHECK_EXPECTED_REVS:
> - return check_expected_revs(argv, argc);
> + res = check_expected_revs(argv, argc);
> + break;
> + case BISECT_WRITE:
> + if (argc != 4 && argc != 5)
> + die(_("--bisect-write requires either 4 or 5 
> arguments"));
> + nolog = (argc == 5) && !strcmp(argv[4], "nolog");
> + strbuf_addstr(_good, argv[2]);
> + strbuf_addstr(_bad, argv[3]);

Here,

terms.term_good = argv[2];
terms.term_bad = argv[3];

and then you do not need bisect_terms_release() at all.

> + res = bisect_write(argv[0], argv[1], , nolog);
> + break;
>   default:
>   die("BUG: unknown subcommand '%d'", cmdmode);
>   }
> - return 0;
> + 

Re: [PATCH v14 11/27] bisect--helper: `bisect_next_check` & bisect_voc shell function in C

[Junio C Hamano]

Pranit Bauva  writes:

> +static int mark_good(const char *refname, const struct object_id *oid,
> +  int flag, void *cb_data)
> +{
> + int *m_good = (int *)cb_data;
> + *m_good = 0;
> + return 1;
> +}
> +
> +static char *bisect_voc(char *revision_type)
> +{
> + if (!strcmp(revision_type, "bad"))
> + return "bad|new";
> + if (!strcmp(revision_type, "good"))
> + return "good|old";
> +
> + return NULL;
> +}
> +
> +static int bisect_next_check(const struct bisect_terms *terms,
> +  const char *current_term)
> +{
> + int missing_good = 1, missing_bad = 1;
> + char *bad_ref = xstrfmt("refs/bisect/%s", terms->term_bad.buf);
> + char *good_glob = xstrfmt("%s-*", terms->term_good.buf);
> + char *bad_syn, *good_syn;
> +
> + if (ref_exists(bad_ref))
> + missing_bad = 0;
> + free(bad_ref);
> +
> + for_each_glob_ref_in(mark_good, good_glob, "refs/bisect/",
> +  (void *) _good);
> + free(good_glob);
> +
> + if (!missing_good && !missing_bad)
> + return 0;
> +
> + if (!current_term)
> + return -1;
> +
> + if (missing_good && !missing_bad && current_term &&
> + !strcmp(current_term, terms->term_good.buf)) {
> + char *yesno;
> + /*
> +  * have bad (or new) but not good (or old). We could bisect
> +  * although this is less optimum.
> +  */
> + fprintf(stderr, _("Warning: bisecting only with a %s commit\n"),
> + terms->term_bad.buf);
> + if (!isatty(0))
> + return 0;
> + /*
> +  * TRANSLATORS: Make sure to include [Y] and [n] in your
> +  * translation. The program will only accept English input
> +  * at this point.
> +  */
> + yesno = git_prompt(_("Are you sure [Y/n]? "), PROMPT_ECHO);
> + if (starts_with(yesno, "N") || starts_with(yesno, "n"))
> + return -1;
> +
> + return 0;
> + }
> + bad_syn = xstrdup(bisect_voc("bad"));
> + good_syn = xstrdup(bisect_voc("good"));
> + if (!is_empty_or_missing_file(git_path_bisect_start())) {
> + error(_("You need to give me at least one %s and "
> + "%s revision. You can use \"git bisect %s\" "
> + "and \"git bisect %s\" for that. \n"),
> + bad_syn, good_syn, bad_syn, good_syn);
> + free(bad_syn);
> + free(good_syn);
> + return -1;
> + }
> + else {
> + error(_("You need to start by \"git bisect start\". You "
> + "then need to give me at least one %s and %s "
> + "revision. You can use \"git bisect %s\" and "
> + "\"git bisect %s\" for that.\n"),
> + good_syn, bad_syn, bad_syn, good_syn);
> + free(bad_syn);
> + free(good_syn);
> + return -1;
> + }
> + free(bad_syn);
> + free(good_syn);
> +
> + return 0;

This one looks OK, but I think the same "Wouldn't it become cleaner
to have a 'finish:' label at the end and jump there?" comment
applies to this implementation, too.

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

Re: [PATCHv2] push: change submodule default to check

[Stefan Beller]

On Wed, Aug 24, 2016 at 12:37 PM, Junio C Hamano  wrote:
> Jeff King  writes:
>
> This seems to be dropped from the list, probably due to no "To:"
> header in the original, which led to "no", "To-header" "on" and
> "input <" on YOUR recipient list, so I am quoting it in full without
> trimming.
>
>> On Wed, Aug 24, 2016 at 10:30:17AM -0700, Stefan Beller wrote:
>>
>>> When working with submodules, it is easy to forget to push the submodules.
>>> The setting 'check', which checks if any existing submodule is present on
>>> at least one remote of the submodule remotes, is designed to prevent this
>>> mistake.
>>>
>>> Flipping the default to check for submodules is safer than the current
>>> default of ignoring submodules while pushing.
>>
>> It is safer, and that's good. But it's also slower, because it requires
>> an extra traversal of all of the pushed commits. And now people will
>> have to pay the price even if they are not using submodules at all.
>>
>> For instance, try this from a checkout of linux.git:
>>
>>   for i in no check; do
>>   rm -rf dst.git
>>   git init --bare dst.git
>>   echo "==> Pushing with submodules=$i"
>>   time git push --recurse-submodules=$i dst.git HEAD
>>   done
>>
>> The second case takes 30-40 seconds longer. This is a full push of
>> history, so it's an extreme case[1], but it's still rather unfortunate.
>>
>> Can we tie this default to some sign that submodules are actually in
>> use? I don't think the presence of .gitmodules is perfect (because you
>> might be in a bare repo, for example, and have just fetched some other
>> history you are relaying), but it may be a good compromise.  I'm
>> envisioning something like "--recurse-submodules=auto-check" which
>> auto-detects common situations (e.g., presence of .gitmodules or
>> .git/modules checkouts) and enables "check", and then setting the
>> default to that in the long run.
>>
>> -Peff
>>
>> [1] Actually, there is another much worse case lurking there. Try:
>>
>>   git push --recurse-submodules=check --mirror dst.git
>>
>> from the kernel. I didn't let it finish, but I'd estimate it would
>> take on the order of 5 hours. The problem is that push feeds each
>> updated ref tip to find_unpushed_submodules(), so we end up walking
>> over the same history over and over.
>>
>> I think it should feed all of the "before" and "after" ref tips it
>> proposes to update to a _single_ revision traversal.
>
> That sounds massively ... broken.  So before even thinking about
> flipping it to default, this needs to be fixed first.
>

I agree. That sounds bad.

However having the --auto-check feels like papering over the
actual problem which to me sounds like a design problem.
However this may be a viable short term solution.

About the design issue:
We need to answer the question: Which submodule commits
are referenced by a given set of superproject commits.

This question is advancing a very similar question that we'd
have to ask in git-gc. In gc we would end up without having to
worry about a specific set, but rather the all reachable commits
of the superproject are in the given set.

So we could solve two issues at the same time if we had a quick
way to answer this question quickly.

And for that I considered introducing a ref in the submodule
(e.g.) refs/superproject/gc_protector, which records both the
superprojects commit as well as the submodule commit
in case the superproject changed the submodule pointer.

I have some rough patches for this, but I did not consider sending
that to the list as the patches are still rough and not quite
fully thought out on the design level.

--
Actually screw this. After an office discussion with Jonathan (cc'd):

1)  We need to have an "alternate refs namespace", which contains
secondary data (as this can be derived from the primary data with
lots of compute). So maybe we need a file similar to the packed refs
file, "superproject-refs" that behaves as if it is storing refs,
but that file
is safe to delete as we know all its contents can be recreated.

2) In this new alternate refs space, we can have
refs/superproject/ in the submodule with the sha1 of the
superproject that points to a commit which is a dirty merge of all
submodule commits, that have no other parents.

e.g.
In the superproject we might have a history of:

  A <- B <- C

with A being origin/master and C our local master.

In the submodule we have:

  D <- E <- F
\
  G

F is the master branch in the submodule, G is a dangling commit.

Now we could have the following git links:

A -> D
B -> G
C -> F

Then the refs/superproject/
would be a merge of F and G.

When pushing in the superproject (A..C) we would need to make sure
the submodule is updated as well, i.e. we'd look at
refs/superproject/ to know we need to advance the remote
submodule history to contain at least G and F.

Then we can do a standard rev 

Re: [PATCH v14 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C

[Junio C Hamano]

Pranit Bauva  writes:

> +static int is_expected_rev(const char *expected_hex)
> +{
> + struct strbuf actual_hex = STRBUF_INIT;
> + int res = 0;
> + if (strbuf_read_file(_hex, git_path_bisect_expected_rev(), 0) >= 
> 0) {
> + strbuf_trim(_hex);
> + res = !strcmp(actual_hex.buf, expected_hex);

If it is known to have 40-hex:

 (1) accepting ">= 0" seems way too lenient.  You only expect a
 41-byte file (or 42 if somebody would write CRLF, but I do not
 think anybody other than yourself is expected to write into
 this file, and you do not write CRLF yourself);

 (2) strbuf_trim() is overly loose.  You only want to trim the
 terimnating LF and it is an error to have other trailing
 whitespaces.

I think the latter is not a new problem and it is OK to leave it
as-is; limiting (1) to >= 40 may still be a good change, though,
because it makes the intention of the code clearer.

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

Re: What's cooking in git.git (Aug 2016, #08; Wed, 24)

[Junio C Hamano]

Jeff King  writes:

> On Wed, Aug 24, 2016 at 02:16:02PM -0700, Junio C Hamano wrote:
>
>> * jk/pack-objects-optim-mru (2016-08-11) 4 commits
>>   (merged to 'next' on 2016-08-11 at c0a7dae)
>>  + pack-objects: use mru list when iterating over packs
>>  + pack-objects: break delta cycles before delta-search phase
>>  + sha1_file: make packed_object_info public
>>  + provide an initializer for "struct object_info"
>> 
>>  "git pack-objects" in a repository with many packfiles used to
>>  spend a lot of time looking for/at objects in them; the accesses to
>>  the packfiles are now optimized by checking the most-recently-used
>>  packfile first.
>> 
>>  Will hold to see if people scream.
>
> Just a note that we've been running with this at GitHub on all of our
> servers for a bit over a week, and no screaming yet. That's not
> necessarily proof of anything, but it does make the audience of "people"
> a bit bigger than just "next", as we run quite a few invocations of
> pack-objects in a day.
>
> I don't think that changes anything in the near future, since this is
> obviously not for v2.10, but barring any complaints, it's probably a
> reasonable topic to consider for the version after (and of course I'll
> relay any issues we come across on our servers).

Yeah, that "Will hold" is primarily me being lazy [*1*] during the
-rc period when updating the "What's cooking".  After following the
"break delta cycles" patch carefully, I am no longer worried about
this topic.  It should be in 'master' early in the next cycle, among
other topics that are competently done.

> I'm planning to deploy the delta-cache topic soon, too, so that should
> give it some good exercise.

Good.  Thanks.


[Footnote]

*1* ... and a bit more careful, as any "Will merge to 'next'" thing
gets marked as "Will merge to 'master'" by default and having any
entry under "Will merge to 'master'" in "Meta/cook -w" report tempts
me to merge them even during -rc period.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: diff using 3-dot behavior

[Jeff King]

On Wed, Aug 24, 2016 at 01:48:30PM -0700, Jacob Keller wrote:

> On Wed, Aug 24, 2016 at 9:22 AM, Junio C Hamano  wrote:
> > The merge-base is to compute the point you forked your topic from
> > the mainline.  So if you want to compare your current state in the
> > index with the fork point, you'd do
> >
> > git diff --cached $(git merge-base master topic)
> >
> > and comparing your working tree state would be
> >
> > git diff --cached $(git merge-base master topic)
> >
> 
> Those are both identical?

I imagine the second one just meant to drop "--cached".

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

Re: on Amazon EFS (NFS): "Reference directory conflict: refs/heads/" with status code 128

[Jeff King]

On Wed, Aug 24, 2016 at 04:52:33PM -0400, Alex Nauda wrote:

> Elastic File System (EFS) is Amazon's scalable filesystem product that
> is exposed to the OS as an NFS mount. We're using EFS to host the
> filesystem used by a Jenkins CI server. Sometimes when Jenkins tries
> to git fetch, we get this error:
> $ git -c core.askpass=true fetch --tags --progress
> g...@github.com:mediasilo/dodo.git
> +refs/pull/*:refs/remotes/origin/pr/*
> fatal: Reference directory conflict: refs/heads/
> $ echo $? 128
> 
> Has anyone seen anything like this before? Any tips on how to troubleshoot it?

No, I haven't seen it before. That's an internal assertion in the refs
code that shouldn't ever happen. It looks like it happens when the loose
refs end up with duplicate directory entries. While a bug in git is an
obvious culprit, I wonder if it's possible that your filesystem might
expose the same name twice in one set of readdir() results.

+cc Michael, who added this assertion long ago (and since this is the
first report in all these years, it does make me suspect that the
filesystem is a critical part of reproducing).

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

Re: What's cooking in git.git (Aug 2016, #08; Wed, 24)

[Jeff King]

On Wed, Aug 24, 2016 at 02:16:02PM -0700, Junio C Hamano wrote:

> * jk/pack-objects-optim-mru (2016-08-11) 4 commits
>   (merged to 'next' on 2016-08-11 at c0a7dae)
>  + pack-objects: use mru list when iterating over packs
>  + pack-objects: break delta cycles before delta-search phase
>  + sha1_file: make packed_object_info public
>  + provide an initializer for "struct object_info"
> 
>  "git pack-objects" in a repository with many packfiles used to
>  spend a lot of time looking for/at objects in them; the accesses to
>  the packfiles are now optimized by checking the most-recently-used
>  packfile first.
> 
>  Will hold to see if people scream.

Just a note that we've been running with this at GitHub on all of our
servers for a bit over a week, and no screaming yet. That's not
necessarily proof of anything, but it does make the audience of "people"
a bit bigger than just "next", as we run quite a few invocations of
pack-objects in a day.

I don't think that changes anything in the near future, since this is
obviously not for v2.10, but barring any complaints, it's probably a
reasonable topic to consider for the version after (and of course I'll
relay any issues we come across on our servers).

I'm planning to deploy the delta-cache topic soon, too, so that should
give it some good exercise.

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

Re: [PATCHv2] push: change submodule default to check

[Junio C Hamano]

Junio C Hamano  writes:

> Jeff King  writes:
>
>> For instance, try this from a checkout of linux.git:
>>
>>   for i in no check; do
>>  rm -rf dst.git
>>  git init --bare dst.git
>>  echo "==> Pushing with submodules=$i"
>>  time git push --recurse-submodules=$i dst.git HEAD
>>   done
>>
>> The second case takes 30-40 seconds longer. This is a full push of
>> history, so it's an extreme case[1], but it's still rather unfortunate.

This actually bit me just now.  github.com/gitster/git.git is
mirror pushed, and it would seem to take forever, so I killed it,
and flipped the configuration variable off.  Which means the feature
won't ever get any testing from me in real life.

People, git.git is not a large project in any sense of the word.

Why wasn't it discovered that "push.recursesubmodules = check" is
UNUSABLE since it was introduced is simply beyond me.

I am mad (which is unusual for me, isn't it? -- I've seen somebody
else saying "I am mad", but I do not think I ever said it here).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] Speed up sparse checkout when $GIT_DIR/info/sparse-checkout is unchanged

[Ben Peart]

I didn't apply the patch and test it out but from just a code review, the logic 
behind and the design of this patch makes sense and it's a relatively small 
patch for the gain.  It's also a great example of the minimal amount of work 
required to add a new extension into the index.  Thank you for that.

For more extreme cases where there are large numbers of entries in the 
sparse-checkout file, another approach may be more helpful.  Our virtualization 
solution uses sparse checkout extensively.  The sparse-checkout file starts out 
empty and as files are hydrated locally, they are added to the sparse-checkout 
file.  This results in thousands of entries in the sparse-checkout file.   

To make that fast enough, we ended up creating a hashmap in 
mark_new_skip_worktree and then in clear_ce_flags_1 we use that hashmap to 
implement the pattern matching logic.  The hashmap lookup is dramatically 
faster than the current recursive and complex pattern matching via the excludes 
mechanism so easily supports very large numbers of entries.  

Note, these are extreme test cases that caused the unmodified git.exe to crash 
so I can't really give before/after comparisons:

With 35,000 entries in the sparse-checkout file, "git checkout -b xxx" took 
42.5 seconds
With 100,000 entries in the sparse-checkout file, "git checkout -b xxx" took 
42.5 seconds
With 3,279,254 entries in the sparse-checkout file, "git checkout -b xxx" took 
1 min 38 seconds

Note the first two numbers are the same because the hashmap based pattern 
matching is so fast, 95% of the time is now spent in merge_working_tree.  Since 
the tip commit doesn’t change between the new and old branch and because a 
merge is done, the files in the working directory don’t change either so I 
believe there must be an optimization there but I haven't been able to figure 
one out that doesn't break a lot of other things.  

Ben

p.s.  Yes, I actually ran "git ls-files > .git/info/sparse-checkout" to test 
the extreme case. :)

-Original Message-
From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of 
Duy Nguyen
Sent: Saturday, August 13, 2016 4:37 AM
To: Git Mailing List ; Junio C Hamano 
Cc: Nguyễn Thái Ngọc Duy 
Subject: Re: [PATCH] Speed up sparse checkout when 
$GIT_DIR/info/sparse-checkout is unchanged

Ping..

On Tue, Jul 12, 2016 at 1:15 AM, Nguyễn Thái Ngọc Duy  wrote:
> When a "tree unpacking" operation is needed, which is part of 
> switching branches using "git checkout", the following happens in a 
> sparse checkout:
>
> 1) Run all existing entries through $GIT_DIR/info/sparse-checkout,
>mark entries that are to-be-excluded or to-be-included.
>
> 2) Do n-way merge stuff to add, modify and delete entries.
>
> 3) Run all new entries added at step 2 through
>$GIT_DIR/info/sparse-checkout, mark entries that are to-be-excluded
>or to-be-included.
>
> 4) Compare the current excluded/include status with the to-be-status
>from steps 1 and 3, delete newly excluded entries from worktree and
>add newly included ones to worktree.
>
> The "all existing entries" number in step 1 does not scale well. As 
> worktree gets bigger (or more sparse patterns are added), step 1 runs 
> slower. Which does not help because large worktrees are the reason 
> sparse-checkout is used, to keep the real worktree small again.
>
> If we know that $GIT_DIR/info/sparse-checkout has not changed, we know 
> that running checking again would result in the exact same 
> included/excluded as recorded in the current index because 
> "sparse-checkout" is the only input to the exclude machinery. In this 
> case, marking the to-be-status is simply copying the current status 
> over, which is a lot faster.
>
> The time breakdown of "git checkout" (no arguments) on webkit.git 
> (100k files) with a sparse checkout file of 4 negative patterns is 
> like this, where "sparse checkout loop #1" takes about 10% execution 
> time, which is the time saved after this patch.
>
> read-cache.c:1661   performance: 0.057816104 s: read cache .git/index
> files-backend.c:1097performance: 0.23980 s: read packed refs
> preload-index.c:104 performance: 0.039178367 s: preload index
> read-cache.c:1260   performance: 0.002700730 s: refresh index
> name-hash.c:128 performance: 0.030409968 s: initialize name hash
>
> unpack-trees.c:1173 performance: 0.100353572 s: sparse checkout loop #1
>
> cache-tree.c:431performance: 0.137213472 s: cache_tree_update
> unpack-trees.c:1305 performance: 0.648923590 s: unpack_trees
> read-cache.c:2139   performance: 0.074800165 s: write index, changed mask 
> = 28
> unpack-trees.c:1305 performance: 0.137108835 s: unpack_trees
> diff-lib.c:506  performance: 0.137152238 s: diff-index
> trace.c:420 performance: 0.972682413 s: git command: 'git' 
> 'checkout'
>
> Signed-off-by: Nguyễn 

What's cooking in git.git (Aug 2016, #08; Wed, 24)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

v2.10.0-rc1 was tagged last week, then I got sick and lost an entire
day.  I've merged a handful more topics to 'next' and marked 5 among
them to be merged to 'master' by -rc2.  A notable is the ja/i18n
topic, which hopefully will reduce the translator's load somewhat.
Also the change to arrange the file-descriptors to the tempfile be
closed for subprocesses to avoid deadlocks on Windows is also in;
even though it is not a new problem, the fix looked safe enough.
Linus's "32-bit GPG key-ids are so last century" change is already
in, as reported in the 2.10.0-rc1 announcement.

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

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

--
[Graduated to "master"]

* ab/hooks (2016-08-16) 1 commit
  (merged to 'next' on 2016-08-17 at b56e55d)
 + rev-parse: respect core.hooksPath in --git-path

 "git rev-parse --git-path hooks/" learned to take
 core.hooksPath configuration variable (introduced during 2.9 cycle)
 into account.


* jk/difftool-command-not-found (2016-08-15) 1 commit
  (merged to 'next' on 2016-08-17 at 32baf03)
 + difftool: always honor fatal error exit codes

 "git difftool" by default ignores the error exit from the backend
 commands it spawns, because often they signal that they found
 differences by exiting with a non-zero status code just like "diff"
 does; the exit status codes 126 and above however are special in
 that they are used to signal that the command is not executable,
 does not exist, or killed by a signal.  "git difftool" has been
 taught to notice these exit status codes.


* lt/gpg-show-long-key-in-signature-verification (2016-08-16) 1 commit
  (merged to 'next' on 2016-08-17 at 1ee8a00)
 + Merge branch 'lt/gpg-show-long-key-in-signature-verification-maint' into 
lt/gpg-show-long-key-in-signature-verification
 (this branch uses lt/gpg-show-long-key-in-signature-verification-maint.)

 "git log --show-signature" and other commands that display the
 verification status of PGP signature now shows the longer key-id,
 as 32-bit key-id is so last century.

 A last-minute merge of this topic was a bit scary but I made sure
 push-certificate codepath would not be negatively affected.


* lt/gpg-show-long-key-in-signature-verification-maint (2016-08-16) 1 commit
 + gpg-interface: prefer "long" key format output when verifying pgp signatures
 (this branch is used by lt/gpg-show-long-key-in-signature-verification.)

 "git log --show-signature" and other commands that display the
 verification status of PGP signature now shows the longer key-id,
 as 32-bit key-id is so last century.  This is based on older
 codebase, just in case somebody wants to have it.


* rs/pull-signed-tag (2016-08-13) 4 commits
  (merged to 'next' on 2016-08-17 at cecca71)
 + commit: use FLEX_ARRAY in struct merge_remote_desc
 + merge-recursive: fix verbose output for multiple base trees
 + commit: factor out set_merge_remote_desc()
 + commit: use xstrdup() in get_merge_parent()

 When "git merge-recursive" works on history with many criss-cross
 merges in "verbose" mode, the names the command assigns to the
 virtual merge bases could have overwritten each other by unintended
 reuse of the same piece of memory.


* sb/checkout-explit-detach-no-advice (2016-08-15) 1 commit
  (merged to 'next' on 2016-08-17 at fb64716)
 + checkout: do not mention detach advice for explicit --detach option

 "git checkout --detach " used to give the same advice
 message as that is issued when "git checkout " (or anything
 that is not a branch name) is given, but asking with "--detach" is
 an explicit enough sign that the user knows what is going on.  The
 advice message has been squelched in this case.


* tb/t0027-raciness-fix (2016-08-14) 1 commit
  (merged to 'next' on 2016-08-17 at 39a6635)
 + convert: Correct NNO tests and missing `LF will be replaced by CRLF`

 The t0027 test for CRLF conversion was timing dependent and flaky.

--
[New Topics]

* cc/receive-pack-limit (2016-08-24) 3 commits
 - receive-pack: allow a maximum input size to be specified
 - unpack-objects: add --max-input-size= option
 - index-pack: add --max-input-size= option

 An incoming "git push" that attempts to push too many bytes can now
 be rejected by setting a new configuration variable at the receiving
 end.

 Will merge to 'next'.


* hv/doc-commit-reference-style (2016-08-17) 1 commit
  (merged to 'next' on 2016-08-24 at b187d45)
 + SubmittingPatches: document how to reference previous commits

 A small doc update.

 Will merge to 'master'.


* js/no-html-bypass-on-windows (2016-08-19) 1 

Re: [PATCH v14 07/27] bisect--helper: `bisect_reset` shell function in C

[Junio C Hamano]

Pranit Bauva  writes:

> +static int bisect_reset(const char *commit)
> +{
> + struct strbuf branch = STRBUF_INIT;
> +
> + if (!commit) {
> + if (strbuf_read_file(, git_path_bisect_start(), 0) < 1) {

Hmm, tricky but correct to do the "< 1" comparison.  If the file
does not exist, you'd get -1; if it fails to read it, you'd get -1;
if it turns out to be empty, you'd get 0.

> + printf("We are not bisecting.\n");
> + return 0;
> + }
> + strbuf_rtrim();
> + } else {
> + struct object_id oid;
> + if (get_oid(commit, ))
> + return error(_("'%s' is not a valid commit"), commit);

The original is

rev-parse --quiet --verify "$1^{commit}"

Unless the caller of this function already appended "^{commit}" to
whatever the user gave "bisect--helper bisect-reset", this
conversion is not equivalent.  If you said

git bisect reset HEAD:

get_oid() would tell you that the top-level tree object of the
current commit exists in the object store, but the original is
meant to catch "That's not a commit, it's a tree!" before attempting
to run "git checkout" on it.

I think get_sha1_committish() is what you want to use here.

> + strbuf_addstr(, commit);
> + }

Also this version fails to catch "bisect reset a b c" as an error, I
suspect.

> @@ -627,7 +603,7 @@ case "$#" in
>   visualize|view)
>   bisect_visualize "$@" ;;
>   reset)
> - bisect_reset "$@" ;;
> + git bisect--helper --bisect-reset "$@" ;;
>   replay)
>   bisect_replay "$@" ;;
>   log)
>
> --
> https://github.com/git/git/pull/287
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

on Amazon EFS (NFS): "Reference directory conflict: refs/heads/" with status code 128

[Alex Nauda]

Elastic File System (EFS) is Amazon's scalable filesystem product that
is exposed to the OS as an NFS mount. We're using EFS to host the
filesystem used by a Jenkins CI server. Sometimes when Jenkins tries
to git fetch, we get this error:
$ git -c core.askpass=true fetch --tags --progress
g...@github.com:mediasilo/dodo.git
+refs/pull/*:refs/remotes/origin/pr/*
fatal: Reference directory conflict: refs/heads/
$ echo $? 128

Has anyone seen anything like this before? Any tips on how to troubleshoot it?

Related Jenkins issue: https://issues.jenkins-ci.org/browse/JENKINS-37653
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v14 04/27] bisect--helper: `bisect_clean_state` shell function in C

[Junio C Hamano]

Pranit Bauva  writes:

> Reimplement `bisect_clean_state` shell function in C and add a
> `bisect-clean-state` subcommand to `git bisect--helper` to call it from
> git-bisect.sh .
>
> Using `--bisect-clean-state` subcommand is a measure to port shell
> function to C so as to use the existing test suite. As more functions
> are ported, this subcommand will be retired but its implementation  will
> be called by bisect_reset() and bisect_start().
>
> Mentored-by: Lars Schneider 
> Mentored-by: Christian Couder 
> Signed-off-by: Pranit Bauva 
> ---

This seems to be where this round diverges from the previous one.
This patch in this round has more stuff that used to be in
builtin/bisect--helper.c in the previous one in bisect.c.  Because I
am not sure if the distinction would make that much of a difference
(after all, I do not think of a good reason why many bisect internals
need to be exposed to anything other than the eventual builtin/bisect.c
that retires git-bisect.sh), I am OK with the change to this patch
between the previous round and this round.

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

Re: diff using 3-dot behavior

[Jacob Keller]

On Wed, Aug 24, 2016 at 9:22 AM, Junio C Hamano  wrote:
> The merge-base is to compute the point you forked your topic from
> the mainline.  So if you want to compare your current state in the
> index with the fork point, you'd do
>
> git diff --cached $(git merge-base master topic)
>
> and comparing your working tree state would be
>
> git diff --cached $(git merge-base master topic)
>

Those are both identical?

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

Re: Extending "extended SHA1" syntax to traverse through gitlinks?

[Josh Triplett]

On Wed, Aug 24, 2016 at 07:05:17PM +0200, Jakub Narębski wrote:
> W dniu 24.08.2016 o 16:20, Josh Triplett pisze:
> > On Wed, Aug 24, 2016 at 03:16:56PM +0200, Jakub Narębski wrote:
> [...]
> >> Not really.
> >>
> >> The above means only that the support for new syntax would be not
> >> as easy as adding it to 'git rev-parse' (and it's built-in equivalent),
> >> except for the case where submodule uses the same object database as
> >> supermodule.
> >>
> >> So it wouldn't be as easy (on conceptual level) as adding support
> >> for ':/' or '^{/}'.  It would be at least as
> >> hard, if not harder, as adding support for '@{-1}' and its '-'
> >> shortcut.
> > 
> > Depends on which cases you want to handle.  In the most general case,
> > you'd need to find and process the applicable .gitmodules file, which
> > would only work if you started from the top-level tree, not a random
> > treeish.  On the other hand, in the most general case, you don't
> > necessarily even have the module you need, because .git/modules only
> > contains the modules the *current* version needed, not every past
> > version.
> 
> There is an additional problem, namely that directory with submodule
> can be renamed.
> 
> I don't know if there is an existing API, but assuming modern
> git-submodule (with repository in .git/modules) you would have to
> do the following steps for ://:
> 
>  * look up :.gitmodules for module which 'path'
>is ; let's say it is named 
>  * check if : commit object
>is present in .git/modules/
>  * look up this object

This also assumes your lookup started with a  and not an
intermediate , but that'll work in many cases.

> > As an alternate approach (pun intended): treat every module in
> > .git/modules as an alternate and just look up the object by hash.  
> 
> This could be a good fallback, to search through all submodules.
> 
> > Or, teach git-submodule to store all the objects for submodules in the
> > supermodule's .git/objects (and teach git's reachability algorithm to
> > respect refs in .git/modules, or store their refs in
> > .git/refs/submodules/ or in a namespace).
> 
> And fallback to this fallback could be searching through supermodule
> object repository.

I'd flip those around: first search registered .gitmodules, then look up
the object in the superproject (since you have it at hand), and then
maybe search every submodule.

> >> Josh, what was the reason behind proposing this feature? Was it
> >> conceived as adding completeness to gitrevisions syntax, a low-hanging
> >> fruit?  It isn't (the latter).  Or was it some problem with submodule
> >> handling that you would want to use this syntax for?
> > 
> > This wasn't an abstract/theoretical completeness issue.  I specifically
> > wanted this syntax for practical use with actual trees containing
> > gitlinks, motivated by having a tool that creates and uses such
> > gitlinks. :)
> 
> Could you explain what you need in more detail?  Is it a fragment
> of history of submodule, a contents of a file at given point of
> superproject history, diff between file-in-submodule and something
> else, or what?

As part of git-series, I have commits, whose trees contain various
gitlinks, such as "series" and "base".  Those gitlinks point to commits
in the same repository.  I'd like to use those gitlinks everywhere I
could use any other committish, such as a branch name.  In particular,
I'd like to write things like some_feature:series:path/to/file ("what
does path/to/file look like in the current version of some_feature"),
some_feature:series^ ("what's the second-to-last commit in
some_feature"), some_feature~5:series:path/to/file ("what did
path/to/file look like in an older version of some_feature"), or
some_feature~5:base..some_feature~5:series~2 ("all but the last two
patches in some_feature~5").  Those should work with show, diff,
format-patch, log, etc.

> >> As for usefulness: this fills the hole in accessing submodules, one
> >> that could be handled by combining plumbing-level commands.  Namely,
> >> there are 5 states of submodule (as I understand it)
> >>
> >>  * recorded in ref / commit in supermodule
> >>  * recorded in the index in supermodule
> >>  - recorded in ref / commit in submodule
> >>  - recorded in the index in submodule
> >>  - state of worktree in submodule
> >>
> >> The last three can be easyly acessed by cd-ing to submodule.  The first
> >> two are not easy to get, AFAIUC.
> > 
> > Right.  I primarily care about those first two cases, especially the
> > first one: given a commit containing a gitlink, how can I easily dig
> > into the linked commit?
> 
> All right.
> 
> Though you can cobble it with plumbing... just saying.

Sure, but that makes the expression much more complex.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Eric Wong]

Jeff King  wrote:
> On Wed, Aug 24, 2016 at 06:49:38PM +, Eric Wong wrote:
> > > > Given that public-inbox provides an NNTP interface, couldn't the ARTICLE
> > > >  NNTP command be used to easily retrieve the messages in a
> > > > given patch series (at least compared to POP or IMAP).  Perhaps
> > > > git-send-email could be modified to include the message-id value of each
> > > > patch in the series that it sends to the mailing list and include it in
> > > > the cover letter.
> > 
> > I think that makes sense; perhaps an X-Git-Followups: header
> > from send-email which lists the child Message-IDs the same way
> > References: does for ancestors.  (perhaps there's already a
> > standardized header for listing children)
> 
> I think that's harder to adapt to some workflows, since it implies
> generating all of the message-ids ahead of time (whereas if you are
> feeding the messages into an existing MUA, it may generate them on the
> fly as it sends).

Yeah, it would be limited to git send-email users, only :<

> > I thought about allowing a giant MIME message with all the
> > patches attached, too but that won't work for a large patch
> > series due to size limits along various SMTP hops.
> > Compression might make spam filters unhappy, too.
> 
> This was a problem faced by binary groups on Usenet, which had to split
> large files across many messages.
> 
> It has been a long time since I've dealt with those, but I think the
> state of the art involved using "1/20", "2/20", etc in the subjects to
> piece together the original. There may also have been header or body
> content that included a unique id, so you always knew which messages
> were part of a set.
> 
> They also used things like forward error correction to handle dropped
> messages, but I don't think we need to go that far.
> 
> So parsing the "PATCH 1/20" headers sounds hacky, but I think it has
> worked for years in other communities.

nzb (an XML format) seems to be the thing for Usenet binaries,
nowadays.  Maybe it's workable for git, maybe it's overkill or
not worth it for the two (non-.onion) NNTP servers we have.

nzb seems widely supported enough (on a Debian jessie system):

$ apt-cache search nzb
sabnzbdplus - web-based binary newsgrabber with nzb support
sabnzbdplus-theme-classic - classic interface templates for the SABnzbd+ binary 
newsgrabber
sabnzbdplus-theme-iphone - transitional package for migration to 
sabnzbdplus-theme-mobile
sabnzbdplus-theme-mobile - mobile interface templates for the SABnzbd+ binary 
newsgrabber
sabnzbdplus-theme-plush - plush interface templates for the SABnzbd+ binary 
newsgrabber
sabnzbdplus-theme-smpl - smpl interface templates for the SABnzbd+ binary 
newsgrabber
libnzb-dev - An nzb based Usenet binary grabber (development files)
libnzb0c2a - An nzb based Usenet binary grabber (runtime library)
lottanzb - simple and automated Usenet downloader for Newzbin (NZB) files
nzb - Usenet binary grabber
nzbget - command-line based binary newsgrabber for nzb files
python-pynzb - unified API for parsing NZB files from NNTP (Usenet) servers
spotweb - web interface to search and filter Usenet spots
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] push: change submodule default to check

[Junio C Hamano]

Jeff King  writes:

This seems to be dropped from the list, probably due to no "To:"
header in the original, which led to "no", "To-header" "on" and
"input <" on YOUR recipient list, so I am quoting it in full without
trimming.

> On Wed, Aug 24, 2016 at 10:30:17AM -0700, Stefan Beller wrote:
>
>> When working with submodules, it is easy to forget to push the submodules.
>> The setting 'check', which checks if any existing submodule is present on
>> at least one remote of the submodule remotes, is designed to prevent this
>> mistake.
>> 
>> Flipping the default to check for submodules is safer than the current
>> default of ignoring submodules while pushing.
>
> It is safer, and that's good. But it's also slower, because it requires
> an extra traversal of all of the pushed commits. And now people will
> have to pay the price even if they are not using submodules at all.
>
> For instance, try this from a checkout of linux.git:
>
>   for i in no check; do
>   rm -rf dst.git
>   git init --bare dst.git
>   echo "==> Pushing with submodules=$i"
>   time git push --recurse-submodules=$i dst.git HEAD
>   done
>
> The second case takes 30-40 seconds longer. This is a full push of
> history, so it's an extreme case[1], but it's still rather unfortunate.
>
> Can we tie this default to some sign that submodules are actually in
> use? I don't think the presence of .gitmodules is perfect (because you
> might be in a bare repo, for example, and have just fetched some other
> history you are relaying), but it may be a good compromise.  I'm
> envisioning something like "--recurse-submodules=auto-check" which
> auto-detects common situations (e.g., presence of .gitmodules or
> .git/modules checkouts) and enables "check", and then setting the
> default to that in the long run.
>
> -Peff
>
> [1] Actually, there is another much worse case lurking there. Try:
>
>   git push --recurse-submodules=check --mirror dst.git
>
> from the kernel. I didn't let it finish, but I'd estimate it would
> take on the order of 5 hours. The problem is that push feeds each
> updated ref tip to find_unpushed_submodules(), so we end up walking
> over the same history over and over.
>
> I think it should feed all of the "before" and "after" ref tips it
> proposes to update to a _single_ revision traversal.

That sounds massively ... broken.  So before even thinking about
flipping it to default, this needs to be fixed first.

> I also notice that it uses "--remote=...", which is weird, because
> push knows exactly what it proposes to update, which may be ahead of
> where our refs/remotes/* cache is. Not to mention that we may be
> pushing to a remote for which we do not keep tracking refs at all!
>
> So I'd actually suspect that with your patch, a bare URL like:
>
>   git push https://github.com/peff/linux.git
>
> would do the full 40-second walk, even if I was only pushing up one
> or two objects.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 0/3] limit the size of the packs we receive

[Junio C Hamano]

Christian Couder  writes:

> Diff with previous v2 version
> ~
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index f5b6061..8a115b3 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2517,10 +2517,11 @@ receive.unpackLimit::
>   especially on slow filesystems.  If not set, the value of
>   `transfer.unpackLimit` is used instead.
>  
> -receive.maxsize::
> - If the size of a pack file is larger than this limit, then
> - git-receive-pack will error out, instead of accepting the pack
> - file. If not set or set to 0, then the size is unlimited.
> +receive.maxInputSize::
> + If the size of the incoming pack stream is larger than this
> + limit, then git-receive-pack will error out, instead of
> + accepting the pack file. If not set or set to 0, then the size
> + is unlimited.
>  
>  receive.denyDeletes::
>   If set to true, git-receive-pack will deny a ref update that deletes
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> ...
> diff --git a/t/t5546-receive-limits.sh b/t/t5546-receive-limits.sh
> new file mode 100755
> index 000..10cb0be
> --- /dev/null
> +++ b/t/t5546-receive-limits.sh
> @@ -0,0 +1,55 @@
> +#!/bin/sh
> + ...
> +test_done
> ---
>
> Christian Couder (1):
>   unpack-objects: add --max-input-size= option
>
> Jeff King (2):
>   index-pack: add --max-input-size= option
>   receive-pack: allow a maximum input size to be specified

This was a pleasant read.  All looked sensible.

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

Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Eric Wong]

Johannes Schindelin  wrote:
> On Mon, 22 Aug 2016, Philip Oakley wrote:
> > I do note that dscho's patches now have the extra footer (below the three
> > dashes) e.g.
> > 
> > Published-As: https://github.com/dscho/git/releases/tag/cat-file-filters-v1
> > Fetch-It-Via: git fetch https://github.com/dscho/git cat-file-filters-v1
> > 
> > If say I used that, and sent my patch series via Outlook Express (),
> > with it's white space damage, would those footers help once the content has
> > been reviewed (rather than white spacing style) in the applying the patch?
> 
> I considered recommending this as some way to improve the review process.
> The problem, of course, is that it is very easy to craft an email with an
> innocuous patch and then push some malicious patch to the linked
> repository.

Perhaps an automated checker of some sort packaged with git
would help.
(And perhaps combinable with the downloader Arif proposed)

> Now, with somebody like me who would lose a lot when destroying trust, it
> is highly unlikely. But it is possible that in between the hundreds of
> sincere contributors a bad apple tries to sneak in bad stuff.

Yes, I would never mix reviews + patch applications of emails vs
git-fetched data.  Having a sender providing both is good; but
the recipient needs to pick one or the other to use exclusively
for that series.

Either look exclusively at what is fetched and respond to that;
or look exclusively at emails and ignore data from git fetch.

However, ensuring the emails and the contents of the git fetch
could be done optionally to ensure there's no tampering or
accidents for other reviewers.

> Therefore, if we were to support a Git-driven contribution process that
> *also* sends mail, that mail needs to be generated by a trusted source, to
> ensure that the content of the mail is identical to the original Git
> commits.

For decentralized systems, independent reproducibilility is
needed.  Rather than trusting one source, I'd rather have some
sort of downloading + checking tool which checks multiple
mirrors (git protocols and NNTP).  That would allow users to
independently verify the veracity of what they got emailed vs
what is fetched.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Jeff King]

On Wed, Aug 24, 2016 at 06:49:38PM +, Eric Wong wrote:

> > > Given that public-inbox provides an NNTP interface, couldn't the ARTICLE
> > >  NNTP command be used to easily retrieve the messages in a
> > > given patch series (at least compared to POP or IMAP).  Perhaps
> > > git-send-email could be modified to include the message-id value of each
> > > patch in the series that it sends to the mailing list and include it in
> > > the cover letter.
> 
> I think that makes sense; perhaps an X-Git-Followups: header
> from send-email which lists the child Message-IDs the same way
> References: does for ancestors.  (perhaps there's already a
> standardized header for listing children)

I think that's harder to adapt to some workflows, since it implies
generating all of the message-ids ahead of time (whereas if you are
feeding the messages into an existing MUA, it may generate them on the
fly as it sends).

> I thought about allowing a giant MIME message with all the
> patches attached, too but that won't work for a large patch
> series due to size limits along various SMTP hops.
> Compression might make spam filters unhappy, too.

This was a problem faced by binary groups on Usenet, which had to split
large files across many messages.

It has been a long time since I've dealt with those, but I think the
state of the art involved using "1/20", "2/20", etc in the subjects to
piece together the original. There may also have been header or body
content that included a unique id, so you always knew which messages
were part of a set.

They also used things like forward error correction to handle dropped
messages, but I don't think we need to go that far.

So parsing the "PATCH 1/20" headers sounds hacky, but I think it has
worked for years in other communities.

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

Re: [PATCH v3 3/3] receive-pack: allow a maximum input size to be specified

[Jeff King]

On Wed, Aug 24, 2016 at 08:41:57PM +0200, Christian Couder wrote:

> +test_pack_input_limit () {
> + case "$1" in
> + index) unpack_limit=1 ;;
> + unpack) unpack_limit=1 ;;
> + esac

Nice, this is pretty self-explanatory.

> + test_expect_success 'prepare destination repository' '
> + rm -fr dest &&
> + git --bare init dest
> + '
> +
> + test_expect_success "set unpacklimit to $unpack_limit" '
> + git --git-dir=dest config receive.unpacklimit "$unpack_limit"
> + '
> +
> + test_expect_success 'setting receive.maxInputSize to 512 rejects push' '
> + git --git-dir=dest config receive.maxInputSize 512 &&
> + test_must_fail git push dest HEAD
> + '
> +
> + test_expect_success 'bumping limit to 4k allows push' '
> + git --git-dir=dest config receive.maxInputSize 4k &&
> + git push dest HEAD
> + '

Makes sense. We couldn't push, and then we could.

> + test_expect_success 'prepare destination repository (again)' '
> + rm -fr dest &&
> + git --bare init dest
> + '
> +
> + test_expect_success 'lifting the limit allows push' '
> + git --git-dir=dest config receive.maxInputSize 0 &&
> + git push dest HEAD
> + '

This is new in this iteration, I think. At first I thought "but every
_other_ test script is implicitly testing that we work without the
limit". But this is showing that setting the limit explicitly to 0 does
work, which is good.

The whole series looks fine to me.

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

Re: [PATCH v2] for-each-ref: add %(upstream:gone) to mark missing refs

[Jeff King]

On Wed, Aug 24, 2016 at 08:26:26PM +0200, Øystein Walle wrote:

> In the mean time, however, I have discovered that this conflicts with
> kn/ref-filter-branch-list in pu. In that topic this specific feature is
> implemented as well. They incorporate it into %(upstream:track) instead
> of having a separate "sub-atom" (what's the correct nomenclature, by the
> way?) more in line with with branch -vv and your idea.

Ah, right. I was feeling like this was all vaguely familiar. I think it
would be better to push forward kn/ref-filter-branch-list. According to
the last "what's cooking", I think that topic is waiting on more review.
If you're willing and able to do so, that would be a big help.

Thanks.

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

Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Eric Wong]

Johannes Schindelin  wrote:
> Hi Arif,
> 
> On Tue, 23 Aug 2016, Arif Khokar wrote:
> 
> > On 08/20/2016 03:57 PM, Jakub Narębski wrote:
> > 
> > > But perhaps the problem is current lack of tooling in the opposite
> > > direction, namely getting patches from mailing list and applying them
> > > to GitHub repo, or Bitbucket, or GitLab.  Though with working Git, it
> > > is something easier than sending patches via email; it is enough that
> > > email client can save email to a file (or better, whole sub-thread to
> > > file or files).
> > 
> > Given that public-inbox provides an NNTP interface, couldn't the ARTICLE
> >  NNTP command be used to easily retrieve the messages in a
> > given patch series (at least compared to POP or IMAP).  Perhaps
> > git-send-email could be modified to include the message-id value of each
> > patch in the series that it sends to the mailing list and include it in
> > the cover letter.

I think that makes sense; perhaps an X-Git-Followups: header
from send-email which lists the child Message-IDs the same way
References: does for ancestors.  (perhaps there's already a
standardized header for listing children)

I thought about allowing a giant MIME message with all the
patches attached, too but that won't work for a large patch
series due to size limits along various SMTP hops.
Compression might make spam filters unhappy, too.

> I am no expert in the NNTP protocol (I abandoned News long ago), but if
> you go from HTML, you can automate the process without requiring changes
> in format-patch.
> 
> > Then a script could be written (i.e., git-download-patch) which could
> > parse the cover letter message (specified using its message-id), and
> > download all the patches in series, which can then be applied using
> > git-am.  This would in fact take the email client out of the equation in
> > terms of saving patches.

w3m -dump -dump_source nntp:///

ought to already work for news.gmane.org and news.public-inbox.org

The Net::NNTP Perl module is a standard part of the Perl distro
for many years, now (along with Net::SMTP), so that would not
be a roadblock for implementing a custom downloader distributed
with git.

> I recently adapted an old script I had to apply an entire patch series
> given the GMane link to its cover letter:
> 
> https://github.com/git-for-windows/build-extra/blob/master/apply-from-gmane.sh
> 
> Maybe you find it in you to adapt that to work with public-inbox.org?

I would be hesitant to depend too much on public-inbox.org until
more mirrors appear.  Even then, NNTP is a better-established
protocol and a fallback to news.gmane still works.
(public-inbox.org is powered by hamsters running on wheels,
 sometimes I let them rest :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] push: change submodule default to check

[Junio C Hamano]

Stefan Beller  writes:

> When working with submodules, it is easy to forget to push the submodules.
> The setting 'check', which checks if any existing submodule is present on
> at least one remote of the submodule remotes, is designed to prevent this
> mistake.
>
> Flipping the default to check for submodules is safer than the current
> default of ignoring submodules while pushing.
>
> Signed-off-by: Stefan Beller 
> ---
>
> Slightly reworded commit message than in v1,
> (https://public-inbox.org/git/20160817204848.8983-1-sbel...@google.com/)
> The patch itself is however the same.
>
> I just push it out now with a new commit message, such that we can easier
> pick it up later for Git 3.0, when changes that change default make more 
> sense.
>
> As said in an earlier message, you could however also argue that this is
> fixing a bug in your workflow, so it might be worth fixing before 3.0
> as well. I dunno.

With this change suddenly landing on the version of Git a user just
updated, the only possible negative effect would be that somebody
who did not configure push.recurseSubmodules suddenly starts getting
an error.  What would the error message say?

What I want you to think about and explain in the justification is
if it gives the user enough information to decide the best course of
action to adjust to the new world order, when the user's expectation
has been that the superproject gets pushed independent from its
submodules.  If the existing error message gives enough information,
explains why Git suddenly started refusing the push is generally a
good thing for the user, tells some minority users (in whose
workflow it is perfectly safe to push out the toplevel independently
from the submodule) how to turn it back to the old behaviour clearly
enough, then this single-liner may be good enough.  Otherwise we may
need a new logic to allow us to see if recurse_submodules that is
set to RECURSE_SUBMODULES_CHECK is because the user explicitly
configured, or because the user did not have any configuration, and
issue a different message in the latter case.



>  builtin/push.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 3bb9d6b..479150a 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -22,7 +22,7 @@ static int deleterefs;
>  static const char *receivepack;
>  static int verbosity;
>  static int progress = -1;
> -static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
> +static int recurse_submodules = RECURSE_SUBMODULES_CHECK;
>  static enum transport_family family;
>  
>  static struct push_cas_option cas;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 2/3] unpack-objects: add --max-input-size= option

[Christian Couder]

When receiving a pack-file, it can be useful to abort the
`git unpack-objects`, if the pack-file is too big.

Signed-off-by: Christian Couder 
---
 Documentation/git-unpack-objects.txt | 3 +++
 builtin/unpack-objects.c | 7 +++
 2 files changed, 10 insertions(+)

diff --git a/Documentation/git-unpack-objects.txt 
b/Documentation/git-unpack-objects.txt
index 3e887d1..b3de50d 100644
--- a/Documentation/git-unpack-objects.txt
+++ b/Documentation/git-unpack-objects.txt
@@ -44,6 +44,9 @@ OPTIONS
 --strict::
Don't write objects with broken content or links.
 
+--max-input-size=::
+   Die, if the pack is larger than .
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 172470b..4532aa0 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -19,6 +19,7 @@ static const char unpack_usage[] = "git unpack-objects [-n] 
[-q] [-r] [--strict]
 static unsigned char buffer[4096];
 static unsigned int offset, len;
 static off_t consumed_bytes;
+static off_t max_input_size;
 static git_SHA_CTX ctx;
 static struct fsck_options fsck_options = FSCK_OPTIONS_STRICT;
 
@@ -87,6 +88,8 @@ static void use(int bytes)
if (signed_add_overflows(consumed_bytes, bytes))
die("pack too large for current definition of off_t");
consumed_bytes += bytes;
+   if (max_input_size && consumed_bytes > max_input_size)
+   die(_("pack exceeds maximum allowed size"));
 }
 
 static void *get_data(unsigned long size)
@@ -550,6 +553,10 @@ int cmd_unpack_objects(int argc, const char **argv, const 
char *prefix)
len = sizeof(*hdr);
continue;
}
+   if (skip_prefix(arg, "--max-input-size=", )) {
+   max_input_size = strtoumax(arg, NULL, 10);
+   continue;
+   }
usage(unpack_usage);
}
 
-- 
2.10.0.rc1.3.g93be2b9

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

[PATCH v3 3/3] receive-pack: allow a maximum input size to be specified

[Christian Couder]

From: Jeff King 

Receive-pack feeds its input to either index-pack or
unpack-objects, which will happily accept as many bytes as
a sender is willing to provide. Let's allow an arbitrary
cutoff point where we will stop writing bytes to disk.

Cleaning up what has already been written to disk is a
related problem that is not addressed by this patch.

Signed-off-by: Jeff King 
Signed-off-by: Christian Couder 
---
 Documentation/config.txt   |  6 +
 Documentation/git-receive-pack.txt |  3 +++
 builtin/receive-pack.c | 12 +
 t/t5546-receive-limits.sh  | 55 ++
 4 files changed, 76 insertions(+)
 create mode 100755 t/t5546-receive-limits.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..8a115b3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2517,6 +2517,12 @@ receive.unpackLimit::
especially on slow filesystems.  If not set, the value of
`transfer.unpackLimit` is used instead.
 
+receive.maxInputSize::
+   If the size of the incoming pack stream is larger than this
+   limit, then git-receive-pack will error out, instead of
+   accepting the pack file. If not set or set to 0, then the size
+   is unlimited.
+
 receive.denyDeletes::
If set to true, git-receive-pack will deny a ref update that deletes
the ref. Use this to prevent such a ref deletion via a push.
diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index 000ee8d..0ccd5fb 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -33,6 +33,9 @@ post-update hooks found in the Documentation/howto directory.
 option, which tells it if updates to a ref should be denied if they
 are not fast-forwards.
 
+A number of other receive.* config options are available to tweak
+its behavior, see linkgit:git-config[1].
+
 OPTIONS
 ---
 ::
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 011db00..f1ce05c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -46,6 +46,7 @@ static int transfer_unpack_limit = -1;
 static int advertise_atomic_push = 1;
 static int advertise_push_options;
 static int unpack_limit = 100;
+static off_t max_input_size;
 static int report_status;
 static int use_sideband;
 static int use_atomic;
@@ -212,6 +213,11 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (strcmp(var, "receive.maxinputsize") == 0) {
+   max_input_size = git_config_int64(var, value);
+   return 0;
+   }
+
return git_default_config(var, value, cb);
 }
 
@@ -1648,6 +1654,9 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
if (fsck_objects)
argv_array_pushf(, "--strict%s",
fsck_msg_types.buf);
+   if (max_input_size)
+   argv_array_pushf(, 
"--max-input-size=%"PRIuMAX,
+   (uintmax_t)max_input_size);
child.no_stdout = 1;
child.err = err_fd;
child.git_cmd = 1;
@@ -1676,6 +1685,9 @@ static const char *unpack(int err_fd, struct shallow_info 
*si)
fsck_msg_types.buf);
if (!reject_thin)
argv_array_push(, "--fix-thin");
+   if (max_input_size)
+   argv_array_pushf(, 
"--max-input-size=%"PRIuMAX,
+   (uintmax_t)max_input_size);
child.out = -1;
child.err = err_fd;
child.git_cmd = 1;
diff --git a/t/t5546-receive-limits.sh b/t/t5546-receive-limits.sh
new file mode 100755
index 000..10cb0be
--- /dev/null
+++ b/t/t5546-receive-limits.sh
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+test_description='check receive input limits'
+. ./test-lib.sh
+
+# Let's run tests with different unpack limits: 1 and 1
+# When the limit is 1, `git receive-pack` will call `git index-pack`.
+# When the limit is 1, `git receive-pack` will call `git unpack-objects`.
+
+test_pack_input_limit () {
+   case "$1" in
+   index) unpack_limit=1 ;;
+   unpack) unpack_limit=1 ;;
+   esac
+
+   test_expect_success 'prepare destination repository' '
+   rm -fr dest &&
+   git --bare init dest
+   '
+
+   test_expect_success "set unpacklimit to $unpack_limit" '
+   git --git-dir=dest config receive.unpacklimit "$unpack_limit"
+   '
+
+   test_expect_success 'setting receive.maxInputSize to 512 rejects push' '
+   git --git-dir=dest config receive.maxInputSize 512 &&
+   test_must_fail git push dest HEAD
+   '
+
+   test_expect_success 'bumping limit to 4k allows push' '
+   git 

[PATCH v3 1/3] index-pack: add --max-input-size= option

[Christian Couder]

From: Jeff King 

When receiving a pack-file, it can be useful to abort the
`git index-pack`, if the pack-file is too big.

Signed-off-by: Jeff King 
Signed-off-by: Christian Couder 
---
 Documentation/git-index-pack.txt | 2 ++
 builtin/index-pack.c | 5 +
 2 files changed, 7 insertions(+)

diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 7a4e055..1b4b65d 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -87,6 +87,8 @@ OPTIONS
Specifying 0 will cause Git to auto-detect the number of CPU's
and use maximum 3 threads.
 
+--max-input-size=::
+   Die, if the pack is larger than .
 
 Note
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 1d2ea58..4a8b4ae 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -87,6 +87,7 @@ static struct progress *progress;
 static unsigned char input_buffer[4096];
 static unsigned int input_offset, input_len;
 static off_t consumed_bytes;
+static off_t max_input_size;
 static unsigned deepest_delta;
 static git_SHA_CTX input_ctx;
 static uint32_t input_crc32;
@@ -297,6 +298,8 @@ static void use(int bytes)
if (signed_add_overflows(consumed_bytes, bytes))
die(_("pack too large for current definition of off_t"));
consumed_bytes += bytes;
+   if (max_input_size && consumed_bytes > max_input_size)
+   die(_("pack exceeds maximum allowed size"));
 }
 
 static const char *open_pack_file(const char *pack_name)
@@ -1714,6 +1717,8 @@ int cmd_index_pack(int argc, const char **argv, const 
char *prefix)
opts.off32_limit = strtoul(c+1, , 0);
if (*c || opts.off32_limit & 0x8000)
die(_("bad %s"), arg);
+   } else if (skip_prefix(arg, "--max-input-size=", )) 
{
+   max_input_size = strtoumax(arg, NULL, 10);
} else
usage(index_pack_usage);
continue;
-- 
2.10.0.rc1.3.g93be2b9

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

[PATCH v3 0/3] limit the size of the packs we receive

[Christian Couder]

Goal


In https://public-inbox.org/git/20150612182045.GA23698%40peff.net/,
Peff sent a patch that is used by GitHub to abort `git receive-pack`
when the size of the pack we receive is bigger than a configured
limit.

GitLab is interested in using the same approach and in standardizing
the error messages the user could get back.

Comments


I kept Peff as the author of the patches that are made mostly from his
patch, but I added my Signed-off-by to them.

Changes from previous v2 version


All these changes have been suggested by Junio and are in patch 3/3,
the other 2 patches are the same as v2:

  - renamed "receive.maxsize" to "receive.maxInputSize",
  - improved commit message,
  - renamed test script from "t5546-push-limits.sh" to
"t5546-receive-limits.sh",
  - improved the tests in the last patch by adding a
test_pack_input_limit() function and deleting the destination repo
at the beginning of this function.

Links
~

This patch series is available here:

https://github.com/chriscool/git/commits/max-receive

The previous versions are here on GitHub:

RFC: https://github.com/chriscool/git/commits/max-receive2
v1: https://github.com/chriscool/git/commits/max-receive6
v2: https://github.com/chriscool/git/commits/max-receive7

and here on the list:

RFC: 
https://public-inbox.org/git/20160815195729.16826-1-chrisc...@tuxfamily.org/
v1: https://public-inbox.org/git/20160816081701.29949-1-chrisc...@tuxfamily.org/
v2: https://public-inbox.org/git/20160818131553.22580-1-chrisc...@tuxfamily.org/

Peff's initial patch is:

https://public-inbox.org/git/20150612182045.GA23698%40peff.net/

Diff with previous v2 version
~

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f5b6061..8a115b3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2517,10 +2517,11 @@ receive.unpackLimit::
especially on slow filesystems.  If not set, the value of
`transfer.unpackLimit` is used instead.
 
-receive.maxsize::
-   If the size of a pack file is larger than this limit, then
-   git-receive-pack will error out, instead of accepting the pack
-   file. If not set or set to 0, then the size is unlimited.
+receive.maxInputSize::
+   If the size of the incoming pack stream is larger than this
+   limit, then git-receive-pack will error out, instead of
+   accepting the pack file. If not set or set to 0, then the size
+   is unlimited.
 
 receive.denyDeletes::
If set to true, git-receive-pack will deny a ref update that deletes
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4b0379b..f1ce05c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -213,7 +213,7 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
-   if (strcmp(var, "receive.maxsize") == 0) {
+   if (strcmp(var, "receive.maxinputsize") == 0) {
max_input_size = git_config_int64(var, value);
return 0;
}
diff --git a/t/t5546-push-limits.sh b/t/t5546-push-limits.sh
deleted file mode 100755
index 09e958f..000
--- a/t/t5546-push-limits.sh
+++ /dev/null
@@ -1,42 +0,0 @@
-#!/bin/sh
-
-test_description='check input limits for pushing'
-. ./test-lib.sh
-
-test_expect_success 'create remote repository' '
-   git init --bare dest
-'
-
-# Let's run tests with different unpack limits: 1 and 10
-# When the limit is 1, `git receive-pack` will call `git index-pack`.
-# When the limit is 10, `git receive-pack` will call `git unpack-objects`.
-
-while read unpacklimit filesize filename seed
-do
-
-   test_expect_success "create known-size ($filesize bytes) commit 
'$filename'" '
-   test-genrandom "$seed" "$filesize" >"$filename" &&
-   git add "$filename" &&
-   test_commit "$filename"
-   '
-
-   test_expect_success "set unpacklimit to $unpacklimit" '
-   git --git-dir=dest config receive.unpacklimit "$unpacklimit"
-   '
-
-   test_expect_success 'setting receive.maxsize to 512 rejects push' '
-   git --git-dir=dest config receive.maxsize 512 &&
-   test_must_fail git push dest HEAD
-   '
-
-   test_expect_success 'bumping limit to 4k allows push' '
-   git --git-dir=dest config receive.maxsize 4k &&
-   git push dest HEAD
-   '
-
-done <<\EOF
-1 1024 one-k-file foo
-10 1024 other-one-k-file bar
-EOF
-
-test_done
diff --git a/t/t5546-receive-limits.sh b/t/t5546-receive-limits.sh
new file mode 100755
index 000..10cb0be
--- /dev/null
+++ b/t/t5546-receive-limits.sh
@@ -0,0 +1,55 @@
+#!/bin/sh
+
+test_description='check receive input limits'
+. ./test-lib.sh
+
+# Let's run tests with different unpack limits: 1 and 1
+# When the limit is 1, `git receive-pack` will call `git index-pack`.
+# When the limit is 1, `git 

Re: [PATCH v2] for-each-ref: add %(upstream:gone) to mark missing refs

[Øystein Walle]

Hi, Peff

On 24 August 2016 at 20:07, Jeff King  wrote
>
> Whoops, your v2 spurred me to review, but I accidentally read and
> responded to v1.
>

Thanks for the review! I was worried this patch had been buried :-)

In the mean time, however, I have discovered that this conflicts with
kn/ref-filter-branch-list in pu. In that topic this specific feature is
implemented as well. They incorporate it into %(upstream:track) instead
of having a separate "sub-atom" (what's the correct nomenclature, by the
way?) more in line with with branch -vv and your idea.

I recall seeing discussions about this work earlier, but I based my
patch on master and forgot to check pu. (It was a spur-of-the-moment
thing fueled by a question in #git about how to parse branch -vv to
delete all local branch who had their remote counter-parts removed after
a fetch --prune.)

Unless that topic gets rejected, or is known to not be merged for a
_long_ while, my patch doesn't add much value.

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

Re: [PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Junio C Hamano  writes:

> Mentioned elsewhere, but I think the above should be
>
>   if (!path)
>   path = obj_context.path;
>
>   if (obj_context.mode == S_IFINVALID)
>   obj_context.mode = 0100644;
>
> IOW, even when there is an explicit path supplied, we should fall
> back to assumed "regular blob" mode, so that
>
>   git cat-file --filters --path=README $(git rev-parse :README)
>
> would work as expected.

Actually, I am reading the conditional the other way, but the
conclusion "defaulting from unknown mode to regular blob is
necessary whether the user gave us a path or not" is the same.

The current code may fail if --path is not available and 40-hex that
does not give us any context of look up is given because it won't be
able to decide how to filter, so using "else if" would not have
practical difference there, but conceptually it still is wrong.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] push: change submodule default to check

[Junio C Hamano]

Stefan Beller  writes:

> I guess we can postpone it until 3.0, though I currently think it is not a big
> issue as it helps avoiding "bugs in your workflow".
>
> On the other hand if you really want to push out the superproject without
> the submodules, you need to adapt your behavior (i.e. set an option or
> give a command line flag), and such breaking things we should delay
> until 3.0.
>
> I think I'll resend it with a proper commit message, such that we can just 
> pick
> it up when 3.0 comes around.

A change that needs to wait until a major version bump implies that
it is possibly compatibility breaking.  So "resend IT", implying one
single step, sounds like a bad sign that the users won't have any
transition period.  Shouldn't we do the usual two-step deprecation
process, i.e. warn when an unconfigured user pushes a superproject
that may be ahead of a submodule about upcoming planned default
change with the first patch, and then flip the default in the second
patch while dropping the warning?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] for-each-ref: add %(upstream:gone) to mark missing refs

[Jeff King]

On Mon, Aug 22, 2016 at 07:35:28PM +0200, Øystein Walle wrote:

> git branch -vv will show "gone" next to a remote tracking branch if it
> does not exist. for-each-ref is suitable for parsing but had no way of
> showing this information.
> 
> This introduces "%(upstream:gone)" to display "gone" in the formatted
> output if the ref does not exist or an empty string otherwise, analogous
> to git branch -vv.
> 
> Signed-off-by: Øystein Walle 
> ---
> I took the liberty of sending in a v2 on my own. Removed the last argument to
> stat_tracking_info() and used test_config instead of test_when_finished.

Whoops, your v2 spurred me to review, but I accidentally read and
responded to v1.

I think test_config may not be the right thing here, though; see my
other comments.

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

Re: [PATCH] for-each-ref: add %(upstream:gone) to mark missing refs

[Jeff King]

On Fri, Aug 19, 2016 at 11:50:23PM +0200, Øystein Walle wrote:

> git branch -vv will show "gone" next to a remote tracking branch if it
> does not exist. for-each-ref is suitable for parsing but had no way of
> showing this information.
> 
> This introduces "%(upstream:gone)" to display "gone" in the formatted
> output if the ref does not exist or an empty string otherwise, analogous
> to git branch -vv.

Hmm. So this sounds like the minimum thing to implement "branch -vv"
output, but it feels sort of weirdly non-orthogonal. I guess you'd say:

  %(upstream:track)%(upstream:gone)

and assume that only one of them will show any data.

I almost wonder if %(upstream:track) should just output some specific
token for the "gone" case, which matches what "branch -vv" does.

(I'm also not sure we can match "branch -vv" exactly here anyway, as it
looks like "[%(upstream): %(upstream:track)"], but our
"%(upstream:track)" uses its own brackets.)

I suppose that is a backwards-incompatible change for
"%(upstream:track)", but I'm not sure how much we have promised about
its format. The error cases seem totally undocumented.

> ---
>  Documentation/git-for-each-ref.txt |  5 +++--
>  ref-filter.c   | 10 +-
>  t/t6300-for-each-ref.sh| 12 
>  3 files changed, 24 insertions(+), 3 deletions(-)

The code itself looks OK to me (assuming we want upstream:gone as the
interface), with a few minor nits in the test:

> +test_expect_success 'Check that :gone produces expected results' '
> + cat >expected <<-\EOF &&
> +gone
> + EOF

The "<<-" will strip leading tabs, meaning you can indent "gone" here to
match the rest of the test.

> + test_when_finished "git config branch.master.merge refs/heads/master" &&
> + git config branch.master.merge refs/heads/does-not-exist &&

I thought this could be test_config, but it is not clever enough to
restore an old value, only to remove a value we just added. So this is
probably the best solution.

> + git for-each-ref \
> + --format="%(upstream:gone)" \
> + refs/heads >actual &&
> + test_cmp expected actual
> +'

This tests only that we show "gone"; do you want to add a second branch
and make sure that it shows nothing? Maybe you could even use a new
branch name, which makes test_config applicable again:

  cat >expected <<-\EOF &&
  master 
  no-upstream gone
  EOF
  git branch refs/heads/no-upstream &&
  test_config branch.no-upstream.remote . &&
  test_config branch.no-upstream.merge refs/heads/does-not-exist &&
  git for-each-ref --format="%(refname:short) %(upstream:gone)" >actual &&
  test_cmp expected actual

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

Re: [PATCH 3/7] release_delta_base_cache: reuse existing detach function

[Junio C Hamano]

Jeff King  writes:

> That is not too bad, I guess. I can switch it if you prefer that way.
> Since there are only these two callers with two different sets of needs
> (and the function is static), I just let them continue inspecting the
> elements directly.

I do not think it is too hard to refactor later when need arises; it
is not like we have too many callers and this is a file-scope static
helper to begin with.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] push: change submodule default to check

[Stefan Beller]

When working with submodules, it is easy to forget to push the submodules.
The setting 'check', which checks if any existing submodule is present on
at least one remote of the submodule remotes, is designed to prevent this
mistake.

Flipping the default to check for submodules is safer than the current
default of ignoring submodules while pushing.

Signed-off-by: Stefan Beller 
---

Slightly reworded commit message than in v1,
(https://public-inbox.org/git/20160817204848.8983-1-sbel...@google.com/)
The patch itself is however the same.

I just push it out now with a new commit message, such that we can easier
pick it up later for Git 3.0, when changes that change default make more sense.

As said in an earlier message, you could however also argue that this is
fixing a bug in your workflow, so it might be worth fixing before 3.0
as well. I dunno.

Thanks,
Stefan

 builtin/push.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..479150a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -22,7 +22,7 @@ static int deleterefs;
 static const char *receivepack;
 static int verbosity;
 static int progress = -1;
-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int recurse_submodules = RECURSE_SUBMODULES_CHECK;
 static enum transport_family family;
 
 static struct push_cas_option cas;
-- 
2.10.0.rc1.1.g1ceb01a

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

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Junio C Hamano]

Jeff King  writes:

> I don't suppose anybody cares that much either way, but it feels weird
> to behave differently depending on how we looked up the blob (whereas
> for the HEAD:t case, a tree is always a tree).

I do not care strongly either way, but HEAD:RelNotes case we _know_
it is not a regular blob, and discarding that info for the sake of
being consistent with the common denominator case, instead of using
it to point out a possible mistake, feels wrong, too.

In any case, not outputting anything and silently succeeding is
definitely wrong ;-).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Junio C Hamano  writes:

>> +test_expect_success 'setup ' '
>> +echo "*.txt eol=crlf diff=txt" >.gitattributes &&
>> +echo "hello" | append_cr >world.txt &&
>> +git add .gitattributes world.txt &&
>
>   git update-index --cacheinfo :world.txt,$EMPTY_BLOB,symlink &&

Sorry, last-minute-edit-gone-bad-without-proofreading.  It should
have been something like:

git update-index --add --cacheinfo 16,$EMPTY_BLOB,symlink &&

or

sym=$(echo "hello" | git hash-objects --stdin -w) &&
git update-index --add --cacheinfo 16,$sym,symlink &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Adding more namespace support to git

[Jeff King]

On Sat, Aug 20, 2016 at 08:07:00PM +0100, Richard wrote:

> Because git is not namespace aware for anything but git-upload-pack
> and git-receive-pack, I've had to implement namespace parsing in cgit
> for listing branches, showing logs, displaying notes and commit
> decorations.  It might be more useful if this support was added to git
> itself, so other git servers could make use of it so there's less
> duplicated code.
> 
> I think the way to do this would be to make the low-level ref reading
> functions, read_raw_ref, for_each_reflog_ent*, reflog_exists etc.,
> interpret the ref they are passed as being relative to the current git
> namespace.

At GitHub, we store many forks for a single repository, and we
considered using namespaces for our storage strategy. But like you, we
ran into the problem that you they only work for certain operations. :)

Our solution is to use separate repositories, each with their own ref
storage, but pointing to a shared object store. It works, but there are
a lot of gotchas and performance issues with migrating objects around,
running repacks.

Michael Haggerty (cc'd) has picked up the pluggable ref-backend work
started by others, and I know has some ideas on doing namespaces at that
level. Basically, the concept of "namespaces" should be able to plug in
between the actual storage backend and the rest of the git code. Git
code wouldn't have to care whether the namespace plugin was in use, and
the namespace plugin wouldn't have to care which storage backend was in
use (it would just silently translate "refs/heads/foo" into
"refs/namespaces/123/heads/foo", and vice versa).

That's a very "complete" solution in the sense that the git code does
not know about the namespaces, and cannot even access refs outside of
it. But I think in general it would do what you want. Most operations
would run in a certain namespace (i.e., pretend nothing outside of that
namespace exists, for fetches, diffs, etc), and others would want to
look at the whole namespace (e.g., repacking, pruning). I don't know of
any operations that want to see both views in the same process.

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

Re: [PATCH v2 2/4] cat-file: introduce the --filters option

[Junio C Hamano]

Johannes Schindelin  writes:

> +static int filter_object(const char *path, unsigned mode,
> +  const unsigned char *sha1,
> +  char **buf, unsigned long *size)
> +{
> + enum object_type type;
> +
> + *buf = read_sha1_file(sha1, , size);
> + if (!*buf)
> + return error(_("cannot read object %s '%s'"),
> + sha1_to_hex(sha1), path);
> + if (type != OBJ_BLOB) {
> + free(*buf);
> + return error(_("blob expected for %s '%s'"),
> + sha1_to_hex(sha1), path);
> + }
> + if (S_ISREG(mode)) {
> + struct strbuf strbuf = STRBUF_INIT;
> + if (convert_to_working_tree(path, *buf, *size, )) {
> + free(*buf);
> + *size = strbuf.len;
> + *buf = strbuf_detach(, NULL);
> + }
> + }

When we see a blob that is not ISREG, what is expected to happen?
Is it an error?

We can argue both ways.

Currently the ONLY kind of blob that is not ISREG is a symbolic
link, and it might be OK to output it as-is without any conversion
[*1*], in which case we can simply lose the S_ISREG(mode) check
altogether (and "mode" parameter to this function).

On the other hand, because "cat-file --filters" is meant to be a
smaller-granularity operation that could be used as a building block
to emulate what "git checkout" does, i.e. "imagine that we had the
blob in the index and checking it out from the path, and hand the
caller what would have been written out to the filesystem, so that
the convert_to_working_tree() logic does not have to be emulated in
the userspace", erroring out when we see a symbolic link may be also
a valid way to handle it.  We know the usual CRLF and other conversions
do not apply to them.

In any case, silently succeeding without any output is probably what
the end-user least expects.

If we choose to fail the request, the necessary update to the test
would look like this, I think.

> diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
> new file mode 100755
> index 000..e466634
> --- /dev/null
> +++ b/t/t8010-cat-file-filters.sh
> @@ -0,0 +1,34 @@
> +#!/bin/sh
> +
> +test_description='git cat-file filters support'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup ' '
> + echo "*.txt eol=crlf diff=txt" >.gitattributes &&
> + echo "hello" | append_cr >world.txt &&
> + git add .gitattributes world.txt &&

git update-index --cacheinfo :world.txt,$EMPTY_BLOB,symlink &&

> + test_tick &&
> + git commit -m "Initial commit"
> +'
> +
> +has_cr () {
> + tr '\015' Q <"$1" | grep Q >/dev/null
> +}
> +
> +test_expect_success 'no filters with `git show`' '
> + git show HEAD:world.txt >actual &&
> + ! has_cr actual
> +
> +'
> +
> +test_expect_success 'no filters with cat-file' '
> + git cat-file blob HEAD:world.txt >actual &&
> + ! has_cr actual
> +'
> +
> +test_expect_success 'cat-file --filters converts to worktree version' '
> + git cat-file --filters HEAD:world.txt >actual &&
> + has_cr actual
> +'

test_expect_success 'cat-file --filters rejects a non-blob' '
test_must_fail git cat-file --filters HEAD:
'

test_expect_success 'cat-file --filters rejects a non-regular blob' '
test_must_fail git cat-file --filters HEAD:symlink
'

And if we choose to accept as long as it is a blob, then the last
step can lose test_must_fail (and perhaps the result needs to be
checked against "hello" without CR).


[Footnote]

*1* But of course other kinds of non-ISREG blob we would add later
may not be something you would want to write out as-is.

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

Re: [PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Junio C Hamano]

Johannes Schindelin  writes:

> @@ -65,6 +68,11 @@ static int cat_one_file(int opt, const char *exp_type, 
> const char *obj_name,
>   if (get_sha1_with_context(obj_name, 0, sha1, _context))
>   die("Not a valid object name %s", obj_name);
>  
> + if (!path)
> + path = obj_context.path;
> + else if (obj_context.mode == S_IFINVALID)
> + obj_context.mode = 0100644;
> +
>   buf = NULL;
>   switch (opt) {
>   case 't':

Mentioned elsewhere, but I think the above should be

if (!path)
path = obj_context.path;

if (obj_context.mode == S_IFINVALID)
obj_context.mode = 0100644;

IOW, even when there is an explicit path supplied, we should fall
back to assumed "regular blob" mode, so that

git cat-file --filters --path=README $(git rev-parse :README)

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

Re: [PATCH 3/7] release_delta_base_cache: reuse existing detach function

[Jeff King]

On Tue, Aug 23, 2016 at 02:49:15PM -0700, Junio C Hamano wrote:

> > diff --git a/sha1_file.c b/sha1_file.c
> > index 1d0810c..8264b39 100644
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -2152,10 +2152,7 @@ static inline void release_delta_base_cache(struct 
> > delta_base_cache_entry *ent)
> >  {
> > if (ent->data) {
> > free(ent->data);
> > -   ent->data = NULL;
> > -   ent->lru.next->prev = ent->lru.prev;
> > -   ent->lru.prev->next = ent->lru.next;
> > -   delta_base_cached -= ent->size;
> > +   detach_delta_base_cache_entry(ent);
> 
> If we were designing this from scratch, we might have made detach_*
> to return the pointer to minimize direct access to ent->data, but I
> do not think it is worth it.  Looks very sensible.

I actually looked into that during the conversion in patch 2/7. I didn't
want to just return ent->data, because there are actually several bits
of information to "rescue" from the entry. So it looks more like:

  char *data = detach_delta_base_cache_entry(ent, NULL, NULL);
  free(data);

here, and

  data = detach_delta_base_cache_entry(ent, , );

in unpack_entry().

That is not too bad, I guess. I can switch it if you prefer that way.
Since there are only these two callers with two different sets of needs
(and the function is static), I just let them continue inspecting the
elements directly.

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

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Jeff King]

On Wed, Aug 24, 2016 at 10:02:39AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Wed, Aug 24, 2016 at 09:09:06AM -0700, Junio C Hamano wrote:
> >
> >> >  +   if (!path)
> >> >  +   path = obj_context.path;
> >> >  +   else if (obj_context.mode == S_IFINVALID)
> >> >  +   obj_context.mode = 0100644;
> >> >  +
> >> >  buf = NULL;
> >> >  switch (opt) {
> >> >  case 't':
> >> 
> >> The above two hunks make all the difference in the ease of reading
> >> the remainder of the function.  Very good.
> >
> > Yeah, I agree. Though it took me a moment to figure out why we were
> > setting obj_context.mode but not obj_context.path; the reason is that
> > "mode" is convenient to use as local storage, but "path" is not, because
> > it is not a pointer but an array.
> 
> Wait a minute.  Why is it a cascaded if/elseif, not two independent
> if statements that gives a default value?  In other words, wouldn't
> these two independent and orthogonal decisions?
> 
>  * When forced to use some path, we ignore obj_context.path
> 
>  * Whether we are forced to use a path or not, if we do not know the
>mode from the lookup context, we want to use the regular blob
>mode.
> 
> So that part of the patch is wrong after all, I would have to say.
> 
>   if (!path)
>   path = obj_context.path;
>   if (obj_context.mode == S_IFINVALID)
>   obj_context.mode = 0100644;
> 
> or something like that, perhaps.

Oh, hrm, you are right. I assumed we wanted to force the mode when
--path was in effect, but that is not what the original does. If you
say:

  --path=foo HEAD:bar

then we will take the mode for "bar", whatever it is (maybe a tree or
symlink). But if you say:

  --path=foo $(git rev-parse HEAD:bar)

then we will use 100644, regardless of what "bar" is in HEAD.

I have not thought about it enough to know if that is a good thing or a
bad thing. But I'll bet Dscho has, so I will wait for him to comment. :)

> >   if (!force_path) {
> > /* use file info from sha1 lookup */
> > path = obj_context.path;
> > mode = obj_context.mode;
> >   } else {
> > /* use path requested by user, and assume it is a regular file */
> > path = force_path;
> > mode = 0100644;
> >   }
> 
> Hmph, if you read it that way, then if/elseif makes some sense, but
> we need to assume that the obj_context.mode can be garbage and have
> a fallback for it.
> 
> Just like
> 
>   git cat-file --filters --path=git.c HEAD:t
> 
> would error out because HEAD:t is not even a blob, I would expect
> 
>   git cat-file --filters --path=git.c :RelNotes
> 
> to error out, because the object itself _is_ known to be a
> blob that is not a regular file.
> 
> And that kind of type checking will not be possible with "if the
> user gave us a path, assume it is a regular file".

Right, I agree that is the outcome, but I just wasn't sure that the
second case _should_ error out. IOW, does "--filters --path" mean "treat
this as a regular file at path X", or is the "regular file" part not
implied?

I don't suppose anybody cares that much either way, but it feels weird
to behave differently depending on how we looked up the blob (whereas
for the HEAD:t case, a tree is always a tree).

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

Re: Extending "extended SHA1" syntax to traverse through gitlinks?

[Jakub Narębski]

W dniu 24.08.2016 o 16:20, Josh Triplett pisze:
> On Wed, Aug 24, 2016 at 03:16:56PM +0200, Jakub Narębski wrote:
[...]
>> Not really.
>>
>> The above means only that the support for new syntax would be not
>> as easy as adding it to 'git rev-parse' (and it's built-in equivalent),
>> except for the case where submodule uses the same object database as
>> supermodule.
>>
>> So it wouldn't be as easy (on conceptual level) as adding support
>> for ':/' or '^{/}'.  It would be at least as
>> hard, if not harder, as adding support for '@{-1}' and its '-'
>> shortcut.
> 
> Depends on which cases you want to handle.  In the most general case,
> you'd need to find and process the applicable .gitmodules file, which
> would only work if you started from the top-level tree, not a random
> treeish.  On the other hand, in the most general case, you don't
> necessarily even have the module you need, because .git/modules only
> contains the modules the *current* version needed, not every past
> version.

There is an additional problem, namely that directory with submodule
can be renamed.

I don't know if there is an existing API, but assuming modern
git-submodule (with repository in .git/modules) you would have to
do the following steps for ://:

 * look up :.gitmodules for module which 'path'
   is ; let's say it is named 
 * check if : commit object
   is present in .git/modules/
 * look up this object

In the case of legacy submodule setup, with submodule repository
in the supermodule working directory, you would need:
   
 * look up :.gitmodules for module which 'path'
   is ; let's say it is named 
 * look up current .gitmodules for current path of submodule
   named ; let's say it is 
 * check of : commit object
   is present in :(top)/.git repository
 * look up this object

You could also check if the submodule repository (as stored
in config) is a path, and use it if it is... but that might
be going to far.


BTW. all that reminds me that gitweb should handle submodules
better.

> As an alternate approach (pun intended): treat every module in
> .git/modules as an alternate and just look up the object by hash.  

This could be a good fallback, to search through all submodules.

> Or, teach git-submodule to store all the objects for submodules in the
> supermodule's .git/objects (and teach git's reachability algorithm to
> respect refs in .git/modules, or store their refs in
> .git/refs/submodules/ or in a namespace).

And fallback to this fallback could be searching through supermodule
object repository.

Storing all objects in single repository is counter to the design
decision of submodules (though I don't remember what it was), but
it might be done.  Still, Git needs to be able to deal with legacy
situations anyway.
 
>> Josh, what was the reason behind proposing this feature? Was it
>> conceived as adding completeness to gitrevisions syntax, a low-hanging
>> fruit?  It isn't (the latter).  Or was it some problem with submodule
>> handling that you would want to use this syntax for?
> 
> This wasn't an abstract/theoretical completeness issue.  I specifically
> wanted this syntax for practical use with actual trees containing
> gitlinks, motivated by having a tool that creates and uses such
> gitlinks. :)

Could you explain what you need in more detail?  Is it a fragment
of history of submodule, a contents of a file at given point of
superproject history, diff between file-in-submodule and something
else, or what?
 
>> As for usefulness: this fills the hole in accessing submodules, one
>> that could be handled by combining plumbing-level commands.  Namely,
>> there are 5 states of submodule (as I understand it)
>>
>>  * recorded in ref / commit in supermodule
>>  * recorded in the index in supermodule
>>  - recorded in ref / commit in submodule
>>  - recorded in the index in submodule
>>  - state of worktree in submodule
>>
>> The last three can be easyly acessed by cd-ing to submodule.  The first
>> two are not easy to get, AFAIUC.
> 
> Right.  I primarily care about those first two cases, especially the
> first one: given a commit containing a gitlink, how can I easily dig
> into the linked commit?

All right.

Though you can cobble it with plumbing... just saying.

-- 
Jakub Narębski

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

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Junio C Hamano]

Jeff King  writes:

> On Wed, Aug 24, 2016 at 09:09:06AM -0700, Junio C Hamano wrote:
>
>> >  + if (!path)
>> >  + path = obj_context.path;
>> >  + else if (obj_context.mode == S_IFINVALID)
>> >  + obj_context.mode = 0100644;
>> >  +
>> >buf = NULL;
>> >switch (opt) {
>> >case 't':
>> 
>> The above two hunks make all the difference in the ease of reading
>> the remainder of the function.  Very good.
>
> Yeah, I agree. Though it took me a moment to figure out why we were
> setting obj_context.mode but not obj_context.path; the reason is that
> "mode" is convenient to use as local storage, but "path" is not, because
> it is not a pointer but an array.

Wait a minute.  Why is it a cascaded if/elseif, not two independent
if statements that gives a default value?  In other words, wouldn't
these two independent and orthogonal decisions?

 * When forced to use some path, we ignore obj_context.path

 * Whether we are forced to use a path or not, if we do not know the
   mode from the lookup context, we want to use the regular blob
   mode.

So that part of the patch is wrong after all, I would have to say.

if (!path)
path = obj_context.path;
if (obj_context.mode == S_IFINVALID)
obj_context.mode = 0100644;

or something like that, perhaps.

> So it would have been a little clearer to me as:
>
>   const char *path;
>   unsigned mode;
>   ...
>   if (!force_path) {
>   /* use file info from sha1 lookup */
>   path = obj_context.path;
>   mode = obj_context.mode;
>   } else {
>   /* use path requested by user, and assume it is a regular file */
>   path = force_path;
>   mode = 0100644;
>   }

Hmph, if you read it that way, then if/elseif makes some sense, but
we need to assume that the obj_context.mode can be garbage and have
a fallback for it.

Just like

git cat-file --filters --path=git.c HEAD:t

would error out because HEAD:t is not even a blob, I would expect

git cat-file --filters --path=git.c :RelNotes

to error out, because the object itself _is_ known to be a
blob that is not a regular file.

And that kind of type checking will not be possible with "if the
user gave us a path, assume it is a regular file".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Smart HTTP push permissions failure

[Jeff King]

On Tue, Aug 23, 2016 at 03:45:33PM +, David McGough wrote:

> When I try to push to the server I get this message:
> remote: error: insufficient permission for adding an object to repository 
> database ./objects
> remote: fatal: failed to write object
> [...]
> So I am pretty confused about what the issue.  Which OS user is git
> using to write the files?  I hope somebody can help me understand why
> the project cannot be pushed to the git server.

For a smart-http push, it will be whatever user the web server execs the
CGI as. So I'd think "apache" would be the default, but it's possible
that it runs CGIs as a different user, depending on your config.

One possibility may be to add a simple shell script CGI that does
something like:

  #!/bin/sh
  echo "Content-type: text/plain"
  echo
  id

just to see what's happening.

Based on the data you showed, here are some wild possibilities I can
think of:

  - the CGI runs as "apache", but your files are owned by "git".
"apache" is in the "staff" group, and the directories all have write
permission for that group. But are we sure that apache does not shed
any group permissions when running a CGI? The "id" script above
should hopefully show that.

  - You mentioned CentOS. It has been a while since I dealt with RHEL
and its derivatives, but I think selinux is turned on by default
there. Is it possible that the webserver runs in an selinux profile
that does not allow writing to the repository directory?

I don't recall the specifics of debugging selinux problems, but
there may be logs there.

Sorry those are just stabs in the dark, but I don't see anything else
obviously wrong with what you've posted.

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

Re: [RFC PATCH] completion: support excluding refs

[Junio C Hamano]

Chris Packham  writes:

> Allow completion of refs with a ^ prefix. This allows completion of
> commands like 'git log HEAD ^origin/master'.
> ...
> + [[ "$cur" == ^* ]] && pfx="^"
>   for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
> - if [ -e "$dir/$i" ]; then echo $i; fi
> + if [ -e "$dir/$i" ]; then echo $pfx$i; fi
>   done
>   format="refname:short"
>   refs="refs/tags refs/heads refs/remotes"
>   ;;
>   esac
> - git --git-dir="$dir" for-each-ref --format="%($format)" \
> + git --git-dir="$dir" for-each-ref --format="$pfx%($format)" \
>   $refs

That looks like a reasonable approach to me (but I am no expert in
this area myself).

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

Re: [PATCH] push: change submodule default to check

[Stefan Beller]

On Thu, Aug 18, 2016 at 7:09 AM, Heiko Voigt  wrote:
> On Wed, Aug 17, 2016 at 02:14:11PM -0700, Stefan Beller wrote:
>> On Wed, Aug 17, 2016 at 2:05 PM, Junio C Hamano  wrote:
>> > Stefan Beller  writes:
> [...]
>> >> Flipping the default to check for submodules is safer
>> >> than the current default of ignoring submodules while pushing.
>> >
>> > That part of the assertion, on the other hand, is justifiable.
>>
>> ok.
>
> I also think that this is a good reason to flip the default. IMO more
> people will be annoyed by not being able to checkout a certain version
> if someone forgets to push a submodule then people deliberately pushing
> something with a submodule hash that is not on any remote.
>
> At the same time I am wondering whether it makes sense to keep this for
> a bigger version change (like 3.0) or so? Since that is were people will
> expect such changes. Not sure when 3.0 is planned though.
>
> Cheers Heiko

I guess we can postpone it until 3.0, though I currently think it is not a big
issue as it helps avoiding "bugs in your workflow".

On the other hand if you really want to push out the superproject without
the submodules, you need to adapt your behavior (i.e. set an option or
give a command line flag), and such breaking things we should delay
until 3.0.

I think I'll resend it with a proper commit message, such that we can just pick
it up when 3.0 comes around.

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

Re: core.autocrlf, was Re: [git-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

[Junio C Hamano]

Johannes Schindelin  writes:

>> In any case, in the ideal future, I would imagine that we would want
>> to have "cat-file blob" to enable "--filters" by default; that would
>> make cat-file and hash-objects a pair of symmetric operations.
>
> I would advocate against that. It is not like the terms "hash-object" and
> "cat-file" even *look* like they are opposites.

I do not quite understand your objection.

hash-object is "I have data somewhere on the filesystem, and I want
to store it in the object store even though I am not ready to add it
to the index yet (or I may not even add it to the index ever), just
to make it available to Git tools".  cat-file is "I have data in the
object store, I want to make it available to my other tools that
understand data stored on the filesystem."

When we taught "--no-filters" to "hash-object" back in 2008, we
should have realized that "cat-file :path >path" would want to be a
way to do "checkout path" (with "--no-filters" option to allow us to
inspect the "canonical version"), but we didn't.

Yes, correcting ancient mistakes is costly.  Such is life.

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

Re: [PATCH] checkout: swap the order of ambiguity check for :/ syntax

[Junio C Hamano]

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

> It's not wonderful, but it's in line with how git-checkout stops caring
> about ambiguity after the first argument can be resolved as a ref
> (there's even a test for it, t2010.6).

But that is justifiable because checkout can only ever take one
revision.  What follows, if there are any, must be paths, and more
importantly, it would be perfectly reasonable if some of them were
missing in the working tree ("ow, I accidentally removed that file,
I need to resurrect it from the index").  Does the same justification
apply to this change?

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

Re: [PATCH] transport: report missing submodule pushes consistently on stderr

[Stefan Beller]

On Wed, Aug 24, 2016 at 3:28 AM, Leandro Lucarella
 wrote:
> On Tue, 23 Aug 2016 14:40:08 -0700
> Stefan Beller  wrote:
>> The surrounding advice is printed to stderr, but the list of
>> submodules is not. Make the report consistent by reporting everything
>> to stderr.
>>
>> Signed-off-by: Stefan Beller 
>> ---
>>
>>   This fixes one of the bugs mentioned in
>>   
>> https://public-inbox.org/git/cagz79kbkyupbjfvyx3hj_r5zw36+3ufonnlc-dpic40npja...@mail.gmail.com/T/#t
>>
>>   How to fix the other was not as obvious to me as I do not
>> understand the philosophy on verbosity in the transport code.
>
> I had a look and I would say just enclose all the fprintf() inside a:
>
> if (transport->verbose > 0)
>
> But then this is the first time I look at the code. I was about to send
> a patch too but it will conflict with this one :)

Well you can still send a patch :)

We have

int verbose = (transport->verbose > 0);
int quiet = (transport->verbose < 0);

So you're suggesting to only print these warnings when the
user asked for explicit verbose?

A few lines before the call to die_with_unpushed_submodules we have

 die ("Failed to push all needed submodules!");

which would also need a wrapping like

if (quiet)
return -1;
else
die(...);

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

Re: Extending "extended SHA1" syntax to traverse through gitlinks?

[Stefan Beller]

On Wed, Aug 24, 2016 at 7:20 AM, Josh Triplett  wrote:
> Depends on which cases you want to handle.  In the most general case,
> you'd need to find and process the applicable .gitmodules file, which
> would only work if you started from the top-level tree, not a random
> treeish.  On the other hand, in the most general case, you don't
> necessarily even have the module you need, because .git/modules only
> contains the modules the *current* version needed, not every past
> version.

The code in submodule-config.{c,h} allows exactly that:

submodule_from_path(commit_sha1, path)

returns information about the submodule recorded in a .gitmodules
file of a specific revision of the superproject.

Ideally the .git/modules contains all the modules that existed, ever.
Well "ideally" is the wrong word, but it is at least possible as the
submodules git dir is kept even when you remove the outdated
submodules working dir. (That's why the git dir is in the superprojects
git dir in the first place).

But as you say, it is possible of not having the submodule available.

>
> As an alternate approach (pun intended): treat every module in
> .git/modules as an alternate and just look up the object by hash.  Or,
> teach git-submodule to store all the objects for submodules in the
> supermodule's .git/objects (and teach git's reachability algorithm to
> respect refs in .git/modules, or store their refs in
> .git/refs/submodules/ or in a namespace).

This is a sensible thing to do no matter the outcome of this discussion.

>>  * recorded in ref / commit in supermodule
>>  * recorded in the index in supermodule
>>  - recorded in ref / commit in submodule
>>  - recorded in the index in submodule
>>  - state of worktree in submodule
>>
>> The last three can be easyly acessed by cd-ing to submodule.  The first
>> two are not easy to get, AFAIUC.
>
> Right.  I primarily care about those first two cases, especially the
> first one: given a commit containing a gitlink, how can I easily dig
> into the linked commit?

What do you exactly need? (What is digging here?)

See for example the series, that Jake Keller currently tries to land:
"submodule inline diff format"
https://public-inbox.org/git/20160822234344.22797-1-jacob.e.kel...@intel.com

That would enhance all the log/diff/show things.

Reading the original message, do you want to create patches in
the submodule from the superproject?

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

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Jeff King]

On Wed, Aug 24, 2016 at 09:09:06AM -0700, Junio C Hamano wrote:

> >  +  if (!path)
> >  +  path = obj_context.path;
> >  +  else if (obj_context.mode == S_IFINVALID)
> >  +  obj_context.mode = 0100644;
> >  +
> > buf = NULL;
> > switch (opt) {
> > case 't':
> 
> The above two hunks make all the difference in the ease of reading
> the remainder of the function.  Very good.

Yeah, I agree. Though it took me a moment to figure out why we were
setting obj_context.mode but not obj_context.path; the reason is that
"mode" is convenient to use as local storage, but "path" is not, because
it is not a pointer but an array.

So it would have been a little clearer to me as:

  const char *path;
  unsigned mode;
  ...
  if (!force_path) {
/* use file info from sha1 lookup */
path = obj_context.path;
mode = obj_context.mode;
  } else {
/* use path requested by user, and assume it is a regular file */
path = force_path;
mode = 0100644;
  }

but I don't know if that is even worth a re-roll.

> >  +test_expect_success 'path= complains without 
> > --textconv/--filters' '
> >  +  sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
> >  +  test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
> >  +  test ! -s actual &&
> >  +  grep "path.*needs.*filters" err
> >  +'
> 
> This will need to become test_i18ngrep once the error message is
> made translatable, but it is correct for now.  I personally think
> there is no need to check "actual" or "err", though---just running
> cat-file under test_must_fail should be sufficient.
> 
> Thanks, will queue.

The whole thing looks good to me, except for the weird doubled "--" in
the test description you quoted above. :)

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

Re: [PATCH] Documentation/git-patch-id: give working code example

[Junio C Hamano]

Michael J Gruber  writes:

> As it stands, the documentation gives the impression that
>
> git diff-tree  | git patch-id
>
> would be a working invocation of git patch-id, leaving the novice user
> in the dark.
>
> Make it explicit that 'git diff-tree -p' would be the command to use.

I take the lack of  as a strong hint that it is not meant
to be the full command line but merely there to make the command the
description talks about stand out from others (i.e. "This
description does not apply to 'diff-files' or 'diff-cache' output").

So I would think it is fine as-is.

Having said that, if we were to change something, I would rather
replace the mention of "diff-tree" with something like "git show" or
"git log -p", neither of which were even present in their current
form back when this piece of text was written.

>
> Signed-off-by: Michael J Gruber 
> ---
>  Documentation/git-patch-id.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt
> index cf71fba..67f8e28 100644
> --- a/Documentation/git-patch-id.txt
> +++ b/Documentation/git-patch-id.txt
> @@ -21,7 +21,7 @@ have the same "patch ID" are almost guaranteed to be the 
> same thing.
>  
>  IOW, you can use this thing to look for likely duplicate commits.
>  
> -When dealing with 'git diff-tree' output, it takes advantage of
> +When dealing with 'git diff-tree -p' output, it takes advantage of
>  the fact that the patch is prefixed with the object name of the
>  commit, and outputs two 40-byte hexadecimal strings.  The first
>  string is the patch ID, and the second string is the commit ID.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: diff using 3-dot behavior

[Junio C Hamano]

Robert Dailey  writes:

> $ git diff master...topic

... which is defined roughly as

git diff $(git merge-base master topic) topic

The merge-base is to compute the point you forked your topic from
the mainline.  So if you want to compare your current state in the
index with the fork point, you'd do

git diff --cached $(git merge-base master topic)

and comparing your working tree state would be

git diff --cached $(git merge-base master topic)

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

Re: [PATCH v2 0/4] cat-file: optionally convert to worktree version

[Junio C Hamano]

Johannes Schindelin  writes:

>  diff --git a/builtin/cat-file.c b/builtin/cat-file.c
>  index 5f91cf4..f8a3a08 100644
>  --- a/builtin/cat-file.c
>  +++ b/builtin/cat-file.c
>  @@ -61,6 +61,7 @@ static int cat_one_file(int opt, const char *exp_type, 
> const char *obj_name,
>   struct object_info oi = {NULL};
>   struct strbuf sb = STRBUF_INIT;
>   unsigned flags = LOOKUP_REPLACE_OBJECT;
>  +const char *path = force_path;
>   
>   if (unknown_type)
>   flags |= LOOKUP_UNKNOWN_OBJECT;
>  @@ -68,6 +69,11 @@ static int cat_one_file(int opt, const char *exp_type, 
> const char *obj_name,
>   if (get_sha1_with_context(obj_name, 0, sha1, _context))
>   die("Not a valid object name %s", obj_name);
>   
>  +if (!path)
>  +path = obj_context.path;
>  +else if (obj_context.mode == S_IFINVALID)
>  +obj_context.mode = 0100644;
>  +
>   buf = NULL;
>   switch (opt) {
>   case 't':

The above two hunks make all the difference in the ease of reading
the remainder of the function.  Very good.

>  +test_expect_success 'path= complains without 
> --textconv/--filters' '
>  +sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
>  +test_must_fail git cat-file --path=hello.txt blob $sha1 >actual 2>err &&
>  +test ! -s actual &&
>  +grep "path.*needs.*filters" err
>  +'

This will need to become test_i18ngrep once the error message is
made translatable, but it is correct for now.  I personally think
there is no need to check "actual" or "err", though---just running
cat-file under test_must_fail should be sufficient.

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

Re: Git for Windows documentation, was Re: [git-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

[Dakota Hawkins]

On Wed, Aug 24, 2016 at 11:41 AM, Johannes Schindelin
 wrote:
> Hi Dakota,
>
> On Tue, 23 Aug 2016, Dakota Hawkins wrote:
>
>> I use GFW almost exclusively, but I pretty much always consult the
>> upstream documentation anyway (because I find it easier).
>
> Oh... I thought that typing "git help git-commit" opening a nice HTML page
> in your favorite browser was good enough.
>
> Do you have any suggestion how to improve the user experience?

Just a small one, and that's that I'd prefer the option to have help
display in my terminal (that option might exist and I don't know how
to turn it on). That would be very convenient for me.

Opening a nice HTML page is probably nice for a lot of users. What
frustrates me about it is that I don't know which browser window on
which monitor (of 3) it's going to open in, so it's a context-switch
to a different window somewhere that I don't have much control over.

The thing I find easier about using the upstream documentation is just
that I can pick the browser window I want it to come up in, and it's
usually faster enough for me to just google "git-whatever" or
re-purpose an open doc tab. I don't prefer the _content_ of the
upstream documentation, it's just less frustrating for me to use, if
that makes sense.

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

Re: [PATCH 05/15] sequencer: lib'ify read_and_refresh_cache()

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -638,18 +638,21 @@ static int prepare_revs(struct replay_opts *opts)
> > -static void read_and_refresh_cache(struct replay_opts *opts)
> > +static int read_and_refresh_cache(struct replay_opts *opts)
> >  {
> > static struct lock_file index_lock;
> > int index_fd = hold_locked_index(_lock, 0);
> > if (read_index_preload(_index, NULL) < 0)
> > -   die(_("git %s: failed to read the index"), 
> > action_name(opts));
> > +   return error(_("git %s: failed to read the index"),
> > +   action_name(opts));
> > refresh_index(_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, 
> > NULL, NULL);
> > if (the_index.cache_changed && index_fd >= 0) {
> > if (write_locked_index(_index, _lock, 
> > COMMIT_LOCK))
> > -   die(_("git %s: failed to refresh the index"), 
> > action_name(opts));
> > +   return error(_("git %s: failed to refresh the 
> > index"),
> > +   action_name(opts));
> 
> Do these two error returns need to rollback the lockfile?

Here, too, the atexit() handler does the job, and again, this is not a
change in behavior.

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

Re: diff using 3-dot behavior

[Robert Dailey]

On Wed, Aug 24, 2016 at 11:00 AM, Michael J Gruber
 wrote:
> The 3-dot notation means:
>
> Show the difference between the merge-base of master and topic, and topic.
>
> I'm not completely sure, but I guess what you want is:
>
> Show the difference between the merge-base of master and topic, and the
> worktree.
>
> You can accomplish this with:
>
> git diff $(git merge-base master topic)
>
> I guess a shorter notation for that could come in handy. OTOH, I usually
> diff against HEAD in a situation like that.

Great solution. I feel silly now, the answer ended up being rather
obvious. I can create an alias for that. But I can't diff against HEAD
because then I am not seeing a diff of changes already committed.
Thanks for your help.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 11/15] sequencer: lib'ify save_todo()

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -929,24 +929,29 @@ static int sequencer_rollback(struct replay_opts 
> > *opts)
> > -static void save_todo(struct commit_list *todo_list, struct replay_opts 
> > *opts)
> > +static int save_todo(struct commit_list *todo_list, struct replay_opts 
> > *opts)
> >  {
> > static struct lock_file todo_lock;
> > struct strbuf buf = STRBUF_INIT;
> > int fd;
> >
> > -   fd = hold_lock_file_for_update(_lock, git_path_todo_file(), 
> > LOCK_DIE_ON_ERROR);
> > +   fd = hold_lock_file_for_update(_lock, git_path_todo_file(), 0);
> > +   if (fd < 0)
> > +   return error_errno(_("Could not lock '%s'"),
> > +  git_path_todo_file());
> > if (format_todo(, todo_list, opts) < 0)
> > -   die(_("Could not format %s."), git_path_todo_file());
> > +   return error(_("Could not format %s."), 
> > git_path_todo_file());
> 
> format_todo() doesn't seem to make any promises about the state of the
> strbuf upon error, so should this be releasing the strbuf before
> returning?

Yes, it should. Thank you!

> > if (write_in_full(fd, buf.buf, buf.len) < 0) {
> > strbuf_release();
> > -   die_errno(_("Could not write to %s"), git_path_todo_file());
> > +   return error_errno(_("Could not write to %s"),
> > +  git_path_todo_file());
> 
> Do the above two new error returns need to rollback the lockfile?

As before, atexit() handler to the rescue ;-)

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

Re: diff using 3-dot behavior

[Michael J Gruber]

Robert Dailey venit, vidit, dixit 24.08.2016 16:28:
> I want to view the complete diff of my branch (topic) relative to its
> parent branch (master). This should include cached/staged files and
> unstaged working tree changes.
> 
> If I do this:
> 
> $ git diff master
> 
> This will include changes on master *since* my last merge, which I do
> not want (I don't want to see changes on master, only on topic). I
> tried this:
> 
> $ git diff master --not master
> 
> This didn't give me any output. If I do this:
> 
> $ git diff master...topic
> 
> This shows me only committed changes on topic, but excludes staged &
> unstaged changes.
> 
> How can I get the results I want?

The 3-dot notation means:

Show the difference between the merge-base of master and topic, and topic.

I'm not completely sure, but I guess what you want is:

Show the difference between the merge-base of master and topic, and the
worktree.

You can accomplish this with:

git diff $(git merge-base master topic)

I guess a shorter notation for that could come in handy. OTOH, I usually
diff against HEAD in a situation like that.

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

Re: [PATCH 10/15] sequencer: lib'ify save_head()

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -844,18 +844,22 @@ static int create_seq_dir(void)
> > -static void save_head(const char *head)
> > +static int save_head(const char *head)
> >  {
> > static struct lock_file head_lock;
> > struct strbuf buf = STRBUF_INIT;
> > int fd;
> >
> > -   fd = hold_lock_file_for_update(_lock, git_path_head_file(), 
> > LOCK_DIE_ON_ERROR);
> > +   fd = hold_lock_file_for_update(_lock, git_path_head_file(), 0);
> > +   if (fd < 0)
> > +   return error_errno(_("Could not lock HEAD"));
> > strbuf_addf(, "%s\n", head);
> > if (write_in_full(fd, buf.buf, buf.len) < 0)
> > -   die_errno(_("Could not write to %s"), git_path_head_file());
> > +   return error_errno(_("Could not write to %s"),
> > +  git_path_head_file());
> 
> Does this need to rollback the lockfile before returning?

Again, this is handled by the atexit() handler, as before.

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

Re: [PATCH 09/15] sequencer: lib'ify create_seq_dir()

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -839,8 +839,8 @@ static int create_seq_dir(void)
> > return -1;
> > }
> > else if (mkdir(git_path_seq_dir(), 0777) < 0)
> > -   die_errno(_("Could not create sequencer directory %s"),
> > - git_path_seq_dir());
> > +   return error(_("Could not create sequencer directory %s 
> > (%s)"),
> > + git_path_seq_dir(), strerror(errno));
> 
> error_errno()?

Yep!

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

Re: [PATCH 06/15] sequencer: lib'ify read_populate_todo()

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:07 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -754,18 +754,21 @@ static void read_populate_todo(struct commit_list 
> > **todo_list,
> >
> > fd = open(git_path_todo_file(), O_RDONLY);
> > if (fd < 0)
> > -   die_errno(_("Could not open %s"), git_path_todo_file());
> > +   return error(_("Could not open %s (%s)"),
> > +   git_path_todo_file(), strerror(errno));
> 
> error_errno() perhaps?

Absolutely!

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

Re: [PATCH 02/15] sequencer: lib'ify do_recursive_merge()

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:06 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -303,7 +303,8 @@ static int do_recursive_merge(struct commit *base, 
> > struct commit *next,
> > if (active_cache_changed &&
> > write_locked_index(_index, _lock, COMMIT_LOCK))
> > /* TRANSLATORS: %s will be "revert" or "cherry-pick" */
> > -   die(_("%s: Unable to write new index file"), 
> > action_name(opts));
> > +   return error(_("%s: Unable to write new index file"),
> > +   action_name(opts));
> 
> Does this need to rollback the lockfile before returning?
> 
> A cursory scan of read-cache.c:do_write_locked_index() seems to
> indicate that lockfile disposition is not handled automatically in
> case of error (unless I'm misreading).

As mentioned elsewhere in this thread: an atexit() handler is tasked with
rolling back uncommitted lockfiles.

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

Re: [PATCH 01/15] sequencer: lib'ify write_message()

[Johannes Schindelin]

Hi Eric,

On Wed, 24 Aug 2016, Eric Sunshine wrote:

> On Tue, Aug 23, 2016 at 12:06 PM, Johannes Schindelin
>  wrote:
> > To be truly useful, the sequencer should never die() but always return
> > an error.
> >
> > Signed-off-by: Johannes Schindelin 
> > ---
> > diff --git a/sequencer.c b/sequencer.c
> > @@ -180,17 +180,20 @@ static void print_advice(int show_hint, struct 
> > replay_opts *opts)
> > -static void write_message(struct strbuf *msgbuf, const char *filename)
> > +static int write_message(struct strbuf *msgbuf, const char *filename)
> >  {
> > static struct lock_file msg_file;
> >
> > -   int msg_fd = hold_lock_file_for_update(_file, filename,
> > -  LOCK_DIE_ON_ERROR);
> > +   int msg_fd = hold_lock_file_for_update(_file, filename, 0);
> > +   if (msg_fd < 0)
> > +   return error_errno(_("Could not lock '%s'"), filename);
> > if (write_in_full(msg_fd, msgbuf->buf, msgbuf->len) < 0)
> > -   die_errno(_("Could not write to %s"), filename);
> > +   return error_errno(_("Could not write to %s"), filename);
> 
> Does this need to rollback the lockfile before returning[*]?
> 
> [*] I'm not terribly familiar with the lockfile mechanism and I don't
> have a lot of time to study it presently, so ignore me if this is a
> stupid question.

Not a stupid question at all.

The way lockfiles work is that there is an atexit() handler that ensures
that uncommitted lockfiles get rolled back automatically. So it happens to
work correctly right now, because all (direct and transitive) callers
simply stop doing things as quickly as possible. That means that no
subsequent attempt is made to write to the same file.

Technically, I agree with you that it may look a bit cleaner to roll back
at this point. However, this is outside the purview of this here patch
series, as it would actually change the behavior (the previous die() would
rely on the atexit() handler to roll back the locked file, too). So I
think if this is to be changed, it has to be in its own, separate patch
series.

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

Re: [PATCH 3/3] i18n: simplify numeric error reporting

[Junio C Hamano]

I'll squash this in to avoid compilation breakage.  We do not allow
decl-after-stmt.

Thanks.

 config.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index e33c703..0dfed68 100644
--- a/config.c
+++ b/config.c
@@ -652,10 +652,11 @@ int git_parse_ulong(const char *value, unsigned long *ret)
 NORETURN
 static void die_bad_number(const char *name, const char *value)
 {
+   const char * error_type = (errno == ERANGE)? _("out of 
range"):_("invalid unit");
+
if (!value)
value = "";
 
-   const char * error_type = (errno == ERANGE)? _("out of 
range"):_("invalid unit");
if (!(cf && cf->name))
die(_("bad numeric config value '%s' for '%s': %s"),
value, name, error_type);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

core.autocrlf, was Re: [git-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

[Johannes Schindelin]

Hi Junio,

On Tue, 23 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > The feature in question is also highly unlikely to be used as much by
> > non-Windows users as by Windows users due to the unfortunate choice of the
> > default setting for core.autocrlf.
> 
> My vague recollection from some years ago was that even among those
> who were active in msysGit development there were people who
> advocated for straight-thru and others who wanted core.autocrlf as
> the default, but I do not know the current state of the affairs.

I remember this very non-vaguely, for I was one of the people advocating
straight-through. I basically was shot down by people using Windows more
regularly than I did.

In any case, now is not the time to lament about this.

> In any case, in the ideal future, I would imagine that we would want
> to have "cat-file blob" to enable "--filters" by default; that would
> make cat-file and hash-objects a pair of symmetric operations.

I would advocate against that. It is not like the terms "hash-object" and
"cat-file" even *look* like they are opposites.

The main problem you face with making --filters the default is that it is
a possibly costly switch. Too costly in my opinion, just think of git-lfs.

> That certainly will not happen within 2.x timeframe, and the new
> "cat-file --filter" feature can appear in 2.11 at the earliest, but

s/--filter/--filters/

> I think by that time (or with a few more cycles) we may have a
> handful other improvements that are backward incompatible lined up
> to urge us to think about bumping the version number to 3.0.  I
> recall writing "Will keep in next to see if anybody screams" a few
> times already, and they are all good excuses to invite a version
> bump.
> 
> To prepare for that future, we would probably want to start updating
> in-tree scripts (including the tests) so that they call "cat-file
> --no-filter blob" whereever they currently say "cat-file blob" in or

s/--no-filter/--no-filters/

> soon after 2.11.  Of course, if some of them currently pipe
> "cat-file blob" output to munge it to produce what --filters would
> have done (I didn't check), we would want to rewrite them to use the
> new feature "cat-file --filter blob" when we do so.  In short, there

s/--filter/--filters/

> won't be any "cat-file blob" call that does not have either --filters
> or --no-filters, except the ones we write specifically to check the
> updated default behaviour when that happens.
> 
> Would that sound like a good longer-term plan?

Apart from my objecting against changing the default of cat-file, it sure
sounds like a good long-term plan ;-)

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

Re: [PATCH 1/3] i18n: fix typos for translation

[Junio C Hamano]

Here comes suggested replacement for 1/3 and 2/3.

Let's not correct "C" in "Cannot" at this stage and leave them to
the next cycle; there are too many of them.

-- >8 --
From: Jean-Noel Avila 
Date: Sun, 21 Aug 2016 16:50:37 +0200
Subject: [PATCH 1/3] i18n: fix typos for translation

Signed-off-by: Jean-Noel Avila 
Signed-off-by: Junio C Hamano 
---
 sequencer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sequencer.c b/sequencer.c
index 2e9c7d0..3804fa9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -702,7 +702,7 @@ static struct commit *parse_insn_line(char *bol, char *eol, 
struct replay_opts *
if (action != opts->action) {
if (action == REPLAY_REVERT)
  error((opts->action == REPLAY_REVERT)
-   ? _("Cannot revert during a another revert.")
+   ? _("Cannot revert during another revert.")
: _("Cannot revert during a cherry-pick."));
else
  error((opts->action == REPLAY_REVERT)
-- 
2.10.0-rc1-130-gf5df0f2

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

Re: [PATCH 1/3] i18n: fix typos for translation

[Junio C Hamano]

And here is the second one, using wording suggested by Jiang.

-- >8 -- 
From: Jean-Noel Avila 
Date: Sun, 21 Aug 2016 16:50:38 +0200
Subject: [PATCH] i18n: fix git rebase interactive commit messages

For proper i18n, the logic cannot embed english specific processing.

Signed-off-by: Jean-Noel Avila 
Signed-off-by: Junio C Hamano 
---
 git-rebase--interactive.sh | 45 +++--
 1 file changed, 3 insertions(+), 42 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index e2da524..7e558b0 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -404,51 +404,12 @@ pick_one_preserving_merges () {
 
 this_nth_commit_message () {
n=$1
-   case "$n" in
-   1) gettext "This is the 1st commit message:";;
-   2) gettext "This is the 2nd commit message:";;
-   3) gettext "This is the 3rd commit message:";;
-   4) gettext "This is the 4th commit message:";;
-   5) gettext "This is the 5th commit message:";;
-   6) gettext "This is the 6th commit message:";;
-   7) gettext "This is the 7th commit message:";;
-   8) gettext "This is the 8th commit message:";;
-   9) gettext "This is the 9th commit message:";;
-   10) gettext "This is the 10th commit message:";;
-   # TRANSLATORS: if the language you are translating into
-   # doesn't allow you to compose a sentence in this fashion,
-   # consider translating as if this and the following few strings
-   # were "This is the commit message ${n}:"
-   *1[0-9]|*[04-9]) eval_gettext "This is the \${n}th commit message:";;
-   *1) eval_gettext "This is the \${n}st commit message:";;
-   *2) eval_gettext "This is the \${n}nd commit message:";;
-   *3) eval_gettext "This is the \${n}rd commit message:";;
-   *) eval_gettext "This is the commit message \${n}:";;
-   esac
+   eval_gettext "This is the commit message #\${n}:"
 }
+
 skip_nth_commit_message () {
n=$1
-   case "$n" in
-   1) gettext "The 1st commit message will be skipped:";;
-   2) gettext "The 2nd commit message will be skipped:";;
-   3) gettext "The 3rd commit message will be skipped:";;
-   4) gettext "The 4th commit message will be skipped:";;
-   5) gettext "The 5th commit message will be skipped:";;
-   6) gettext "The 6th commit message will be skipped:";;
-   7) gettext "The 7th commit message will be skipped:";;
-   8) gettext "The 8th commit message will be skipped:";;
-   9) gettext "The 9th commit message will be skipped:";;
-   10) gettext "The 10th commit message will be skipped:";;
-   # TRANSLATORS: if the language you are translating into
-   # doesn't allow you to compose a sentence in this fashion,
-   # consider translating as if this and the following few strings
-   # were "The commit message ${n} will be skipped:"
-   *1[0-9]|*[04-9]) eval_gettext "The \${n}th commit message will be 
skipped:";;
-   *1) eval_gettext "The \${n}st commit message will be skipped:";;
-   *2) eval_gettext "The \${n}nd commit message will be skipped:";;
-   *3) eval_gettext "The \${n}rd commit message will be skipped:";;
-   *) eval_gettext "The commit message \${n} will be skipped:";;
-   esac
+   eval_gettext "The commit message #\${n} will be skipped:"
 }
 
 update_squash_messages () {
-- 
2.10.0-rc1-130-gf5df0f2

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

Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Johannes Schindelin]

Hi Arif,

On Tue, 23 Aug 2016, Arif Khokar wrote:

> On 08/20/2016 03:57 PM, Jakub Narębski wrote:
> 
> > But perhaps the problem is current lack of tooling in the opposite
> > direction, namely getting patches from mailing list and applying them
> > to GitHub repo, or Bitbucket, or GitLab.  Though with working Git, it
> > is something easier than sending patches via email; it is enough that
> > email client can save email to a file (or better, whole sub-thread to
> > file or files).
> 
> Given that public-inbox provides an NNTP interface, couldn't the ARTICLE
>  NNTP command be used to easily retrieve the messages in a
> given patch series (at least compared to POP or IMAP).  Perhaps
> git-send-email could be modified to include the message-id value of each
> patch in the series that it sends to the mailing list and include it in
> the cover letter.

I am no expert in the NNTP protocol (I abandoned News long ago), but if
you go from HTML, you can automate the process without requiring changes
in format-patch.

> Then a script could be written (i.e., git-download-patch) which could
> parse the cover letter message (specified using its message-id), and
> download all the patches in series, which can then be applied using
> git-am.  This would in fact take the email client out of the equation in
> terms of saving patches.

I recently adapted an old script I had to apply an entire patch series
given the GMane link to its cover letter:

https://github.com/git-for-windows/build-extra/blob/master/apply-from-gmane.sh

Maybe you find it in you to adapt that to work with public-inbox.org?

Ciao,
Johannes

Git for Windows documentation, was Re: [git-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

[Johannes Schindelin]

Hi Dakota,

On Tue, 23 Aug 2016, Dakota Hawkins wrote:

> I use GFW almost exclusively, but I pretty much always consult the
> upstream documentation anyway (because I find it easier).

Oh... I thought that typing "git help git-commit" opening a nice HTML page
in your favorite browser was good enough.

Do you have any suggestion how to improve the user experience?

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

Re: [git-for-windows] Re: [ANNOUNCE] Git for Windows 2.9.3

[Johannes Schindelin]

Hi Junio,

On Tue, 23 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > In case it is not crystal-clear, let me clarify one very important point.
> > It seems that some people mistake the work I do for something I do on a
> > whim. This is not so.
> >
> > The patch series that triggered this entire unfortunate discussion
> > introduced the --smudge option, which I have subsequently renamed to
> > --filters and submitted as a patch series to the Git project.
> 
> As the "--filters" is meant as a new feature, it will not land on
> the maintenance track.  It is very likely that it won't be in 2.10,
> so it won't appear in 2.10.x maintenance track, either.

Right. Which is even more a reason for me to decouple this feature from
non-Windows Git.

> [...] whatever new feature you unleash ahead of upstream to your Windows
> port has cost to your end-users.  Its implementation or its external
> interface may have to change when you upstream the new feature that has
> already been in the field, and your end-users would have to adjust their
> scripts and/or muscle memory.

This is nothing new. As I said earlier, I have plenty of experience with
this. Including the experience of worsimproving a feature that has been
battle-tested for years, only to be broken in the process to appease
reviewers on the Git mailing list.

I talk about the core.hideDotFiles feature, of course, which in the
process of being integrated into core Git lost its ability to respect the
setting to be "false".

Git for Windows has a work-around already, of course, it's just ugly, so I
am hesitating to "upstream it" yet.

As I said. All of this is old hat. Git for Windows has been, and probably
will be for a long time to come, diverging from upstream Git. This is not
something I wanted, or worked toward. It's just reality that happened and
I have to deal with it and there is nothing to see here, please move on.

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

[PATCH] diff: mark a file-local symbol static

[Ramsay Jones]

Signed-off-by: Ramsay Jones 
---

Hi Michael,

If you need to re-roll your 'mh/diff-indent-heuristic' branch, could
you please squash this into the relevant patch (commit 8f5cc189,
"diff: improve positioning of add/delete blocks in diffs", 22-08-2016).

Thanks!

ATB,
Ramsay Jones

 xdiff/xdiffi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index dc79628..67c1ccc 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -684,7 +684,7 @@ static void score_add_split(const struct split_measurement 
*m, struct split_scor
}
 }
 
-int score_cmp(struct split_score *s1, struct split_score *s2)
+static int score_cmp(struct split_score *s1, struct split_score *s2)
 {
/* -1 if s1.effective_indent < s2->effective_indent, etc. */
int cmp_indents = ((s1->effective_indent > s2->effective_indent) -
-- 
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Extending "extended SHA1" syntax to traverse through gitlinks?

[Josh Triplett]

On Wed, Aug 24, 2016 at 03:16:56PM +0200, Jakub Narębski wrote:
> W dniu 24.08.2016 o 07:36, Junio C Hamano pisze:
> > Jakub Narębski  writes:
> > 
> >> The point is that submodule has it's own object database.  It might
> >> be the same as superproject's, but you need to handle submodule objects
> >> being in separate submodule repository anyway.  Common repository is
> >> just a special case.
> >>
> >> By the way, this also means that proposed "extended extended SHA1"
> >> syntax would be useful to user's of submodules...
> > 
> > Not really.
> > 
> > I think that you gave a prime example why ://
> > is not a useful thing for submodules.  When the syntax resolves to a
> > 40-hex object name, that object name by itself is not useful.
> > 
> > You also need to carry an additional piece of information that lets
> > you identify the location of the repository, in which the object
> > name is valid, in the current user's context (i.e. somewhere in the
> > superproject where the submodule lives).  In other words, you'd need
> > to carry : around anyway for the object name to be
> > useful, so there is no good reason why anybody should insist that
> > the plumbing level resolve :// directly to an
> > object name in the first place.
> 
> Not really.
> 
> The above means only that the support for new syntax would be not
> as easy as adding it to 'git rev-parse' (and it's built-in equivalent),
> except for the case where submodule uses the same object database as
> supermodule.
> 
> So it wouldn't be as easy (on conceptual level) as adding support
> for ':/' or '^{/}'.  It would be at least as
> hard, if not harder, as adding support for '@{-1}' and its '-'
> shortcut.

Depends on which cases you want to handle.  In the most general case,
you'd need to find and process the applicable .gitmodules file, which
would only work if you started from the top-level tree, not a random
treeish.  On the other hand, in the most general case, you don't
necessarily even have the module you need, because .git/modules only
contains the modules the *current* version needed, not every past
version.

As an alternate approach (pun intended): treat every module in
.git/modules as an alternate and just look up the object by hash.  Or,
teach git-submodule to store all the objects for submodules in the
supermodule's .git/objects (and teach git's reachability algorithm to
respect refs in .git/modules, or store their refs in
.git/refs/submodules/ or in a namespace).

> Josh, what was the reason behind proposing this feature? Was it
> conceived as adding completeness to gitrevisions syntax, a low-hanging
> fruit?  It isn't (the latter).  Or was it some problem with submodule
> handling that you would want to use this syntax for?

This wasn't an abstract/theoretical completeness issue.  I specifically
wanted this syntax for practical use with actual trees containing
gitlinks, motivated by having a tool that creates and uses such
gitlinks. :)

> As for usefulness: this fills the hole in accessing submodules, one
> that could be handled by combining plumbing-level commands.  Namely,
> there are 5 states of submodule (as I understand it)
> 
>  * recorded in ref / commit in supermodule
>  * recorded in the index in supermodule
>  - recorded in ref / commit in submodule
>  - recorded in the index in submodule
>  - state of worktree in submodule
> 
> The last three can be easyly acessed by cd-ing to submodule.  The first
> two are not easy to get, AFAIUC.

Right.  I primarily care about those first two cases, especially the
first one: given a commit containing a gitlink, how can I easily dig
into the linked commit?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Getting the "message" given a branch designation and conversely

[Jakub Narębski]

W dniu 15.08.2016 o 14:28, Jeff King pisze:
> On Sun, Aug 14, 2016 at 07:58:14AM -0700, n...@dad.org wrote:
> 
>> I am learning how to use git. I would like to know how:
>>
>> Given a branch's designation, such as "master~4", how can I see the message I
>> furnished when I created the branch using "git commit"?
> 
> Somebody already pointed you at "git log", which is the right tool for
> looking at commit messages (or perhaps "git show" if you only want to
> see a single entry).

I think you would want "git log -1 master~4" or "git show master~4" to
see the commit message of a single commit (without diff).

>> Conversely, given the message I furnished to "git commit", when I created a
>> branch, how can I see the branch's designation?
> 
> Try "git log --grep=some.regex" to find a particular commit. Usually we
> refer to commits by their sha1 id, which will be shown by git-log.

There is also :/ and ^{/} syntax, if you want 
composability (see gitrevisions(7)).

> However, you can use git-describe to generate a name for any commit that
> is based on traversing from a tag. Try:
> 
>   git describe --contains --all 
> 
> for example. Using "--all" tells git to consider names based on branches
> as well as tags. Using "--contains" will generate a name based on
> traversing backwards from the tags and branches (like "master~4") rather
> than basing the name on a tag that you build off of.

The "git describe --contains" is interface to "git name-rev" plumbing

-- 
Jakub Narębski

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

diff using 3-dot behavior

[Robert Dailey]

I want to view the complete diff of my branch (topic) relative to its
parent branch (master). This should include cached/staged files and
unstaged working tree changes.

If I do this:

$ git diff master

This will include changes on master *since* my last merge, which I do
not want (I don't want to see changes on master, only on topic). I
tried this:

$ git diff master --not master

This didn't give me any output. If I do this:

$ git diff master...topic

This shows me only committed changes on topic, but excludes staged &
unstaged changes.

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

Re: [PATCH v4] config: add conditional include

[Jeff King]

On Wed, Aug 24, 2016 at 02:44:29PM +0200, Jakub Narębski wrote:

> W dniu 24.08.2016 o 11:37, Duy Nguyen pisze:
> 
> > It works for me either way. So I'm going to assume we want
> > "[include-if "gitdir-is:..."]", unless people think it needs more
> > discussion (then just write something, anything, so I can put it back
> > in my backlog)
> 
> A final note: [include "if-gitdir:..."] is inclusive by default
> (I think), that is old Git would include it unconditionally,
> while [include-if "gitdir-is:..."] is exclusive by default, that
> is old Git would ignore it.
> 
> But I might be mistaken.

I don't think this is the case. Conditionals were considered when
the include feature was added, and anything except "include.path" is
explicitly ignored. So either syntax will behave the same.

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

Re: Extending "extended SHA1" syntax to traverse through gitlinks?

[Jakub Narębski]

W dniu 24.08.2016 o 07:36, Junio C Hamano pisze:
> Jakub Narębski  writes:
> 
>> The point is that submodule has it's own object database.  It might
>> be the same as superproject's, but you need to handle submodule objects
>> being in separate submodule repository anyway.  Common repository is
>> just a special case.
>>
>> By the way, this also means that proposed "extended extended SHA1"
>> syntax would be useful to user's of submodules...
> 
> Not really.
> 
> I think that you gave a prime example why ://
> is not a useful thing for submodules.  When the syntax resolves to a
> 40-hex object name, that object name by itself is not useful.
> 
> You also need to carry an additional piece of information that lets
> you identify the location of the repository, in which the object
> name is valid, in the current user's context (i.e. somewhere in the
> superproject where the submodule lives).  In other words, you'd need
> to carry : around anyway for the object name to be
> useful, so there is no good reason why anybody should insist that
> the plumbing level resolve :// directly to an
> object name in the first place.

Not really.

The above means only that the support for new syntax would be not
as easy as adding it to 'git rev-parse' (and it's built-in equivalent),
except for the case where submodule uses the same object database as
supermodule.

So it wouldn't be as easy (on conceptual level) as adding support
for ':/' or '^{/}'.  It would be at least as
hard, if not harder, as adding support for '@{-1}' and its '-'
shortcut.


Josh, what was the reason behind proposing this feature? Was it
conceived as adding completeness to gitrevisions syntax, a low-hanging
fruit?  It isn't (the latter).  Or was it some problem with submodule
handling that you would want to use this syntax for?

As for usefulness: this fills the hole in accessing submodules, one
that could be handled by combining plumbing-level commands.  Namely,
there are 5 states of submodule (as I understand it)

 * recorded in ref / commit in supermodule
 * recorded in the index in supermodule
 - recorded in ref / commit in submodule
 - recorded in the index in submodule
 - state of worktree in submodule

The last three can be easyly acessed by cd-ing to submodule.  The first
two are not easy to get, AFAIUC.

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

Re: Working with public-inbox.org [Was: [PATCH] rev-parse: respect core.hooksPath in --git-path]

[Johannes Schindelin]

Hi Philip,

On Mon, 22 Aug 2016, Philip Oakley wrote:

> From: "Duy Nguyen" 
> > On Mon, Aug 22, 2016 at 8:06 PM, Johannes Schindelin
> >  wrote:
> > > My point stands. We are way more uninviting to contributors than
> > > necessary. And a huge part of the problem is that we require contributors
> > > to send their patches inlined into whitespace-preserving mails.
> >
> > We probably can settle this in the next git survey with a new
> > question: what's stopping you from contributing to git?
> > -- 
> One has to be careful that some of this is preaching to the choir.

Exactly.

> I do note that dscho's patches now have the extra footer (below the three
> dashes) e.g.
> 
> Published-As: https://github.com/dscho/git/releases/tag/cat-file-filters-v1
> Fetch-It-Via: git fetch https://github.com/dscho/git cat-file-filters-v1
> 
> If say I used that, and sent my patch series via Outlook Express (),
> with it's white space damage, would those footers help once the content has
> been reviewed (rather than white spacing style) in the applying the patch?

I considered recommending this as some way to improve the review process.
The problem, of course, is that it is very easy to craft an email with an
innocuous patch and then push some malicious patch to the linked
repository.

Now, with somebody like me who would lose a lot when destroying trust, it
is highly unlikely. But it is possible that in between the hundreds of
sincere contributors a bad apple tries to sneak in bad stuff.

Therefore, if we were to support a Git-driven contribution process that
*also* sends mail, that mail needs to be generated by a trusted source, to
ensure that the content of the mail is identical to the original Git
commits.

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

[PATCH] Documentation/git-patch-id: give working code example

[Michael J Gruber]

As it stands, the documentation gives the impression that

git diff-tree  | git patch-id

would be a working invocation of git patch-id, leaving the novice user
in the dark.

Make it explicit that 'git diff-tree -p' would be the command to use.

Signed-off-by: Michael J Gruber 
---
 Documentation/git-patch-id.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-patch-id.txt b/Documentation/git-patch-id.txt
index cf71fba..67f8e28 100644
--- a/Documentation/git-patch-id.txt
+++ b/Documentation/git-patch-id.txt
@@ -21,7 +21,7 @@ have the same "patch ID" are almost guaranteed to be the same 
thing.
 
 IOW, you can use this thing to look for likely duplicate commits.
 
-When dealing with 'git diff-tree' output, it takes advantage of
+When dealing with 'git diff-tree -p' output, it takes advantage of
 the fact that the patch is prefixed with the object name of the
 commit, and outputs two 40-byte hexadecimal strings.  The first
 string is the patch ID, and the second string is the commit ID.
-- 
2.10.0.rc0.268.ge3ba28a

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

Re: [PATCH v4] config: add conditional include

[Jakub Narębski]

W dniu 24.08.2016 o 11:37, Duy Nguyen pisze:

> It works for me either way. So I'm going to assume we want
> "[include-if "gitdir-is:..."]", unless people think it needs more
> discussion (then just write something, anything, so I can put it back
> in my backlog)

A final note: [include "if-gitdir:..."] is inclusive by default
(I think), that is old Git would include it unconditionally,
while [include-if "gitdir-is:..."] is exclusive by default, that
is old Git would ignore it.

But I might be mistaken.

P.S. I personally prefer [include-if ...]
-- 
Jakub Narębski

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

[PATCH v2 2/4] cat-file: introduce the --filters option

[Johannes Schindelin]

The --filters option applies the convert_to_working_tree() filter for
the path when showing the contents of a regular file blob object.

This feature comes in handy when a 3rd-party tool wants to work with
the contents of files from past revisions as if they had been checked
out, but without detouring via temporary files.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt | 12 +---
 builtin/cat-file.c | 41 -
 t/t8010-cat-file-filters.sh| 34 ++
 3 files changed, 83 insertions(+), 4 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 071029b..537d02c 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,15 +9,15 @@ git-cat-file - Provide content or type and size information 
for repository objec
 SYNOPSIS
 
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv ) 
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) 
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
 ---
 In its first form, the command provides the content or the type of an object in
 the repository. The type is required unless `-t` or `-p` is used to find the
-object type, or `-s` is used to find the object size, or `--textconv` is used
-(which implies type "blob").
+object type, or `-s` is used to find the object size, or `--textconv` or
+`--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
 stdin, and the SHA-1, type, and size of each object is printed on stdout.
@@ -58,6 +58,12 @@ OPTIONS
order to apply the filter to the content recorded in the index at
.
 
+--filters::
+   Show the content as converted by the filters configured in
+   the current working tree for the given  (i.e. smudge filters,
+   end-of-line conversion, etc). In this case,  has to be of
+   the form :, or :.
+
 --batch::
 --batch=::
Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2dfe626..0b74afa 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,33 @@ struct batch_options {
const char *format;
 };
 
+static int filter_object(const char *path, unsigned mode,
+const unsigned char *sha1,
+char **buf, unsigned long *size)
+{
+   enum object_type type;
+
+   *buf = read_sha1_file(sha1, , size);
+   if (!*buf)
+   return error(_("cannot read object %s '%s'"),
+   sha1_to_hex(sha1), path);
+   if (type != OBJ_BLOB) {
+   free(*buf);
+   return error(_("blob expected for %s '%s'"),
+   sha1_to_hex(sha1), path);
+   }
+   if (S_ISREG(mode)) {
+   struct strbuf strbuf = STRBUF_INIT;
+   if (convert_to_working_tree(path, *buf, *size, )) {
+   free(*buf);
+   *size = strbuf.len;
+   *buf = strbuf_detach(, NULL);
+   }
+   }
+
+   return 0;
+}
+
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
int unknown_type)
 {
@@ -61,6 +88,16 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
case 'e':
return !has_sha1_file(sha1);
 
+   case 'w':
+   if (!obj_context.path[0])
+   die("git cat-file --filters %s:  must be "
+   "", obj_name);
+
+   if (filter_object(obj_context.path, obj_context.mode,
+ sha1, , ))
+   return -1;
+   break;
+
case 'c':
if (!obj_context.path[0])
die("git cat-file --textconv %s:  must be 
",
@@ -440,7 +477,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv) "),
+   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv|--filters) "),
N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
NULL
 };
@@ -486,6 +523,8 @@ int cmd_cat_file(int argc, const char **argv, const char 
*prefix)
OPT_CMDMODE('p', NULL, , N_("pretty-print object's 
content"), 'p'),
OPT_CMDMODE(0, "textconv", ,
N_("for blob objects, run textconv on object's 
content"), 'c'),
+   OPT_CMDMODE(0, "filters", ,
+   N_("for blob 

[PATCH v2 3/4] cat-file --textconv/--filters: allow specifying the path separately

[Johannes Schindelin]

There are circumstances when it is relatively easy to figure out the
object name for a given path, but not the name of the containing tree.
For example, when looking at a diff generated by Git, the object names
are recorded, but not the revision. As a matter of fact, the revisions
from which the diff was generated may not even exist locally.

In such a case, the user would have to generate a fake revision just to
be able to use --textconv or --filters.

Let's simplify this dramatically, because we do not really need that
revision at all: all we care about is that we know the path. In the
scenario described above, we do know the path, and we just want to
specify it separately from the object name.

Example usage:

git cat-file --textconv --path=main.c 0f1937fd

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt |  7 ++-
 builtin/cat-file.c | 26 +-
 t/t8010-cat-file-filters.sh| 20 
 3 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 537d02c..4fa9041 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information for 
repository objec
 SYNOPSIS
 
 [verse]
-'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) 
+'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) [--path=] 
 'git cat-file' (--batch | --batch-check) [--follow-symlinks]
 
 DESCRIPTION
@@ -64,6 +64,11 @@ OPTIONS
end-of-line conversion, etc). In this case,  has to be of
the form :, or :.
 
+--path=::
+   For use with --textconv or --filters, to allow specifying an object
+   name and a path separately, e.g. when it is difficult to figure out
+   the revision from which the blob came.
+
 --batch::
 --batch=::
Print object information and contents for each object provided
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 0b74afa..2c799ac 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -20,6 +20,8 @@ struct batch_options {
const char *format;
 };
 
+static const char *force_path;
+
 static int filter_object(const char *path, unsigned mode,
 const unsigned char *sha1,
 char **buf, unsigned long *size)
@@ -58,6 +60,7 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
struct object_info oi = {NULL};
struct strbuf sb = STRBUF_INIT;
unsigned flags = LOOKUP_REPLACE_OBJECT;
+   const char *path = force_path;
 
if (unknown_type)
flags |= LOOKUP_UNKNOWN_OBJECT;
@@ -65,6 +68,11 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
if (get_sha1_with_context(obj_name, 0, sha1, _context))
die("Not a valid object name %s", obj_name);
 
+   if (!path)
+   path = obj_context.path;
+   else if (obj_context.mode == S_IFINVALID)
+   obj_context.mode = 0100644;
+
buf = NULL;
switch (opt) {
case 't':
@@ -89,21 +97,22 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
return !has_sha1_file(sha1);
 
case 'w':
-   if (!obj_context.path[0])
+   if (!path[0])
die("git cat-file --filters %s:  must be "
"", obj_name);
 
-   if (filter_object(obj_context.path, obj_context.mode,
+   if (filter_object(path, obj_context.mode,
  sha1, , ))
return -1;
break;
 
case 'c':
-   if (!obj_context.path[0])
+   if (!path[0])
die("git cat-file --textconv %s:  must be 
",
obj_name);
 
-   if (textconv_object(obj_context.path, obj_context.mode, sha1, 
1, , ))
+   if (textconv_object(path, obj_context.mode,
+   sha1, 1, , ))
break;
 
case 'p':
@@ -477,7 +486,7 @@ static int batch_objects(struct batch_options *opt)
 }
 
 static const char * const cat_file_usage[] = {
-   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv|--filters) "),
+   N_("git cat-file (-t [--allow-unknown-type]|-s 
[--allow-unknown-type]|-e|-p||--textconv|--filters) [--path=] 
"),
N_("git cat-file (--batch | --batch-check) [--follow-symlinks]"),
NULL
 };
@@ -525,6 +534,8 @@ int cmd_cat_file(int argc, const char **argv, const char 
*prefix)
N_("for blob objects, run textconv on object's 
content"), 'c'),
OPT_CMDMODE(0, 

[PATCH v2 0/4] cat-file: optionally convert to worktree version

[Johannes Schindelin]

When third-party tools need to access to contents of blobs in the
database, they might be more interested in the worktree version than in
the "clean" version of said contents.

This branch introduces the --filters option to make that happen, the
--use-path option to provide the path separately if the blob name rather
than the tree name is availale, and offers batch support in which case
it expects the object names and the path on its input lines, separated
by white space.

The new --filters option is an obvious sibling of --textconv, and shares
the peculiar feature that the drivers (and end-of-line convention) are
determined from the current worktree, not from the attributes stored in
the revision that may have been part of the object name.

As --textconv is so similar to --filters, it was taught to understand
the --use-path option and it was made compatible with batch mode, too.

I briefly considered teaching the batch mode to extract the path from
object names if they are specified as :. The changes
would be quite intrusive, though, and uglify the code substanitially. So
I decided against that.

Changes vs v2:

- two commit messages were touched up

- --use-path= was renamed to --path=

- a test was added to verify that --path without --textconv/--filters
  errors out with helpful advice


Johannes Schindelin (4):
  cat-file: fix a grammo in the man page
  cat-file: introduce the --filters option
  cat-file --textconv/--filters: allow specifying the path separately
  cat-file: support --textconv/--filters in batch mode

 Documentation/git-cat-file.txt |  40 +++
 builtin/cat-file.c | 110 ++---
 t/t8010-cat-file-filters.sh|  64 
 3 files changed, 196 insertions(+), 18 deletions(-)
 create mode 100755 t/t8010-cat-file-filters.sh

Published-As: https://github.com/dscho/git/releases/tag/cat-file-filters-v2
Fetch-It-Via: git fetch https://github.com/dscho/git cat-file-filters-v2

Interdiff vs v1:

 diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
 index 1f4d954..204541c 100644
 --- a/Documentation/git-cat-file.txt
 +++ b/Documentation/git-cat-file.txt
 @@ -9,7 +9,7 @@ git-cat-file - Provide content or type and size information 
for repository objec
  SYNOPSIS
  
  [verse]
 -'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) [--use-path=] 
 +'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) [--path=] 
  'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] 
[--follow-symlinks]
  
  DESCRIPTION
 @@ -63,12 +63,12 @@ OPTIONS
.
  
  --filters::
 -  Show the content as transformed by the filters configured in
 +  Show the content as converted by the filters configured in
the current working tree for the given  (i.e. smudge filters,
end-of-line conversion, etc). In this case,  has to be of
the form :, or :.
  
 ---use-path=::
 +--path=::
For use with --textconv or --filters, to allow specifying an object
name and a path separately, e.g. when it is difficult to figure out
the revision from which the blob came.
 diff --git a/builtin/cat-file.c b/builtin/cat-file.c
 index 5f91cf4..f8a3a08 100644
 --- a/builtin/cat-file.c
 +++ b/builtin/cat-file.c
 @@ -61,6 +61,7 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
struct object_info oi = {NULL};
struct strbuf sb = STRBUF_INIT;
unsigned flags = LOOKUP_REPLACE_OBJECT;
 +  const char *path = force_path;
  
if (unknown_type)
flags |= LOOKUP_UNKNOWN_OBJECT;
 @@ -68,6 +69,11 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
if (get_sha1_with_context(obj_name, 0, sha1, _context))
die("Not a valid object name %s", obj_name);
  
 +  if (!path)
 +  path = obj_context.path;
 +  else if (obj_context.mode == S_IFINVALID)
 +  obj_context.mode = 0100644;
 +
buf = NULL;
switch (opt) {
case 't':
 @@ -92,23 +98,21 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
return !has_sha1_file(sha1);
  
case 'w':
 -  if (!force_path && !obj_context.path[0])
 +  if (!path[0])
die("git cat-file --filters %s:  must be "
"", obj_name);
  
 -  if (filter_object(force_path ? force_path : obj_context.path,
 -force_path ? 0100644 : obj_context.mode,
 +  if (filter_object(path, obj_context.mode,
  sha1, , ))
return -1;
break;
  
case 'c':
 -  if (!force_path && !obj_context.path[0])
 +  if (!path[0])
   

[PATCH v2 1/4] cat-file: fix a grammo in the man page

[Johannes Schindelin]

"... has be ..." -> "... has to be ..."

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 18d03d8..071029b 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -54,8 +54,9 @@ OPTIONS
 
 --textconv::
Show the content as transformed by a textconv filter. In this case,
-has be of the form :, or : in order
-   to apply the filter to the content recorded in the index at .
+has to be of the form :, or : in
+   order to apply the filter to the content recorded in the index at
+   .
 
 --batch::
 --batch=::
-- 
2.10.0.rc1.99.gcd66998


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

[PATCH v2 4/4] cat-file: support --textconv/--filters in batch mode

[Johannes Schindelin]

With this patch, --batch can be combined with --textconv or --filters.
For this to work, the input needs to have the form



so that the filters can be chosen appropriately.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-cat-file.txt | 18 +++-
 builtin/cat-file.c | 49 +-
 t/t8010-cat-file-filters.sh| 10 +
 3 files changed, 67 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 4fa9041..204541c 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git cat-file' (-t [--allow-unknown-type]| -s [--allow-unknown-type]| -e | -p 
|  | --textconv | --filters ) [--path=] 
-'git cat-file' (--batch | --batch-check) [--follow-symlinks]
+'git cat-file' (--batch | --batch-check) [ --textconv | --filters ] 
[--follow-symlinks]
 
 DESCRIPTION
 ---
@@ -20,7 +20,11 @@ object type, or `-s` is used to find the object size, or 
`--textconv` or
 `--filters` is used (which imply type "blob").
 
 In the second form, a list of objects (separated by linefeeds) is provided on
-stdin, and the SHA-1, type, and size of each object is printed on stdout.
+stdin, and the SHA-1, type, and size of each object is printed on stdout. The
+output format can be overridden using the optional `` argument. If
+either `--textconv` or `--filters` was specified, the input is expected to
+list the object names followed by the path name, separated by a single white
+space, so that the appropriate drivers can be determined.
 
 OPTIONS
 ---
@@ -72,13 +76,17 @@ OPTIONS
 --batch::
 --batch=::
Print object information and contents for each object provided
-   on stdin.  May not be combined with any other options or arguments.
-   See the section `BATCH OUTPUT` below for details.
+   on stdin.  May not be combined with any other options or arguments
+   except `--textconv` or `--filters`, in which case the input lines
+   also need to specify the path, separated by white space.  See the
+   section `BATCH OUTPUT` below for details.
 
 --batch-check::
 --batch-check=::
Print object information for each object provided on stdin.  May
-   not be combined with any other options or arguments.  See the
+   not be combined with any other options or arguments except
+   `--textconv` or `--filters`, in which case the input lines also
+   need to specify the path, separated by white space.  See the
section `BATCH OUTPUT` below for details.
 
 --batch-all-objects::
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2c799ac..f8a3a08 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -17,6 +17,7 @@ struct batch_options {
int print_contents;
int buffer_output;
int all_objects;
+   int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
const char *format;
 };
 
@@ -285,7 +286,32 @@ static void print_object_or_die(struct batch_options *opt, 
struct expand_data *d
if (data->type == OBJ_BLOB) {
if (opt->buffer_output)
fflush(stdout);
-   if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
+   if (opt->cmdmode) {
+   char *contents;
+   unsigned long size;
+
+   if (!data->rest)
+   die("missing path for '%s'", sha1_to_hex(sha1));
+
+   if (opt->cmdmode == 'w') {
+   if (filter_object(data->rest, 0100644, sha1,
+ , ))
+   die("could not convert '%s' %s",
+   sha1_to_hex(sha1), data->rest);
+   } else if (opt->cmdmode == 'c') {
+   enum object_type type;
+   if (!textconv_object(data->rest, 0100644, sha1,
+1, , ))
+   contents = read_sha1_file(sha1, ,
+ );
+   if (!contents)
+   die("could not convert '%s' %s",
+   sha1_to_hex(sha1), data->rest);
+   } else
+   die("BUG: invalid cmdmode: %c", opt->cmdmode);
+   batch_write(opt, contents, size);
+   free(contents);
+   } else if (stream_blob_to_fd(1, sha1, NULL, 0) < 0)
die("unable to stream %s to stdout", sha1_to_hex(sha1));
}
else {
@@ -422,6 +448,8 @@ static int batch_objects(struct batch_options *opt)
data.mark_query = 1;

Company Payment Agent Needed

[Shougang Group of company]

Shougang Group of company
45 Huagong Road Xinji City,
Hebei Province China.
webpage: www.shougang.com.cn


This is an official request for Professional/consultants who will stand as
our regional representative to run logistics on behalf of Shougang Group.
We are only looking for individual or company from USA and Canada. You
will be entitle to ten percent (10%) of every payment you receive from our
customers in your region on behalf of the company.

NOTE: You are not require to travel, all communications with our customers
will be through email or phone. All fees, including taxes will be handled
by us.You can apply for this position by filling the information below and
send back to us;

Full Name:
Residencial or office address:
Town:
State:
Zipcode:
Country:
Current Job:
If you own a company, please state the company name:
Prefered Email:
Gender:
Age:
Telephone:

If you have a current job, you can still be part of our business, as
services for us will not bother with your hours of work.

Respectfully,
Mr. Zhu Jimin (Chief Executive Officer)
Shougang Group



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

Re: [PATCH] transport: report missing submodule pushes consistently on stderr

[Leandro Lucarella]

On Tue, 23 Aug 2016 14:40:08 -0700
Stefan Beller  wrote:
> The surrounding advice is printed to stderr, but the list of
> submodules is not. Make the report consistent by reporting everything
> to stderr.
> 
> Signed-off-by: Stefan Beller 
> ---
> 
>   This fixes one of the bugs mentioned in
>   
> https://public-inbox.org/git/cagz79kbkyupbjfvyx3hj_r5zw36+3ufonnlc-dpic40npja...@mail.gmail.com/T/#t
>   
>   How to fix the other was not as obvious to me as I do not
> understand the philosophy on verbosity in the transport code.

I had a look and I would say just enclose all the fprintf() inside a:

if (transport->verbose > 0)

But then this is the first time I look at the code. I was about to send
a patch too but it will conflict with this one :)

Anyway, thanks for the quick fix to the inconsistent printing with
--quiet.


-- 
Leandro Lucarella
Technical Development Lead
Sociomantic Labs GmbH 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] git-series: track changes to a patch series over time

[Jakub Narębski]

W dniu 11.08.2016 o 00:07, Josh Triplett pisze:
> On August 9, 2016 11:37:31 PM HST, Richard Ipsum
>  wrote:
>> On Thu, Aug 04, 2016 at 12:40:58PM -1000, Josh Triplett wrote:
[...]

>>> If you use a format-patch diff that includes the headers and
>>> commit message, you could also support commenting on those in the
>>> same way. Does the notedb format support commenting on those?
>> 
>> Comments in notedb are just a git note keyed on the sha of the 
>> commit being commented on, I'm not certain what advantage a 
>> format-patch diff provides in this case?
> 
> I meant for opening in an editor to write email-reply-style comments.
> The review tool and review storage format should allow commenting on
> commit messages, not just diffs.

There is also cover letter and interdiff, and one would want to
be able to comment also on those.

So how notedb solve problem of in-diff comments, in-commit comments,
post-commit comments, whole series cover letter and cover-letter
comments, interdiff and interdiff message / comments?


Nb. GitHub Pull Requests include only some of those, compared to
the mailing list / Usenet news interface.
 
>> I've been closely following the 'patch submission process' thread, 
>> and given the discussion there I'm having doubts over the value of
>> comments in git-candidate vs the mailing list. It seems to me that 
>> git-candidate has many of the disadvantages of Github/Gitlab when
>> it comes to comments, for example, there is no threading.
> 
> That's not inherent, though. You could allow commenting on a comment
> easily enough. (Of course, at some point you've recreated email-style
> in-reply-to headers...)

I wonder if we could use 'parent' header of a commit message for this,
or equivalent...
 
>> Also the system would be less open than the mailing list, since, as
>> it stands currently you would require push access to the
>> repository to comment on anything.
> 
> You'd need a federation mechanism.

...which is as easy to set up and use as mailing list, for sending
patches, applying patches, and patch review.  And/or provide 
bi-directional interface to the mailing list (I think Debian 
infrastructure tries to be (inter)operable by email).

There are various federated technologies (like pump.io), the
problem might be their popularity.

>> It may be worth reflecting that one reason some organizations have
>> switched away from mailing list reviews to Github/Gitlab is that 
>> they provide patch tracking, where the mailing list provides none, 
>> so patches there can be 'lost'. So instead of trying to
>> reimplement an entire Gerrit/Github/Gitlab UI on the commandline, I
>> wonder whether it would be sufficient to add the minimum
>> functionality necessary to provide git with native patch tracking,
>> and leave comments for the mailing list. Of course this is exactly
>> what git-series seems to do, so in some sense I may be advocating
>> dropping my own work in favour of improving git-series.
> 
> I think the two serve different (though related) functions. I'd love
> to be able to use a text editor and command-line tool to produce and
> submit comments to systems like Gerrit or GitHub.

I think there are command-line tools that allow to submit comments
to GitHub.

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

Re: [PATCH v4] config: add conditional include

[Duy Nguyen]

On Tue, Aug 23, 2016 at 8:42 PM, Johannes Schindelin
 wrote:
> Hi Duy,
>
> On Mon, 22 Aug 2016, Duy Nguyen wrote:
>
>> On Mon, Aug 22, 2016 at 8:22 PM, Matthieu Moy
>>  wrote:
>> >>> I think the syntax should be design to allow arbitrary boolean
>> >>> expression later if needed.
>> >>
>> >> I would be against that. We may extend it more in future, but it
>> >> should be under control, not full boolean expressions.
>> >
>> > Why?
>> >
>> > I'm not saying we absolutely need it, but if we allow several kinds of
>> > conditions (gitdir-is:... and others in the future), then it's natural
>> > to allow combining them, and arbitrary boolean expression is both simple
>> > and powerful (operators and/or/not and parenthesis and you have
>> > everything you'll ever need).
>>
>> For starter, we don't want another debate "python vs ruby vs lua vs
>> ..." as the scripting language :) (for the record I vote Scheme! maybe
>> with infix syntax)
>
> FWIW I do not think that Matthieu implied that this has to be implemented.
> But it does not make sense to slam the door shut prematurely, either.
>
> Meaning: the more doors you can keep open with the new syntax, the better.

It works for me either way. So I'm going to assume we want
"[include-if "gitdir-is:..."]", unless people think it needs more
discussion (then just write something, anything, so I can put it back
in my backlog)
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] diff-highlight: add some tests.

[Lars Schneider]

> On 16 Aug 2016, at 22:48, Junio C Hamano  wrote:
> 
> Lars Schneider  writes:
> 
>>> On 30 Jul 2016, at 17:11, Brian Henderson  wrote:
>>> 
>>> ---
>>> contrib/diff-highlight/Makefile  |  5 ++
>>> contrib/diff-highlight/t/Makefile| 19 +++
>>> contrib/diff-highlight/t/t9400-diff-highlight.sh | 63 ++
>>> contrib/diff-highlight/t/test-diff-highlight.sh  | 69 
>>> 
>>> 4 files changed, 156 insertions(+)
>>> create mode 100644 contrib/diff-highlight/Makefile
>>> create mode 100644 contrib/diff-highlight/t/Makefile
>>> create mode 100644 contrib/diff-highlight/t/t9400-diff-highlight.sh
>>> create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh
>>> 
>>> diff --git a/contrib/diff-highlight/Makefile 
>>> b/contrib/diff-highlight/Makefile
>> 
>> Would it make sense to add the contrib tests to the Travis-CI build, too?
>> https://travis-ci.org/git/git/branches
> 
> The more the merrier ;-)?  I do not think of a downside, especially
> if you are adding it as a separate thing that comes after the main
> test, or for even better isolation, an entirely separate job.

OK, if I will post a patch (might take a while).


> By the way, how flaky are existing tests?  Are people actively
> following breakage reports?

Good question - I was curious about that, too. 
That's why I made a little website (only tested on Chrome, requires JS): 

https://larsxschneider.github.io/git-ci-stats/

There you can see all builds for all branches including their failure reason.
In general I would say flakiness is no issue anymore.

I don't know who else is looking at the breakage reports but I do. 
That's the reason for my "next" breakage reports in the past:

http://public-inbox.org/git/a6131c47-3230-4ec4-b3f6-b2507c937...@gmail.com/
http://public-inbox.org/git/4877318e-3cbf-4c87-b24d-aae35c427...@gmail.com/
http://public-inbox.org/git/fb76544f-16f7-45ca-9649-fd62ee44b...@gmail.com/
...

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

Re: Bug with git worktrees and git init

[Duy Nguyen]

On Tue, Aug 23, 2016 at 10:21 PM, Michael J Gruber
 wrote:
> I've cc'ed the master of worktrees.

Thanks for the analysis. I'll provide some patch as soon as possible.
This git-init may also be a good place to repair broken worktrees too
(e.g. because you moved a worktree manually)..

PS. Master? Wh I'm a master!! :D
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC PATCH] completion: support excluding refs

[Chris Packham]

Allow completion of refs with a ^ prefix. This allows completion of
commands like 'git log HEAD ^origin/master'.

Signed-off-by: Chris Packham 
---
I often find myself using variations of 'git log HEAD ^origin/master' to
see commits that I have locally that have not been pushed. It is also
useful when reviewing changes before integrating them e.g. 'git fetch
...; git log FETCH_HEAD ^origin/master'.

 contrib/completion/git-completion.bash | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 9c8f738..21016bf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -338,7 +338,7 @@ __git_tags ()
 __git_refs ()
 {
local i hash dir="$(__gitdir "${1-}")" track="${2-}"
-   local format refs
+   local format refs pfx
if [ -d "$dir" ]; then
case "$cur" in
refs|refs/*)
@@ -347,14 +347,15 @@ __git_refs ()
track=""
;;
*)
+   [[ "$cur" == ^* ]] && pfx="^"
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
-   if [ -e "$dir/$i" ]; then echo $i; fi
+   if [ -e "$dir/$i" ]; then echo $pfx$i; fi
done
format="refname:short"
refs="refs/tags refs/heads refs/remotes"
;;
esac
-   git --git-dir="$dir" for-each-ref --format="%($format)" \
+   git --git-dir="$dir" for-each-ref --format="$pfx%($format)" \
$refs
if [ -n "$track" ]; then
# employ the heuristic used by git checkout
-- 
2.9.2.518.ged577c6.dirty

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