Re: [PATCH v1 1/2] fsexcludes: add a programmatic way to exclude files from git's working directory traversal logic
Ben Peart writes:
> +void fsexcludes_free() {
Write this line like so:
void fsexcludes_free(void)
{
> +void fsexcludes_free();
void fsexcludes_free(void);
[PATCH v3 1/1] perl: fix installing modules from contrib
Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules) removed a target that allowed Makefiles from contrib/ to get the correct install path. This introduces a new target for main Makefile and fixes installation for Mediawiki module. v2: Pass prefix as that can have influence as well, add single quotes for _SQ variant. v3: Rename target, add to .PHONY. Signed-off-by: Christian Hesse --- Makefile | 3 +++ contrib/mw-to-git/Makefile | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f18168725..5a06eddf2 100644 --- a/Makefile +++ b/Makefile @@ -2014,6 +2014,9 @@ GIT-PERL-DEFINES: FORCE echo "$$FLAGS" >$@; \ fi +.PHONY: say-perllibdir +say-perllibdir: + @echo '$(perllibdir_SQ)' .PHONY: gitweb gitweb: diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile index a4b6f7a2c..e301a5b4e 100644 --- a/contrib/mw-to-git/Makefile +++ b/contrib/mw-to-git/Makefile @@ -21,8 +21,9 @@ HERE=contrib/mw-to-git/ INSTALL = install SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL)) -INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \ --s --no-print-directory instlibdir) +INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \ +-s --no-print-directory prefix=$(prefix) \ +perllibdir=$(perllibdir) say-perllibdir) DESTDIR_SQ = $(subst ','\'',$(DESTDIR)) INSTLIBDIR_SQ = $(subst ','\'',$(INSTLIBDIR))
Re: [PATCH v8 0/5] RUNTIME_PREFIX relocatable Git
Dan Jacques writes: > This is a minor update based on comments from the v6 series. > I'm hoping this set is good to go! > > This patch set expands support for the RUNTIME_PREFIX configuration flag, > currently only used on Windows builds, to include Linux, Darwin, and > FreeBSD. When Git is built with RUNTIME_PREFIX enabled, it resolves its > ancillary paths relative to the runtime location of its executable > rather than hard-coding them at compile-time, allowing a Git > installation to be deployed to a path other than the one in which it > was built/installed. > > Note that RUNTIME_PREFIX is not currently used outside of Windows. > This patch set should not have an impact on default Git builds. > > Previous threads: > v1: https://public-inbox.org/git/20171116170523.28696-1-...@google.com/ > v2: https://public-inbox.org/git/20171119173141.4896-1-...@google.com/ > v3: https://public-inbox.org/git/20171127164055.93283-1-...@google.com/ > v4: https://public-inbox.org/git/20171129223807.91343-1-...@google.com/ > v5: https://public-inbox.org/git/20180108030239.92036-1-...@google.com/ > v5: https://public-inbox.org/git/20180108030239.92036-1-...@google.com/ > v6: https://public-inbox.org/git/20180319025046.58052-1-...@google.com/ > v7: https://public-inbox.org/git/20180325205120.17730-1-...@google.com/ > > Changes in v8 from v7: > > - Add Johannes's Windows patch series to the end (see v7 thread). Wonderful. That gives me one less separate topic to worry about ;-) > - Fix more typos and formatting nits. > - Rebased on top of "master".
Re: [PATCH v6 04/15] sequencer: introduce new commands to reset the revision
Hi Johannes, Johannes Schindelin writes: [...] > We disallow '#' as label because that character will be used as separator > in the upcoming `merge` command. Please consider to use # not only in `merge` and `reset`, but in the rest of the commands as well, to unify this new syntax. I.e., right now it seems to be: pick abcd A commit message merge beaf # B commit message I suggest to turn it to: pick abcd # A commit message merge beaf # B commit message So that the # is finally universally the start of comment. While we are at this, I couldn't find any even semi-formal syntax description of the entire todo list. Is there one already? Are you going to provide one? -- Sergey
Tip of 'next' has been rewound post 2.17
I'll follow up with the full copy of "What's cooking" report later, but here is a quick heads-up to warn those who have been building their private editions on top of 'next' that the tip of 'next' has been rebuilt, while kicking a few topics back to 'pu'. Thanks.
Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology
Hi Johannes, Johannes Schindelin writes: > Hi Sergey, > > On Tue, 10 Apr 2018, Sergey Organov wrote: > >> Johannes Schindelin writes: >> >> > Once upon a time, I dreamt of an interactive rebase that would not >> > flatten branch structure, but instead recreate the commit topology >> > faithfully. >> >> [...] >> >> > Think of --rebase-merges as "--preserve-merges done right". >> >> Both option names seem to miss the primary point of the mode of >> operation that you've formulated in the first sentence. I suggest to >> rather call the new option in accordance to your description, say, >> --no-flatten, --keep-topology, or --preserve-shape. > > A very quick A/B test shows that neither --no-flatten nor --keep-topology > and certainly not --preserve-shape conveys to Git users what those options > are supposed to do. In fact, my preference would be --[no-]flatten, exactly because the default mode of rebase operation flattens the history, and thus what I'm talking about is: git rebase --no-flatten vs git rebase --rebase-merges I honestly fail to see how the latter conveys the purpose of the option better than the former, sorry. To tell the truth, the latter also looks plain ugly to me. > But --rebase-merges did convey the purpose of my patch series. So > there. - Except that your primary description of the series (that I find pretty solid) doesn't mention _merges_ at all and still conveys the purpose? - Except that this patch series _don't_ actually _rebase_ merges? Yeah, I remember a follow-up is to be expected, but anyway. I'm still unconvinced. -- Sergey
Re: Is support for 10.8 dropped?
Hi, Eric, Sorry for the long delay. On Sun, Apr 8, 2018 at 7:59 PM, Eric Sunshine wrote: > On Sun, Apr 8, 2018 at 7:55 PM, Igor Korot wrote: >> On Sun, Apr 8, 2018, 6:23 PM Eric Sunshine wrote: >>> And, as noted earlier, before running "make", you may need to create >>> config.mak to override some settings documented at the top of Makefile >>> (in particular, you may want to set NO_GETTEXT if you don't want to >>> install gettext and don't think you'll need it). As prerequisite, >>> you'll probably need to install OpenSSL. >> >> Is there a way to check for OpenSSL presence? > > Not sure what you're asking. Are you asking how to determine if you > already have OpenSSL built on your machine? Yes, that's what I was asking... > > Note that you might be able to get by without installing OpenSSL since > Git will try to use Apple's "Common Crypto" instead, so you could > define NO_OPENSSL in config.mak and see if the project builds. This is what I got trying to do just "make": MyMac:git-2.17.0 igorkorot$ make * new build flags CC credential-store.o In file included from credential-store.c:1: In file included from ./cache.h:9: ./gettext.h:17:11: fatal error: 'libintl.h' file not found # include ^ 1 error generated. make: *** [credential-store.o] Error 1 And I am also confused. Which file am I suppose to modify here? MyMac:git-2.17.0 igorkorot$ ls -la conf* -rwxr-xr-x@ 1 igorkorot staff 74461 Apr 2 10:13 config.c -rwxr-xr-x@ 1 igorkorot staff 9888 Apr 2 10:13 config.h -rwxr-xr-x@ 1 igorkorot staff540 Apr 2 10:13 config.mak.in -rwxr-xr-x@ 1 igorkorot staff 16940 Apr 2 10:13 config.mak.uname -rwxr-xr-x@ 1 igorkorot staff 37509 Apr 2 10:13 configure.ac -rw-r--r-- 1 igorkorot staff 37500 Apr 8 18:52 configure.ac+ Thank you.
Re: [PATCH v8 0/2] builtin/config.c: support `--type=` as preferred alias for `--type`
On Wed, Apr 11, 2018 at 12:11:47PM +0900, Junio C Hamano wrote:
> Taylor Blau writes:
>
> >> > +#define OPT_CALLBACK_VALUE(s, l, h, f, i) \
> >> > +{ OPTION_CALLBACK, (s), (l), NULL, NULL, (h), PARSE_OPT_NOARG |
> >> > \
> >> > +PARSE_OPT_NONEG, (f), (i) }
> >> > +
> >> > +static struct option builtin_config_options[];
> >>
> >> OK. I am not sure if OPT_CALLBACK_VALUE() needs to take 'f', as you
> >> always pass the option_parse_type function to it.
> >
> > That's fair. I left this in as an indication that something like this
> > _might_ want to make its way into parse-options.h as a general-purpose
> > utility, but was not yet ready to do so. Thus, I defined it inside
> > builtin/config.c.
>
> I understood the reasoning, but as your current verdict is that this
> is not yet ready for parse-options.[ch], I think it is probably
> preferrable to reduce repeated passing of the same function to the
> macro, at least while it is in this builgin/config.c file.
Ah, that seems fair to me. I have removed the duplicate 'f' parameter
and all of the option_parse_type()'s in builtin/config.c within my local
copy, and will happily include these changes in the subsequent re-roll.
I'll wait for more feedback before sending this, however.
Thanks,
Taylor
Re: [PATCH v8 0/2] builtin/config.c: support `--type=` as preferred alias for `--type`
Taylor Blau writes:
>> > +#define OPT_CALLBACK_VALUE(s, l, h, f, i) \
>> > + { OPTION_CALLBACK, (s), (l), NULL, NULL, (h), PARSE_OPT_NOARG | \
>> > + PARSE_OPT_NONEG, (f), (i) }
>> > +
>> > +static struct option builtin_config_options[];
>>
>> OK. I am not sure if OPT_CALLBACK_VALUE() needs to take 'f', as you
>> always pass the option_parse_type function to it.
>
> That's fair. I left this in as an indication that something like this
> _might_ want to make its way into parse-options.h as a general-purpose
> utility, but was not yet ready to do so. Thus, I defined it inside
> builtin/config.c.
I understood the reasoning, but as your current verdict is that this
is not yet ready for parse-options.[ch], I think it is probably
preferrable to reduce repeated passing of the same function to the
macro, at least while it is in this builgin/config.c file.
> @@ -71,7 +71,7 @@ static int option_parse_type(const struct option *opt,
> const char *arg,
>int unset)
> {
> if (unset) {
> - type = 0;
> + *((int *) opt->value) = 0;
> return 0;
> }
Yup. This (if it works) is exactly what I imagined the function
should look like.
Re: [PATCH v2 06/10] commit.c: use generation to halt paint walk
Derrick Stolee writes:
> @@ -800,17 +810,26 @@ static struct commit_list *paint_down_to_common(struct
> commit *one, int n, struc
> return result;
> }
> prio_queue_put(&queue, one);
> + if (one->generation < min_nonstale_gen)
> + min_nonstale_gen = one->generation;
>
> for (i = 0; i < n; i++) {
> twos[i]->object.flags |= PARENT2;
> prio_queue_put(&queue, twos[i]);
> + if (twos[i]->generation < min_nonstale_gen)
> + min_nonstale_gen = twos[i]->generation;
> }
>
> - while (queue_has_nonstale(&queue)) {
> + while (queue_has_nonstale(&queue, min_nonstale_gen)) {
> struct commit *commit = prio_queue_get(&queue);
> struct commit_list *parents;
> int flags;
>
> + if (commit->generation > last_gen)
> + BUG("bad generation skip");
> +
> + last_gen = commit->generation;
> +
> flags = commit->object.flags & (PARENT1 | PARENT2 | STALE);
> if (flags == (PARENT1 | PARENT2)) {
> if (!(commit->object.flags & RESULT)) {
> @@ -830,6 +849,10 @@ static struct commit_list *paint_down_to_common(struct
> commit *one, int n, struc
> return NULL;
> p->object.flags |= flags;
Hmph. Can a commit that used to be not stale (and contributed to
the current value of min_nonstale_gen) become stale here by getting
visited twice, invalidating the value in min_nonstale_gen?
> prio_queue_put(&queue, p);
> +
> + if (!(flags & STALE) &&
> + p->generation < min_nonstale_gen)
> + min_nonstale_gen = p->generation;
> }
> }
Re: [PATCH v2 04/10] commit-graph: compute generation numbers
Derrick Stolee writes:
> + if ((*list)->generation != GENERATION_NUMBER_INFINITY) {
> + if ((*list)->generation > GENERATION_NUMBER_MAX)
> + die("generation number %u is too large to store
> in commit-graph",
> + (*list)->generation);
> + packedDate[0] |= htonl((*list)->generation << 2);
> + }
How serious do we want this feature to be? On one extreme, we could
be irresponsible and say it will be a problem for our descendants in
the future if their repositories have more than billion pearls on a
single strand, and the above certainly is a reasonable way to punt.
Those who actually encounter the problem will notice by Git dying
somewhere rather deep in the callchain.
Or we could say Git actually does support a history that is
arbitrarily long, even though such a deep portion of history will
not benefit from having generation numbers in commit-graph.
I've been assuming that our stance is the latter and that is why I
made noises about overflowing 30-bit generation field in my review
of the previous step.
In case we want to do the "we know this is very large, but we do not
know the exact value", we may actually want a mode where we can
pretend that GENERATION_NUMBER_MAX is set to quite low (say 256) and
make sure that the code to handle overflow behaves sensibly.
> + for (i = 0; i < nr_commits; i++) {
> + if (commits[i]->generation != GENERATION_NUMBER_INFINITY &&
> + commits[i]->generation != GENERATION_NUMBER_ZERO)
> + continue;
> +
> + commit_list_insert(commits[i], &list);
> + while (list) {
> +...
> + }
> + }
So we go over the list of commits just _once_ and make sure each of
them gets the generation assigned correctly by (conceptually
recursively but iteratively in implementation by using a commit
list) making sure that all its parents have generation assigned and
compute the generation for the commit, before moving to the next
one. Which sounds correct.
Re: [PATCH v2 03/10] commit: add generation number to struct commmit
Derrick Stolee writes:
> The generation number of a commit is defined recursively as follows:
>
> * If a commit A has no parents, then the generation number of A is one.
> * If a commit A has parents, then the generation number of A is one
> more than the maximum generation number among the parents of A.
>
> Add a uint32_t generation field to struct commit so we can pass this
> information to revision walks. We use two special values to signal
> the generation number is invalid:
>
> GENERATION_NUMBER_ININITY 0x
> GENERATION_NUMBER_ZERO 0
>
> The first (_INFINITY) means the generation number has not been loaded or
> computed. The second (_ZERO) means the generation number was loaded
> from a commit graph file that was stored before generation numbers
> were computed.
Should it also be possible for a caller to tell if a given commit
has too deep a history, i.e. we do not know its generation number
exactly, but we know it is larger than 1<<30?
It seems that we only have a 30-bit field in the file, so wouldn't
we need a special value defined in (e.g. "0") so that we can tell
that the commit has such a large generation number? E.g.
> + item->generation = get_be32(commit_data + g->hash_len + 8) >> 2;
if (!item->generation)
item->generation = GENERATION_NUMBER_OVERFLOW;
when we read it from the file?
We obviously need to do something similar when assigning a
generation number to a child commit, perhaps like
#define GENERATION_NUMBER_OVERFLOW (GENERATION_NUMBER_MAX + 1)
commit->generation = 1; /* assume no parent */
for (p = commit->parents; p; p++) {
uint32_t gen = p->item->generation + 1;
if (gen >= GENERATION_NUMBER_OVERFLOW) {
commit->generation = GENERATION_NUMBER_OVERFLOW;
break;
} else if (commit->generation < gen)
commit->generation = gen;
}
or something? And then on the writing side you'd encode too large a
generation as '0'.
Re: [PATCH v2 02/10] merge: check config before loading commits
Derrick Stolee writes:
> diff --git a/builtin/merge.c b/builtin/merge.c
> index ee050a47f3..20897f8223 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1183,13 +1183,14 @@ int cmd_merge(int argc, const char **argv, const char
> *prefix)
> branch = branch_to_free = resolve_refdup("HEAD", 0, &head_oid, NULL);
> if (branch)
> skip_prefix(branch, "refs/heads/", &branch);
> + init_diff_ui_defaults();
> + git_config(git_merge_config, NULL);
> +
> if (!branch || is_null_oid(&head_oid))
> head_commit = NULL;
> else
> head_commit = lookup_commit_or_die(&head_oid, "HEAD");
>
> - init_diff_ui_defaults();
> - git_config(git_merge_config, NULL);
Wow, that's tricky. git_merge_config() wants to know which "branch"
we are on, and this place is as early as we can move the call to
without breaking things. Is this to allow parse_object() called
in lookup_commit_reference_gently() to know if we can rely on the
data cached in the commit-graph data?
> Move the config load to be between the initialization of 'branch'
> and the commit lookup. Also add a test to t5318-commit-graph.sh
> that exercises this code path to prevent a regression.
It is not clear to me how a successful merge of commits/8
demonstrates that reading the config earlier than before is
regression free.
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
> index a380419b65..77d85aefe7 100755
> --- a/t/t5318-commit-graph.sh
> +++ b/t/t5318-commit-graph.sh
> @@ -221,4 +221,13 @@ test_expect_success 'write graph in bare repo' '
> graph_git_behavior 'bare repo with graph, commit 8 vs merge 1' bare
> commits/8 merge/1
> graph_git_behavior 'bare repo with graph, commit 8 vs merge 2' bare
> commits/8 merge/2
>
> +test_expect_success 'perform fast-forward merge in full repo' '
> + cd "$TRASH_DIRECTORY/full" &&
> + git checkout -b merge-5-to-8 commits/5 &&
> + git merge commits/8 &&
> + git show-ref -s merge-5-to-8 >output &&
> + git show-ref -s commits/8 >expect &&
> + test_cmp expect output
> +'
> +
> test_done
Re: git-lfs question
On Wed, Apr 11, 2018 at 01:16:42AM +, John Sullivan wrote: > Hello - I've seen instructions that say after installing git to also install > git-lfs. > > But today when installing git I noticed that in the install options > there was a default selected options stating "Git LFS (Large File > Support)". > > Does this mean git is automatically adding git-LFS or just adding > support for it and I'll still need to install git-lfs afterwards? If you're using Git for Windows, it will install Git LFS as well by default. Of course, you can opt-out of this if it's not something that you want :-). This is not the case on other platforms, to the best of my knowledge. Thanks, Taylor
Re: [PATCH v8 0/2] builtin/config.c: support `--type=` as preferred alias for `--type`
On Wed, Apr 11, 2018 at 10:24:45AM +0900, Junio C Hamano wrote:
> Taylor Blau writes:
>
> > Attached is the eighth re-roll of my series to add `--type=` as
> > the preferred alternative for `--`.
> >
> > The main changes since v7 concern handling degenerate cases, such as:
> >
> > * git config --type=int --type=bool
> > * git config --type=int --int
> >
> > We have previously had discussion about whether we should (1) retain the
> > error in previous versions when confronted with multiple, conflicting
> > type specifiers, (2) ignore the error, in favor of making -- and
> > --type= true synonyms, or (3) some combination of the two.
> >
> > I have thought some more about my argument that it would be favorable to
> > make "--type=int" and "--int" behave in the same way, and I am no
> > longer convinced that my argument makes sense. It's based on the premise
> > that "--type=" must _necessarily_ allow multiple invocations, such
> > as '--type=int --type=bool', and therefore "--int --bool" should be
> > updated to behave the same way.
> >
> > We are not constrained to this behavior, so in v8, I have taught Git the
> > following:
> >
> > 1. Allow multiple non-conflicting types, such as '--int --int',
> > '--type=int --int', and '--int --type=int'.
> >
> > 2. Disallow multiple conflicting types, such as '--int --bool',
> > '--type=int --bool', and '--int --type=bool'.
> >
> > 3. Allow conflicting types following --no-type, such as '--int
> > --no-type --bool', '--type=int --no-type --bool', and '--int
> > --no-type --type=bool'. Note that this does _not_ introduce options
> > such as '--no-int' and whatnot.
> >
> > This is accomplished by a new locally defined macro called
> > OPT_CALLBACK_VALUE, which allows us to reuse option_parse_type() to
> > handle --int as well, by sending it through as opt->defval.
> >
> > I think that the above is the best-of-all-worlds choice, but I am
> > curious to hear everyone else's thoughts. Thanks in advance for your
> > review.
>
> I too am curious. Personally I do not think your "last one wins"
> was necessarily bad--in fact it internally was consistent--I just
> thought that the log message did not justify the choice well. And I
> do not think the semantics defined by this one, "once you choose,
> stick to it, or explicitly clear the previous choice", is bad,
> either.
:-). If nothing else, I like that we retain more, stricter behavior from
previous versions.
> > diff --git a/builtin/config.c b/builtin/config.c
> > index 5c8952a17c..7184c09582 100644
> > --- a/builtin/config.c
> > +++ b/builtin/config.c
> > @@ -61,28 +61,53 @@ static int show_origin;
> > #define TYPE_PATH 4
> > #define TYPE_EXPIRY_DATE 5
> >
> > +#define OPT_CALLBACK_VALUE(s, l, h, f, i) \
> > + { OPTION_CALLBACK, (s), (l), NULL, NULL, (h), PARSE_OPT_NOARG | \
> > + PARSE_OPT_NONEG, (f), (i) }
> > +
> > +static struct option builtin_config_options[];
>
> OK. I am not sure if OPT_CALLBACK_VALUE() needs to take 'f', as you
> always pass the option_parse_type function to it.
That's fair. I left this in as an indication that something like this
_might_ want to make its way into parse-options.h as a general-purpose
utility, but was not yet ready to do so. Thus, I defined it inside
builtin/config.c.
> > static int option_parse_type(const struct option *opt, const char *arg,
> > int unset)
> > {
> > - int *type = opt->value;
> > -
> > if (unset) {
> > - *type = 0;
> > + type = 0;
> > return 0;
> > }
> >
> > - if (!strcmp(arg, "bool"))
> > - *type = TYPE_BOOL;
> > - else if (!strcmp(arg, "int"))
> > - *type = TYPE_INT;
> > - else if (!strcmp(arg, "bool-or-int"))
> > - *type = TYPE_BOOL_OR_INT;
> > - else if (!strcmp(arg, "path"))
> > - *type = TYPE_PATH;
> > - else if (!strcmp(arg, "expiry-date"))
> > - *type = TYPE_EXPIRY_DATE;
> > - else
> > - die(_("unrecognized --type argument, %s"), arg);
> > + /*
> > +* To support '--' style flags, begin with new_type equal to
> > +* opt->defval.
> > +*/
> > + int new_type = opt->defval;
> > + if (!new_type) {
> > + if (!strcmp(arg, "bool"))
> > + new_type = TYPE_BOOL;
> > + else if (!strcmp(arg, "int"))
> > + new_type = TYPE_INT;
> > + else if (!strcmp(arg, "bool-or-int"))
> > + new_type = TYPE_BOOL_OR_INT;
> > + else if (!strcmp(arg, "path"))
> > + new_type = TYPE_PATH;
> > + else if (!strcmp(arg, "expiry-date"))
> > + new_type = TYPE_EXPIRY_DATE;
> > + else
> > + die(_("unrecognized --type argument, %s"), arg);
> > + }
> > +
> > + if (type != 0 && type != new_type) {
> > + /*
> > +* Complain when there is a new type not equal to the old type.
> > +* This allo
Re: [PATCH v8 0/2] builtin/config.c: support `--type=` as preferred alias for `--type`
Taylor Blau writes:
> Attached is the eighth re-roll of my series to add `--type=` as
> the preferred alternative for `--`.
>
> The main changes since v7 concern handling degenerate cases, such as:
>
> * git config --type=int --type=bool
> * git config --type=int --int
>
> We have previously had discussion about whether we should (1) retain the
> error in previous versions when confronted with multiple, conflicting
> type specifiers, (2) ignore the error, in favor of making -- and
> --type= true synonyms, or (3) some combination of the two.
>
> I have thought some more about my argument that it would be favorable to
> make "--type=int" and "--int" behave in the same way, and I am no
> longer convinced that my argument makes sense. It's based on the premise
> that "--type=" must _necessarily_ allow multiple invocations, such
> as '--type=int --type=bool', and therefore "--int --bool" should be
> updated to behave the same way.
>
> We are not constrained to this behavior, so in v8, I have taught Git the
> following:
>
> 1. Allow multiple non-conflicting types, such as '--int --int',
> '--type=int --int', and '--int --type=int'.
>
> 2. Disallow multiple conflicting types, such as '--int --bool',
> '--type=int --bool', and '--int --type=bool'.
>
> 3. Allow conflicting types following --no-type, such as '--int
> --no-type --bool', '--type=int --no-type --bool', and '--int
> --no-type --type=bool'. Note that this does _not_ introduce options
> such as '--no-int' and whatnot.
>
> This is accomplished by a new locally defined macro called
> OPT_CALLBACK_VALUE, which allows us to reuse option_parse_type() to
> handle --int as well, by sending it through as opt->defval.
>
> I think that the above is the best-of-all-worlds choice, but I am
> curious to hear everyone else's thoughts. Thanks in advance for your
> review.
I too am curious. Personally I do not think your "last one wins"
was necessarily bad--in fact it internally was consistent--I just
thought that the log message did not justify the choice well. And I
do not think the semantics defined by this one, "once you choose,
stick to it, or explicitly clear the previous choice", is bad,
either.
> diff --git a/builtin/config.c b/builtin/config.c
> index 5c8952a17c..7184c09582 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -61,28 +61,53 @@ static int show_origin;
> #define TYPE_PATH4
> #define TYPE_EXPIRY_DATE 5
>
> +#define OPT_CALLBACK_VALUE(s, l, h, f, i) \
> + { OPTION_CALLBACK, (s), (l), NULL, NULL, (h), PARSE_OPT_NOARG | \
> + PARSE_OPT_NONEG, (f), (i) }
> +
> +static struct option builtin_config_options[];
OK. I am not sure if OPT_CALLBACK_VALUE() needs to take 'f', as you
always pass the option_parse_type function to it.
> static int option_parse_type(const struct option *opt, const char *arg,
>int unset)
> {
> - int *type = opt->value;
> -
> if (unset) {
> - *type = 0;
> + type = 0;
> return 0;
> }
>
> - if (!strcmp(arg, "bool"))
> - *type = TYPE_BOOL;
> - else if (!strcmp(arg, "int"))
> - *type = TYPE_INT;
> - else if (!strcmp(arg, "bool-or-int"))
> - *type = TYPE_BOOL_OR_INT;
> - else if (!strcmp(arg, "path"))
> - *type = TYPE_PATH;
> - else if (!strcmp(arg, "expiry-date"))
> - *type = TYPE_EXPIRY_DATE;
> - else
> - die(_("unrecognized --type argument, %s"), arg);
> + /*
> + * To support '--' style flags, begin with new_type equal to
> + * opt->defval.
> + */
> + int new_type = opt->defval;
> + if (!new_type) {
> + if (!strcmp(arg, "bool"))
> + new_type = TYPE_BOOL;
> + else if (!strcmp(arg, "int"))
> + new_type = TYPE_INT;
> + else if (!strcmp(arg, "bool-or-int"))
> + new_type = TYPE_BOOL_OR_INT;
> + else if (!strcmp(arg, "path"))
> + new_type = TYPE_PATH;
> + else if (!strcmp(arg, "expiry-date"))
> + new_type = TYPE_EXPIRY_DATE;
> + else
> + die(_("unrecognized --type argument, %s"), arg);
> + }
> +
> + if (type != 0 && type != new_type) {
> + /*
> + * Complain when there is a new type not equal to the old type.
> + * This allows for combinations like '--int --type=int' and
> + * '--type=int --type=int', but disallows ones like '--type=bool
> + * --int' and '--type=bool
> + * --type=int'.
> + */
> + error("only one type at a time.");
> + usage_with_options(builtin_config_usage,
> + builtin_config_options);
> + }
> + type = new_type;
Does this rely on a file-scope global variable (type)?
>
> return 0;
> }
git-lfs question
Hello - I've seen instructions that say after installing git to also install git-lfs. But today when installing git I noticed that in the install options there was a default selected options stating "Git LFS (Large File Support)". Does this mean git is automatically adding git-LFS or just adding support for it and I'll still need to install git-lfs afterwards? Thanks, John
[PATCH v8 0/2] builtin/config.c: support `--type=` as preferred alias for `--type`
Hi,
Attached is the eighth re-roll of my series to add `--type=` as
the preferred alternative for `--`.
The main changes since v7 concern handling degenerate cases, such as:
* git config --type=int --type=bool
* git config --type=int --int
We have previously had discussion about whether we should (1) retain the
error in previous versions when confronted with multiple, conflicting
type specifiers, (2) ignore the error, in favor of making -- and
--type= true synonyms, or (3) some combination of the two.
I have thought some more about my argument that it would be favorable to
make "--type=int" and "--int" behave in the same way, and I am no
longer convinced that my argument makes sense. It's based on the premise
that "--type=" must _necessarily_ allow multiple invocations, such
as '--type=int --type=bool', and therefore "--int --bool" should be
updated to behave the same way.
We are not constrained to this behavior, so in v8, I have taught Git the
following:
1. Allow multiple non-conflicting types, such as '--int --int',
'--type=int --int', and '--int --type=int'.
2. Disallow multiple conflicting types, such as '--int --bool',
'--type=int --bool', and '--int --type=bool'.
3. Allow conflicting types following --no-type, such as '--int
--no-type --bool', '--type=int --no-type --bool', and '--int
--no-type --type=bool'. Note that this does _not_ introduce options
such as '--no-int' and whatnot.
This is accomplished by a new locally defined macro called
OPT_CALLBACK_VALUE, which allows us to reuse option_parse_type() to
handle --int as well, by sending it through as opt->defval.
I think that the above is the best-of-all-worlds choice, but I am
curious to hear everyone else's thoughts. Thanks in advance for your
review.
Thanks,
Taylor
Taylor Blau (2):
builtin/config.c: treat type specifiers singularly
builtin/config.c: support `--type=` as preferred alias for
`--type`
Documentation/git-config.txt | 71 +---
builtin/config.c | 101 +--
t/t1300-repo-config.sh | 63 ++
3 files changed, 176 insertions(+), 59 deletions(-)
Inter-diff (since v7):
diff --git a/builtin/config.c b/builtin/config.c
index 5c8952a17c..7184c09582 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -61,28 +61,53 @@ static int show_origin;
#define TYPE_PATH 4
#define TYPE_EXPIRY_DATE 5
+#define OPT_CALLBACK_VALUE(s, l, h, f, i) \
+ { OPTION_CALLBACK, (s), (l), NULL, NULL, (h), PARSE_OPT_NOARG | \
+ PARSE_OPT_NONEG, (f), (i) }
+
+static struct option builtin_config_options[];
+
static int option_parse_type(const struct option *opt, const char *arg,
int unset)
{
- int *type = opt->value;
-
if (unset) {
- *type = 0;
+ type = 0;
return 0;
}
- if (!strcmp(arg, "bool"))
- *type = TYPE_BOOL;
- else if (!strcmp(arg, "int"))
- *type = TYPE_INT;
- else if (!strcmp(arg, "bool-or-int"))
- *type = TYPE_BOOL_OR_INT;
- else if (!strcmp(arg, "path"))
- *type = TYPE_PATH;
- else if (!strcmp(arg, "expiry-date"))
- *type = TYPE_EXPIRY_DATE;
- else
- die(_("unrecognized --type argument, %s"), arg);
+ /*
+* To support '--' style flags, begin with new_type equal to
+* opt->defval.
+*/
+ int new_type = opt->defval;
+ if (!new_type) {
+ if (!strcmp(arg, "bool"))
+ new_type = TYPE_BOOL;
+ else if (!strcmp(arg, "int"))
+ new_type = TYPE_INT;
+ else if (!strcmp(arg, "bool-or-int"))
+ new_type = TYPE_BOOL_OR_INT;
+ else if (!strcmp(arg, "path"))
+ new_type = TYPE_PATH;
+ else if (!strcmp(arg, "expiry-date"))
+ new_type = TYPE_EXPIRY_DATE;
+ else
+ die(_("unrecognized --type argument, %s"), arg);
+ }
+
+ if (type != 0 && type != new_type) {
+ /*
+* Complain when there is a new type not equal to the old type.
+* This allows for combinations like '--int --type=int' and
+* '--type=int --type=int', but disallows ones like '--type=bool
+* --int' and '--type=bool
+* --type=int'.
+*/
+ error("only one type at a time.");
+ usage_with_options(builtin_config_usage,
+ builtin_config_options);
+ }
+ type = new_type;
return 0;
}
@@ -110,12 +135,12 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot
[default]"), ACTION_GET_COLOR),
OPT_BIT(0, "get-colorbool",
[PATCH v8 1/2] builtin/config.c: treat type specifiers singularly
Internally, we represent `git config`'s type specifiers as a bitset
using OPT_BIT. 'bool' is 1<<0, 'int' is 1<<1, and so on. This technique
allows for the representation of multiple type specifiers in the `int
types` field, but this multi-representation is left unused.
In fact, `git config` will not accept multiple type specifiers at a
time, as indicated by:
$ git config --int --bool some.section
error: only one type at a time.
This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type
specifier, so that the above command would instead be valid, and a
synonym of:
$ git config --bool some.section
This change is motivated by two urges: (1) it does not make sense to
represent a singular type specifier internally as a bitset, only to
complain when there are multiple bits in the set. `OPT_SET_INT` is more
well-suited to this task than `OPT_BIT` is. (2) a future patch will
introduce `--type=`, and we would like not to complain in the
following situation:
$ git config --int --type=int
Signed-off-by: Taylor Blau
---
builtin/config.c | 49 +++---
t/t1300-repo-config.sh | 11 ++
2 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 01169dd628..92fb8d56b1 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -25,7 +25,7 @@ static char term = '\n';
static int use_global_config, use_system_config, use_local_config;
static struct git_config_source given_config_source;
-static int actions, types;
+static int actions, type;
static int end_null;
static int respect_includes_opt = -1;
static struct config_options config_options;
@@ -55,11 +55,11 @@ static int show_origin;
#define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \
ACTION_GET_REGEXP | ACTION_GET_URLMATCH)
-#define TYPE_BOOL (1<<0)
-#define TYPE_INT (1<<1)
-#define TYPE_BOOL_OR_INT (1<<2)
-#define TYPE_PATH (1<<3)
-#define TYPE_EXPIRY_DATE (1<<4)
+#define TYPE_BOOL 1
+#define TYPE_INT 2
+#define TYPE_BOOL_OR_INT 3
+#define TYPE_PATH 4
+#define TYPE_EXPIRY_DATE 5
static struct option builtin_config_options[] = {
OPT_GROUP(N_("Config file location")),
@@ -84,11 +84,11 @@ static struct option builtin_config_options[] = {
OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot
[default]"), ACTION_GET_COLOR),
OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot
[stdout-is-tty]"), ACTION_GET_COLORBOOL),
OPT_GROUP(N_("Type")),
- OPT_BIT(0, "bool", &types, N_("value is \"true\" or \"false\""),
TYPE_BOOL),
- OPT_BIT(0, "int", &types, N_("value is decimal number"), TYPE_INT),
- OPT_BIT(0, "bool-or-int", &types, N_("value is --bool or --int"),
TYPE_BOOL_OR_INT),
- OPT_BIT(0, "path", &types, N_("value is a path (file or directory
name)"), TYPE_PATH),
- OPT_BIT(0, "expiry-date", &types, N_("value is an expiry date"),
TYPE_EXPIRY_DATE),
+ OPT_SET_INT(0, "bool", &type, N_("value is \"true\" or \"false\""),
TYPE_BOOL),
+ OPT_SET_INT(0, "int", &type, N_("value is decimal number"), TYPE_INT),
+ OPT_SET_INT(0, "bool-or-int", &type, N_("value is --bool or --int"),
TYPE_BOOL_OR_INT),
+ OPT_SET_INT(0, "path", &type, N_("value is a path (file or directory
name)"), TYPE_PATH),
+ OPT_SET_INT(0, "expiry-date", &type, N_("value is an expiry date"),
TYPE_EXPIRY_DATE),
OPT_GROUP(N_("Other")),
OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
@@ -149,26 +149,26 @@ static int format_config(struct strbuf *buf, const char
*key_, const char *value
if (show_keys)
strbuf_addch(buf, key_delim);
- if (types == TYPE_INT)
+ if (type == TYPE_INT)
strbuf_addf(buf, "%"PRId64,
git_config_int64(key_, value_ ? value_ :
""));
- else if (types == TYPE_BOOL)
+ else if (type == TYPE_BOOL)
strbuf_addstr(buf, git_config_bool(key_, value_) ?
"true" : "false");
- else if (types == TYPE_BOOL_OR_INT) {
+ else if (type == TYPE_BOOL_OR_INT) {
int is_bool, v;
v = git_config_bool_or_int(key_, value_, &is_bool);
if (is_bool)
strbuf_addstr(buf, v ? "true" : "false");
else
strbuf_addf(buf, "%d", v);
- } else if (types == TYPE_PATH) {
+ } else if (type == TYPE_PATH) {
const char *v;
if (git_config_pathname(&v, key_, value_) < 0)
return -1;
[PATCH v8 2/2] builtin/config.c: support `--type=` as preferred alias for `--type`
`git config` has long allowed the ability for callers to provide a 'type specifier', which instructs `git config` to (1) ensure that incoming values can be interpreted as that type, and (2) that outgoing values are canonicalized under that type. In another series, we propose to extend this functionality with `--type=color` and `--default` to replace `--get-color`. However, we traditionally use `--color` to mean "colorize this output", instead of "this value should be treated as a color". Currently, `git config` does not support this kind of colorization, but we should be careful to avoid squatting on this option too soon, so that `git config` can support `--color` (in the traditional sense) in the future, if that is desired. In this patch, we support `--type=` in addition to `--int`, `--bool`, and etc. This allows the aforementioned upcoming patch to support querying a color value with a default via `--type=color --default=...`, without squandering `--color`. We retain the historic behavior of complaining when multiple, legacy-style `--` flags are given, as well as extend this to conflicting new-style `--type=` flags. `--int --type=int` (and its commutative pair) does not complain, but `--bool --type=int` (and its commutative pair) does. Signed-off-by: Taylor Blau --- Documentation/git-config.txt | 71 builtin/config.c | 62 --- t/t1300-repo-config.sh | 58 +++-- 3 files changed, 151 insertions(+), 40 deletions(-) diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index e09ed5d7d5..b759009c75 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -9,13 +9,13 @@ git-config - Get and set repository or global options SYNOPSIS [verse] -'git config' [] [type] [--show-origin] [-z|--null] name [value [value_regex]] -'git config' [] [type] --add name value -'git config' [] [type] --replace-all name value [value_regex] -'git config' [] [type] [--show-origin] [-z|--null] --get name [value_regex] -'git config' [] [type] [--show-origin] [-z|--null] --get-all name [value_regex] -'git config' [] [type] [--show-origin] [-z|--null] [--name-only] --get-regexp name_regex [value_regex] -'git config' [] [type] [-z|--null] --get-urlmatch name URL +'git config' [] [--type=] [--show-origin] [-z|--null] name [value [value_regex]] +'git config' [] [--type=] --add name value +'git config' [] [--type=] --replace-all name value [value_regex] +'git config' [] [--type=] [--show-origin] [-z|--null] --get name [value_regex] +'git config' [] [--type=] [--show-origin] [-z|--null] --get-all name [value_regex] +'git config' [] [--type=] [--show-origin] [-z|--null] [--name-only] --get-regexp name_regex [value_regex] +'git config' [] [--type=] [-z|--null] --get-urlmatch name URL 'git config' [] --unset name [value_regex] 'git config' [] --unset-all name [value_regex] 'git config' [] --rename-section old_name new_name @@ -38,12 +38,10 @@ existing values that match the regexp are updated or unset. If you want to handle the lines that do *not* match the regex, just prepend a single exclamation mark in front (see also <>). -The type specifier can be either `--int` or `--bool`, to make -'git config' ensure that the variable(s) are of the given type and -convert the value to the canonical form (simple decimal number for int, -a "true" or "false" string for bool), or `--path`, which does some -path expansion (see `--path` below). If no type specifier is passed, no -checks or transformations are performed on the value. +The `--type=` option instructs 'git config' to ensure that incoming and +outgoing values are canonicalize-able under the given . If no +`--type=` is given, no canonicalization will be performed. Callers may +unset an existing `--type` specifier with `--no-type`. When reading, the values are read from the system, global and repository local configuration files by default, and options @@ -160,30 +158,39 @@ See also <>. --list:: List all variables set in config file, along with their values. ---bool:: - 'git config' will ensure that the output is "true" or "false" +--type :: + 'git config' will ensure that any input or output is valid under the given + type constraint(s), and will canonicalize outgoing values in ``'s + canonical form. ++ +Valid ``'s include: ++ +- 'bool': canonicalize values as either "true" or "false". +- 'int': canonicalize values as simple decimal numbers. An optional suffix of + 'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or + 1073741824 upon input. +- 'bool-or-int': canonicalize according to either 'bool' or 'int', as described + above. +- 'path': canonicalize by adding a leading `~` to the value of `$HOME` and + `~user` to the home directory for the specified user. This specifier has no + effect when setting the value (but you can use `git config section.variable +
Re: git-gui ignores core.hooksPath
Ævar Arnfjörð Bjarmason writes: > Isn't everyone involved much better solved if we come up with some plan > to split these off from git.git? I.e. I think if if git-gui, gitk and > gitweb were proposed for inclusion in-tree today I don't think we'd > bite, and instead point to things like [1] or [2]. It is actually the other way around. gitk and git-gui had active maintainers and they (not Linus nor I) wanted to have their own release schedule and versioning; they started as eparate projects. The arrangement was pleasant to work with while the subsystem was actively managed. People can send patches to it just like to other parts of the system (and the change rarely if ever needs to touch both core and GUI at the same time---otherwise it would be impractical to split them out as a separate projects), reviewers would give comments on the list, and subsystem maintainers would pick them up just like I pick up patches to the core part. Then from time to time, subsystem maintainers would give me a pull request to complete the cycle. I do "pull -Xsubtree=git-gui/". It breaks down when subsystem maintainers go quiet. Even if I were to play a patch monkey backed by volunteer reviewers, I'd still have to pretend that these are separate projects that are occasionally subtree merged, with my own pull requests to subsystem repositories that may never be responded, just in case the separate subsystem projects will become active again. If you split git-gui off, it would just die, unless somebody steps up and takes it over. And if somebody steps up and takes it over, we can keep merging from their repositories without any problem.
Re: git-gui ignores core.hooksPath
On Tue, Apr 10 2018, Junio C. Hamano wrote: > Chris Maes writes: > >> Is there any hope from here that anyone will pick up this / these >> changes? Will anyone else be assigned the main responsible for this >> git-gui repository? >> >> Just hoping to revive the discussion here, since the >> https://github.com/patthoyts/git-gui/ repository seems quite dead. > > It indeed does. > > I've played a patch-monkey in the past for git-gui and have a few > topics queued still in my tree, but that serves merely as a bookmark > that is slightly better than a pointer to the mailing list archive. > > We need a volunteer to take over this part of the subsystem; > somebody who actually uses it, passionate about improving it, and > can speak tcl/tk somewhat fluently (I qualify none of these three > criteria myself). > > Any takers? Isn't everyone involved much better solved if we come up with some plan to split these off from git.git? I.e. I think if if git-gui, gitk and gitweb were proposed for inclusion in-tree today I don't think we'd bite, and instead point to things like [1] or [2]. Of the three gitweb is a bit more glued to the rest of the codebase (test & custom test lib), but gitk and git-gui already have their own external projects. It looks like if they started making tagged releases, along with some small Makefile changes like generating their manpages from ASCIIDOC, downstream packagers of git-gui and gitk could simply start pointing to those as the authoritative source instead of whatever gets shipped with git. 1. https://git-scm.com/downloads/guis/ 2. https://git.wiki.kernel.org/index.php/Interfaces,_frontends,_and_tools#Web_Interfaces
Re: git remote rename problem with trailing \\ for remote.url config entries (on Windows)
Johannes Schindelin writes: > A quick test with my `empty-config-section` patch series shows that it > addresses this issue. A quick bisec confirms that the patch with the > online "git_config_set: make use of the config parser's event stream" is > responsible for this fix. > > At first, I was puzzled by this, because I expected `git remote rename` to > be backed by the `git_config_copy_or_rename_section_in_file()` function > (which my patch series does not touch). I played around with this test data: [sec "tio"] val = a\\ ue = b and saw "git config --rename-section sec.tio sec.tion" just work fine (without the event stream change). Without the event stream change, "git config --unset sec.tio.ue" loses "sec.tio.val" but with it we see we only lose the last line, which is the correct thing to happen. Nice.
Re: git remote rename problem with trailing \\ for remote.url config entries (on Windows)
Hi, [I know, blast from the past...] On Mon, 13 Feb 2017, Junio C Hamano wrote: > Marc Strapetz writes: > > > One of our users has just reported that: > > > > $ git remote rename origin origin2 > > > > will turn following remote entry: > > > > [remote "origin"] > > url = c:\\repo\\ > > fetch = +refs/heads/*:refs/remotes/origin/* > > > > into following entry for which the url is skipped: > > > > [remote "origin2"] > > [remote "origin2"] > > fetch = +refs/heads/*:refs/remotes/origin2/* > > > > I understand that this is caused by the trailing \\ and it's easy to > > fix, but 'git push' and 'git pull' work properly with such URLs and a > > > > $ git clone c:\repo\ > > > > will even result in the problematic remote-entry. So I guess some kind > > of validation could be helpful. > > If you change the original definition of the "origin" to > > [remote "origin"] >url = "c:\\repo\\" >fetch = +refs/heads/*:refs/remotes/origin/* > > or > > [remote "origin"] >url = c:\\repo\\ # ends with bs >fetch = +refs/heads/*:refs/remotes/origin/* > > it seems to give me a better result. I didn't dig to determine > where things go wrong in "remote rename", and it is possible that > the problem is isolated to that command, or the same problem exists > with any value that ends with a backslash. If the latter, "git clone" > and anything that writes to configuration such a value needs to be > taught about this glitch and learn a workaround, I would think. > > Dscho Cc'ed, not for Windows expertise, but as somebody who has done > a lot in . So... I finally had a look at this, and while I agree that the quoted version is better, I also agree that the backslash is mistaken for a continuation character (which is not even allowed in section headers). A quick test with my `empty-config-section` patch series shows that it addresses this issue. A quick bisec confirms that the patch with the online "git_config_set: make use of the config parser's event stream" is responsible for this fix. At first, I was puzzled by this, because I expected `git remote rename` to be backed by the `git_config_copy_or_rename_section_in_file()` function (which my patch series does not touch). But then I looked at the code of the `mv()` function in builtin/remote.c and it uses `git_config_set_multivar()` and `git_config_set()`. And those functions were indeed touched (and fixed) by above-mentioned commit. Ciao, Johannes
Re: git-gui ignores core.hooksPath
Chris Maes writes: > Is there any hope from here that anyone will pick up this / these > changes? Will anyone else be assigned the main responsible for this > git-gui repository? > > Just hoping to revive the discussion here, since the > https://github.com/patthoyts/git-gui/ repository seems quite dead. It indeed does. I've played a patch-monkey in the past for git-gui and have a few topics queued still in my tree, but that serves merely as a bookmark that is slightly better than a pointer to the mailing list archive. We need a volunteer to take over this part of the subsystem; somebody who actually uses it, passionate about improving it, and can speak tcl/tk somewhat fluently (I qualify none of these three criteria myself). Any takers?
Re: [PATCH 0/6] Rename files to use dashes instead of underscores
Hi Johannes,
On Tue, Apr 10, 2018 at 3:39 PM, Johannes Schindelin
wrote:
> Hi Stefan,
>
> On Tue, 10 Apr 2018, Stefan Beller wrote:
>
>> This is the followup for
>> https://public-inbox.org/git/xmqqbmer4vfh@gitster-ct.c.googlers.com/
>>
>> We have no files left with underscores in their names.
>
> Yaaay!
>
>> Stefan Beller (6):
>> write_or_die.c: rename to use dashes in file name
>> unicode_width.h: rename to use dash in file name
>> exec_cmd: rename to use dash in file name
>> sha1_name.c: rename to use dash in file name
>> sha1_file.c: rename to use dash in file name
>> replace_object.c: rename to use dash in file name
>
> These are all obviously correct (I did not apply the series and used `git
> grep` to verify that nothing underscored is left there, but I trust you to
> have done that already).
Yes and I did not tell the full story.
There is still 'check_bindir' as compile output, such that I ignored it.
And there was sha1dc_git.{h, c} which I also ignored as it is unclear
to me how our relationship with the DC library is.
I think that could also be converted as it seems to be a shallow wrapper
around that lib.
Re: git-gui branches, was Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
Hi Junio, On Wed, 11 Apr 2018, Junio C Hamano wrote: > Johannes Schindelin writes: > > > On Mon, 9 Apr 2018, Junio C Hamano wrote: > > > >> * bb/git-gui-ssh-key-files (2018-03-02) 2 commits > >> * bp/git-gui-bind-kp-enter (2018-03-02) 2 commits > >> * cb/git-gui-ttk-style (2018-03-05) 2 commits > > > > What is your plan with those? I thought they were on track for v2.17.0, > > but now I see that they are not even in `next`... > > There is no plan. I was waiting for somebody to raise noises, get > irritated by lack of active subsystem maintainer(s), which would > eventually lead us to find a replacement for Pat. > > I can play patch-monkey for git-gui part but I do not want to be the > one who judges if proposed changes to it are good ones. Have they > been reviewed by git-gui competent people? I would call myself "reasonably literate" in Tcl/Tk, not "git-gui competent", but for what it is worth: I remember having reviewed those patches and AFAIR they looked fine enough for inclusion. Certainly the ssk-key-files one. Ciao, Dscho
Re: [PATCH 0/6] Rename files to use dashes instead of underscores
Hi Stefan, On Tue, 10 Apr 2018, Stefan Beller wrote: > This is the followup for > https://public-inbox.org/git/xmqqbmer4vfh@gitster-ct.c.googlers.com/ > > We have no files left with underscores in their names. Yaaay! > Stefan Beller (6): > write_or_die.c: rename to use dashes in file name > unicode_width.h: rename to use dash in file name > exec_cmd: rename to use dash in file name > sha1_name.c: rename to use dash in file name > sha1_file.c: rename to use dash in file name > replace_object.c: rename to use dash in file name These are all obviously correct (I did not apply the series and used `git grep` to verify that nothing underscored is left there, but I trust you to have done that already). Ciao, Dscho
Re: [PATCH 6/8] gpg-interface: find the last gpg signature line
Junio C Hamano writes: >> That test was fixed a week ago: >> https://github.com/git/git/commit/a99d903f21d102a5768f19157085a9733aeb68dd > > Well, you cannot expect any reviewer to know about a change that has > never been sent to the list and has never been part of even 'pu' > branch, no matter how old such a private "fix" is. > > What other unpublished things that are not even on 'pu' do these > patches depend on? Answering my own question after digging a bit more, now I know that a99d903 comes from the 'private' branch in peff/git/ repository hosted at GitHub [*1*]. The branch builds on 'next' by merging many private branches, and 'jk/non-pgp-signatures' branch has that commit. peff.git/private$ git log --oneline --boundary 0c8726318..7a526834e^2 c9ce7c5b7 gpg-interface: handle multiple signing tools 914951682 gpg-interface: handle bool user.signingkey 1f2ea84b3 gpg-interface: return signature type from parse_signature() 6d2ce6547 gpg-interface: prepare for parsing arbitrary PEM blocks fb1d173db gpg-interface: find the last gpg signature line 6bc4e7e17 gpg-interface: extract gpg line matching helper 4f883ac49 gpg-interface: fix const-correctness of "eol" pointer ae6529fdb gpg-interface: use size_t for signature buffer size 1bca4296b gpg-interface: modernize function declarations a99d903f2 t7004: fix mistaken tag name - 468165c1d Git 2.17 It seems to me that Peff did the t7004 change as the earliest preliminary step of the series, but it caused confusion when it was not sent as part of the series by mistake. Judging from the shape of the topic, I do not think this topic has any other hidden dependencies as it builds directly on top of v2.17.0. For those who are reading and reviewing from the sideline, I've attached that missing 0.9/8 patch at the end of this message. [Footnote] *1* I do not know if it deserves to be called a bug, but it certainly is an anomaly GitHub exhibits that a99d903f can be viewed at https://github.com/git/git/commit/a99d903f..., as if it is part of the official git/git history, when it only exists in a fork of that repository. I can understand why it happens but it certainly is unexpected. -- >8 -- From: Jeff King Date: Tue, 3 Apr 2018 17:10:30 -0400 Subject: [PATCH 0.9/8] t7004: fix mistaken tag name We have a series of tests which create signed tags with various properties, but one test accidentally verifies a tag from much earlier in the series. --- t/t7004-tag.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 2aac77af7..ee093b393 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -1056,7 +1056,7 @@ test_expect_success GPG \ git tag -s -F sigblanknonlfile blanknonlfile-signed-tag && get_tag_msg blanknonlfile-signed-tag >actual && test_cmp expect actual && - git tag -v signed-tag + git tag -v blanknonlfile-signed-tag ' # messages with commented lines for signed tags: -- 2.17.0-140-g0b0cc9f867
Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology
Hi Sergey, On Tue, 10 Apr 2018, Sergey Organov wrote: > Johannes Schindelin writes: > > > Once upon a time, I dreamt of an interactive rebase that would not > > flatten branch structure, but instead recreate the commit topology > > faithfully. > > [...] > > > Think of --rebase-merges as "--preserve-merges done right". > > Both option names seem to miss the primary point of the mode of > operation that you've formulated in the first sentence. I suggest to > rather call the new option in accordance to your description, say, > --no-flatten, --keep-topology, or --preserve-shape. A very quick A/B test shows that neither --no-flatten nor --keep-topology and certainly not --preserve-shape conveys to Git users what those options are supposed to do. But --rebase-merges did convey the purpose of my patch series. So there. Ciao, Johannes
Re: [PATCH v1 1/2] fsexcludes: add a programmatic way to exclude files from git's working directory traversal logic
On 10 April 2018 at 23:04, Ben Peart wrote:
> The File System Excludes module is a new programmatic way to exclude files and
> folders from git's traversal of the working directory. fsexcludes_init()
> should
> be called with a string buffer that contains a NUL separated list of path
> names
> of the files and/or directories that should be included. Any path not listed
> will be excluded. The paths should be relative to the root of the working
> directory and be separated by a single NUL.
>
> The excludes logic in dir.c has been updated to honor the results of
> fsexcludes_is_excluded_from(). If fsexcludes does not exclude the file, the
> normal excludes logic is also checked as it could further reduce the set of
> files that should be included.
Here you mention a change in dir.c...
> Makefile | 1 +
> fsexcludes.c | 210 +++
> fsexcludes.h | 27 +++
> 3 files changed, 238 insertions(+)
... but this patch does not seem to touch dir.c at all.
> +static int check_fsexcludes_hashmap(struct hashmap *map, const char
> *pattern, int patternlen)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + struct fsexcludes fse;
> + char *slash;
> +
> + /* Check straight mapping */
> + strbuf_reset(&sb);
You could drop this strbuf_reset(). Or did you intend to use a static
struct strbuf?
> + /*
> +* Check to see if it matches a directory or any path
> +* underneath it. In other words, 'a/b/foo.txt' will match
> +* '/', 'a/', and 'a/b/'.
> +*/
> + slash = strchr(sb.buf, '/');
> + while (slash) {
> + fse.pattern = sb.buf;
> + fse.patternlen = slash - sb.buf + 1;
> + hashmap_entry_init(&fse, fsexcludeshash(fse.pattern,
> fse.patternlen));
> + if (hashmap_get(map, &fse, NULL)) {
> + strbuf_release(&sb);
> + return 0;
> + }
> + slash = strchr(slash + 1, '/');
> + }
Maybe a for-loop would make this slightly more obvious:
for (slash = strchr(sb.buf, '/'); slash; slash = strchr(slash + 1, '/'))
On second thought, maybe not.
> + entry = buf = fsexcludes_data->buf;
> + len = fsexcludes_data->len;
> + for (i = 0; i < len; i++) {
> + if (buf[i] == '\0') {
> + fsexcludes_hashmap_add(map, entry, buf + i - entry);
> + entry = buf + i + 1;
> + }
> + }
> +}
Very minor: I would have found "buf - entry + i" clearer here and later,
but I'm sure you'll find someone of the opposing opinion (e.g.,
yourself). ;-)
> +static int check_directory_hashmap(struct hashmap *map, const char
> *pathname, int pathlen)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + struct fsexcludes fse;
> +
> + /* Check for directory */
> + strbuf_reset(&sb);
Same comment as above about this spurious reset.
> + if (hashmap_get(map, &fse, NULL)) {
> + strbuf_release(&sb);
> + return 0;
> + }
> +
> + strbuf_release(&sb);
> + return 1;
> +}
> +
> +/*
> + * Return 1 for exclude, 0 for include and -1 for undecided.
> + */
> +int fsexcludes_is_excluded_from(struct index_state *istate,
> + const char *pathname, int pathlen, int dtype)
> +{
Will we at some point regret not being able to "return negative on
error"? I guess that would be "-2" or "negative other than -1".
> +void fsexcludes_init(struct strbuf *sb) {
> + fsexcludes_initialized = 1;
> + fsexcludes_data = *sb;
> +}
Grabbing the strbuf's members looks a bit odd. Is this
performance-sensitive enough that you do not want to make a copy? If a
caller releases its strbuf, which would normally be a good thing to do,
we may be in big trouble later. (Not only may .buf be stale, .len may
indicate we actually have something to read.)
I can understand that you do not want to pass a pointer+len, and that it
is not enough to pass sb.buf, since the string may contain nuls.
Maybe detach the original strbuf? That way, if a caller releases its
buffer, that is a no-op. A caller which goes on to use its buffer should
fail quickly and obviously. Right now, an incorrect caller would
probably fail more subtly and less reproducibly.
In any case, maybe document this in the .h-file?
Martin
Re: git-gui ignores core.hooksPath
Hi Chris, On Tue, 10 Apr 2018, Chris Maes wrote: > using git 2.16 the same problem is still present. And probably 2.17, too. > I see that the pull request https://github.com/patthoyts/git-gui/pull/12 > (along with 15 other pull requests) are lying around since about one > year without any sign of life from patthoyts. Yes, this is very sad. I hope he is alive and doing well. As to Git GUI: if you know your way around Tcl/Tk reasonably well, how about stepping up and reviewing those PRs? Even if the PRs are not merged, a review would do those PRs pretty good and we could then take things from there. > Is there any hope from here that anyone will pick up this / these > changes? Will anyone else be assigned the main responsible for this > git-gui repository? There is no "assigning" here, not really. What is missing is a volunteer who earned the trust of the Git developers. Reviewing those PRs would go a long way to earn that trust. > Just hoping to revive the discussion here, since the > https://github.com/patthoyts/git-gui/ repository seems quite dead. Thank you for doing this. I also hope that somebody with reasonable understanding of Tcl/Tk and a vested interest in Git GUI takes up the responsibility of maintaining it. Judging by the rate the PRs trickled into https://github.com/patthoyts/git-gui, I think it would be a minor time commitment. Ciao, Johannes
Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page
Hi Martin,
On Tue, 10 Apr 2018, Martin Ågren wrote:
> On 10 April 2018 at 14:30, Johannes Schindelin
> wrote:
> > The --rebase-merges mode is probably not half as intuitive to use as
> > its inventor hopes, so let's document it some.
>
> I quite like this documentation. Well-structured and well-paced.
> Already after the first reading, I believe I understand how to use this.
Thanks!
> > +The `label` command puts a label to whatever will be the current
> > +revision when that command is executed. Internally, these labels are
> > +worktree-local refs that will be deleted when the rebase finishes or
> > +when it is aborted. That way, rebase operations in multiple worktrees
> > +linked to the same repository do not interfere with one another.
>
> In the above paragraph, you say "internally".
I guess that I should reword this to say "These labels are created as
worktree-local refs (`refs/rewritten/`) that will be ..."
I'll do that, thanks for the sanity check!
> > +At this time, the `merge` command will *always* use the `recursive`
> > +merge strategy, with no way to choose a different one. To work around
> > +this, an `exec` command can be used to call `git merge` explicitly,
> > +using the fact that the labels are worktree-local refs (the ref
> > +`refs/rewritten/onto` would correspond to the label `onto`).
>
> This sort of encourages use of that "internal" detail, which made me a
> little bit surprised at first. But if we can't come up with a reason why
> we would want to change the "refs/rewritten/"-concept later (I
> can't) and if we think the gain this paragraph gives is significant (it
> basically gives access to `git merge` in its entirety), then providing
> this hint might be the correct thing to do.
You are right. I made it sound as if this was an implementation detail
that you should not rely on, when I wanted to say that this is how it is
implemented and you are free to use it in your scripts.
> > +Note: the first command (`reset onto`) labels the revision onto which
> > +the commits are rebased; The name `onto` is just a convention, as a nod
> > +to the `--onto` option.
>
> s/reset onto/label onto/
D'oh!
Thanks, fixed. Current state is in `sequencer-shears` in
https://github.com/dscho/git (I will update the `recreate-merges` branch,
which needs to keep its name so that my scripts will connect the mail
threads for the patch submissions, once I called `git rebase -kir @{u}`).
Ciao,
Dscho
Re: git-gui branches, was Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
On Tue, Apr 10, 2018 at 2:38 PM, Junio C Hamano wrote: > Johannes Schindelin writes: > >> On Mon, 9 Apr 2018, Junio C Hamano wrote: >> >>> * bb/git-gui-ssh-key-files (2018-03-02) 2 commits This looks good to me. Please merge down. >>> * bp/git-gui-bind-kp-enter (2018-03-02) 2 commits I tested and reviewed this, and it also looks good to me. >>> * cb/git-gui-ttk-style (2018-03-05) 2 commits While this looks like it doesn't break anything, and it does what it intends to do, I am not sure if that is the best approach. I'll look into Tcl and experiment to have an opinion. >> What is your plan with those? I thought they were on track for v2.17.0, >> but now I see that they are not even in `next`... > > There is no plan. I was waiting for somebody to raise noises, get > irritated by lack of active subsystem maintainer(s), which would > eventually lead us to find a replacement for Pat. Ok, glad that Johannes made noise then. I am a heavy user of git-gui myself so I would feel sad if it wasn't properly maintained. (On the other hand it "just works" -- for me) I'd be ok to step up as a maintainer there in the long run if nobody else steps up. > I can play patch-monkey for git-gui part but I do not want to be the > one who judges if proposed changes to it are good ones. Have they > been reviewed by git-gui competent people? For now please apply (or monkey-patch as you put it) the first and second. Thanks, Stefan
Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
Derrick Stolee writes: > On 4/9/2018 6:08 PM, Junio C Hamano wrote: >> >> I guess we'd want a final cleaned-up round after all ;-) Thanks. > > v8 sent [1]. Thanks. Thanks, will take a look and queue.
Re: [PATCH v2 1/1] perl: fix installing modules from contrib
Christian Hesse writes: > Commit 20d2a30f (Makefile: replace perl/Makefile.PL with simple make rules) > removed a target that allowed Makefiles from contrib/ to get the correct > install path. This introduces a new target for main Makefile and fixes > installation for Mediawiki module. > > v2: Pass prefix as that can have influence as well, add single quotes > for _SQ variant. > > Signed-off-by: Christian Hesse > --- > Makefile | 2 ++ > contrib/mw-to-git/Makefile | 5 +++-- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/Makefile b/Makefile > index 96f6138f6..19ca5e8de 100644 > --- a/Makefile > +++ b/Makefile > @@ -2011,6 +2011,8 @@ GIT-PERL-DEFINES: FORCE > echo "$$FLAGS" >$@; \ > fi > > +perllibdir: > + @echo '$(perllibdir_SQ)' Sorry for not noticing it before, but as this rule will not create and update timestamp of a filesystem entity 'perllibdir', shouldn't we mark it with .PHONY? I'd call the target 'say-perllibdir' if I were doing this patch but that is merely a personal preference. > .PHONY: gitweb > gitweb: > diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile > index a4b6f7a2c..4e603512a 100644 > --- a/contrib/mw-to-git/Makefile > +++ b/contrib/mw-to-git/Makefile > @@ -21,8 +21,9 @@ HERE=contrib/mw-to-git/ > INSTALL = install > > SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL)) > -INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \ > --s --no-print-directory instlibdir) > +INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/ \ > +-s --no-print-directory prefix=$(prefix) \ > +perllibdir=$(perllibdir) perllibdir) > DESTDIR_SQ = $(subst ','\'',$(DESTDIR)) > INSTLIBDIR_SQ = $(subst ','\'',$(INSTLIBDIR)) >
Re: git-gui branches, was Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
Johannes Schindelin writes: > On Mon, 9 Apr 2018, Junio C Hamano wrote: > >> * bb/git-gui-ssh-key-files (2018-03-02) 2 commits >> * bp/git-gui-bind-kp-enter (2018-03-02) 2 commits >> * cb/git-gui-ttk-style (2018-03-05) 2 commits > > What is your plan with those? I thought they were on track for v2.17.0, > but now I see that they are not even in `next`... There is no plan. I was waiting for somebody to raise noises, get irritated by lack of active subsystem maintainer(s), which would eventually lead us to find a replacement for Pat. I can play patch-monkey for git-gui part but I do not want to be the one who judges if proposed changes to it are good ones. Have they been reviewed by git-gui competent people?
Re: [PATCH 0/6] Rename files to use dashes instead of underscores
On Tue, Apr 10, 2018 at 2:26 PM, Stefan Beller wrote: > This is the followup for > https://public-inbox.org/git/xmqqbmer4vfh@gitster-ct.c.googlers.com/ > > We have no files left with underscores in their names. > It applies on master, and also contains that patch for replace_objects, which I would intend to drop from that other series.
[PATCH 1/6] write_or_die.c: rename to use dashes in file name
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Signed-off-by: Stefan Beller --- Makefile | 2 +- write_or_die.c => write-or-die.c | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename write_or_die.c => write-or-die.c (100%) diff --git a/Makefile b/Makefile index 96f6138f63..be4ac5b2a6 100644 --- a/Makefile +++ b/Makefile @@ -933,7 +933,7 @@ LIB_OBJS += walker.o LIB_OBJS += wildmatch.o LIB_OBJS += worktree.o LIB_OBJS += wrapper.o -LIB_OBJS += write_or_die.o +LIB_OBJS += write-or-die.o LIB_OBJS += ws.o LIB_OBJS += wt-status.o LIB_OBJS += xdiff-interface.o diff --git a/write_or_die.c b/write-or-die.c similarity index 100% rename from write_or_die.c rename to write-or-die.c -- 2.17.0.484.g0c8726318c-goog
[PATCH 4/6] sha1_name.c: rename to use dash in file name
This is more consistent with the project style. The majority of Git's
source files use dashes in preference to underscores in their file names.
Signed-off-by: Stefan Beller
---
Makefile | 2 +-
list-objects-filter.c | 2 +-
object.h | 2 +-
sha1_name.c => sha1-name.c | 0
4 files changed, 3 insertions(+), 3 deletions(-)
rename sha1_name.c => sha1-name.c (100%)
diff --git a/Makefile b/Makefile
index f608c592b7..a54eef2f23 100644
--- a/Makefile
+++ b/Makefile
@@ -898,7 +898,7 @@ LIB_OBJS += setup.o
LIB_OBJS += sha1-array.o
LIB_OBJS += sha1-lookup.o
LIB_OBJS += sha1_file.o
-LIB_OBJS += sha1_name.o
+LIB_OBJS += sha1-name.o
LIB_OBJS += shallow.o
LIB_OBJS += sideband.o
LIB_OBJS += sigchain.o
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 0ec83aaf18..247717561f 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -19,7 +19,7 @@
* in the traversal (until we mark it SEEN). This is a way to
* let us silently de-dup calls to show() in the caller. This
* is subtly different from the "revision.h:SHOWN" and the
- * "sha1_name.c:ONELINE_SEEN" bits. And also different from
+ * "sha1-name.c:ONELINE_SEEN" bits. And also different from
* the non-de-dup usage in pack-bitmap.c
*/
#define FILTER_SHOWN_BUT_REVISIT (1<<21)
diff --git a/object.h b/object.h
index f13f85b2a9..b8e70e5519 100644
--- a/object.h
+++ b/object.h
@@ -37,7 +37,7 @@ struct object_array {
* bundle.c:16
* http-push.c: 16-19
* commit.c:16-19
- * sha1_name.c: 20
+ * sha1-name.c: 20
* list-objects-filter.c: 21
* builtin/fsck.c: 0--3
* builtin/index-pack.c: 2021
diff --git a/sha1_name.c b/sha1-name.c
similarity index 100%
rename from sha1_name.c
rename to sha1-name.c
--
2.17.0.484.g0c8726318c-goog
[PATCH 5/6] sha1_file.c: rename to use dash in file name
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Signed-off-by: Stefan Beller --- Documentation/technical/api-object-access.txt | 2 +- Makefile | 2 +- builtin/index-pack.c | 2 +- sha1_file.c => sha1-file.c| 0 4 files changed, 3 insertions(+), 3 deletions(-) rename sha1_file.c => sha1-file.c (100%) diff --git a/Documentation/technical/api-object-access.txt b/Documentation/technical/api-object-access.txt index a1162e5bcd..5b29622d00 100644 --- a/Documentation/technical/api-object-access.txt +++ b/Documentation/technical/api-object-access.txt @@ -1,7 +1,7 @@ object access API = -Talk about and family, things like +Talk about and family, things like * read_sha1_file() * read_object_with_reference() diff --git a/Makefile b/Makefile index a54eef2f23..d24695f292 100644 --- a/Makefile +++ b/Makefile @@ -897,7 +897,7 @@ LIB_OBJS += server-info.o LIB_OBJS += setup.o LIB_OBJS += sha1-array.o LIB_OBJS += sha1-lookup.o -LIB_OBJS += sha1_file.o +LIB_OBJS += sha1-file.o LIB_OBJS += sha1-name.o LIB_OBJS += shallow.o LIB_OBJS += sideband.o diff --git a/builtin/index-pack.c b/builtin/index-pack.c index e89c039b56..a9b0fefc94 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1592,7 +1592,7 @@ static void read_idx_option(struct pack_idx_option *opts, const char *pack_name) /* * Get rid of the idx file as we do not need it anymore. * NEEDSWORK: extract this bit from free_pack_by_name() in -* sha1_file.c, perhaps? It shouldn't matter very much as we +* sha1-file.c, perhaps? It shouldn't matter very much as we * know we haven't installed this pack (hence we never have * read anything from it). */ diff --git a/sha1_file.c b/sha1-file.c similarity index 100% rename from sha1_file.c rename to sha1-file.c -- 2.17.0.484.g0c8726318c-goog
[PATCH 6/6] replace_object.c: rename to use dash in file name
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Noticed while adding a header corresponding to this file. Signed-off-by: Jonathan Nieder Signed-off-by: Stefan Beller --- Makefile | 2 +- replace_object.c => replace-object.c | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename replace_object.c => replace-object.c (100%) diff --git a/Makefile b/Makefile index d24695f292..6a8168b858 100644 --- a/Makefile +++ b/Makefile @@ -885,7 +885,7 @@ LIB_OBJS += refs/packed-backend.o LIB_OBJS += refs/ref-cache.o LIB_OBJS += ref-filter.o LIB_OBJS += remote.o -LIB_OBJS += replace_object.o +LIB_OBJS += replace-object.o LIB_OBJS += repository.o LIB_OBJS += rerere.o LIB_OBJS += resolve-undo.o diff --git a/replace_object.c b/replace-object.c similarity index 100% rename from replace_object.c rename to replace-object.c -- 2.17.0.484.g0c8726318c-goog
[PATCH 3/6] exec_cmd: rename to use dash in file name
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Signed-off-by: Stefan Beller --- Makefile | 6 +++--- attr.c | 2 +- builtin/add.c| 2 +- builtin/am.c | 2 +- builtin/describe.c | 2 +- builtin/difftool.c | 2 +- builtin/hash-object.c| 2 +- builtin/help.c | 2 +- builtin/index-pack.c | 2 +- builtin/init-db.c| 2 +- builtin/merge-tree.c | 2 +- builtin/notes.c | 2 +- builtin/pull.c | 2 +- builtin/receive-pack.c | 2 +- common-main.c| 2 +- config.c | 2 +- exec_cmd.c => exec-cmd.c | 2 +- exec_cmd.h => exec-cmd.h | 0 fetch-pack.c | 2 +- git.c| 2 +- help.c | 2 +- http-backend.c | 2 +- http-fetch.c | 2 +- http-push.c | 2 +- imap-send.c | 2 +- remote-curl.c| 2 +- remote-testsvn.c | 2 +- run-command.c| 2 +- sequencer.c | 2 +- shell.c | 2 +- upload-pack.c| 2 +- 31 files changed, 32 insertions(+), 32 deletions(-) rename exec_cmd.c => exec-cmd.c (99%) rename exec_cmd.h => exec-cmd.h (100%) diff --git a/Makefile b/Makefile index be4ac5b2a6..f608c592b7 100644 --- a/Makefile +++ b/Makefile @@ -815,7 +815,7 @@ LIB_OBJS += ewah/bitmap.o LIB_OBJS += ewah/ewah_bitmap.o LIB_OBJS += ewah/ewah_io.o LIB_OBJS += ewah/ewah_rlw.o -LIB_OBJS += exec_cmd.o +LIB_OBJS += exec-cmd.o LIB_OBJS += fetch-object.o LIB_OBJS += fetch-pack.o LIB_OBJS += fsck.o @@ -2152,8 +2152,8 @@ else $(OBJECTS): $(LIB_H) endif -exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX -exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ +exec-cmd.sp exec-cmd.s exec-cmd.o: GIT-PREFIX +exec-cmd.sp exec-cmd.s exec-cmd.o: EXTRA_CPPFLAGS = \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ '-DPREFIX="$(prefix_SQ)"' diff --git a/attr.c b/attr.c index dfc3a558d8..03a678fa9b 100644 --- a/attr.c +++ b/attr.c @@ -10,7 +10,7 @@ #define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "config.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "attr.h" #include "dir.h" #include "utf8.h" diff --git a/builtin/add.c b/builtin/add.c index 9ef7fb02d5..c9e2619a9a 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -9,7 +9,7 @@ #include "lockfile.h" #include "dir.h" #include "pathspec.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "cache-tree.h" #include "run-command.h" #include "parse-options.h" diff --git a/builtin/am.c b/builtin/am.c index 1bcc3606c5..269743ff76 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -6,7 +6,7 @@ #include "cache.h" #include "config.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "dir.h" #include "run-command.h" diff --git a/builtin/describe.c b/builtin/describe.c index de840f96a4..b5afc45846 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -6,7 +6,7 @@ #include "blob.h" #include "refs.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "revision.h" #include "diff.h" diff --git a/builtin/difftool.c b/builtin/difftool.c index ee8dce019e..aad0e073ee 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -15,7 +15,7 @@ #include "config.h" #include "builtin.h" #include "run-command.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "argv-array.h" #include "strbuf.h" diff --git a/builtin/hash-object.c b/builtin/hash-object.c index 526da5c185..a9a3a198c3 100644 --- a/builtin/hash-object.c +++ b/builtin/hash-object.c @@ -9,7 +9,7 @@ #include "blob.h" #include "quote.h" #include "parse-options.h" -#include "exec_cmd.h" +#include "exec-cmd.h" /* * This is to create corrupt objects for debugging and as such it diff --git a/builtin/help.c b/builtin/help.c index 598867cfea..2d51071429 100644 --- a/builtin/help.c +++ b/builtin/help.c @@ -4,7 +4,7 @@ #include "cache.h" #include "config.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #include "run-command.h" #include "column.h" diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 657a5dda06..e89c039b56 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -9,7 +9,7 @@ #include "tree.h" #include "progress.h" #include "fsck.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "streaming.h" #include "thread-utils.h" #include "packfile.h" diff --git a/builtin/init-db.c b/builtin/init-db.c index 68ff4ad75a..2542c5244e 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -7,7 +7,7 @@ #include "config.h" #include "refs.h" #include "builtin.h" -#include "exec_cmd.h" +#include "exec-cmd.h" #include "parse-options.h" #
[PATCH 0/6] Rename files to use dashes instead of underscores
This is the followup for https://public-inbox.org/git/xmqqbmer4vfh@gitster-ct.c.googlers.com/ We have no files left with underscores in their names. Thanks, Stefan Stefan Beller (6): write_or_die.c: rename to use dashes in file name unicode_width.h: rename to use dash in file name exec_cmd: rename to use dash in file name sha1_name.c: rename to use dash in file name sha1_file.c: rename to use dash in file name replace_object.c: rename to use dash in file name Documentation/technical/api-object-access.txt | 2 +- Makefile | 14 +++--- attr.c| 2 +- builtin/add.c | 2 +- builtin/am.c | 2 +- builtin/describe.c| 2 +- builtin/difftool.c| 2 +- builtin/hash-object.c | 2 +- builtin/help.c| 2 +- builtin/index-pack.c | 4 ++-- builtin/init-db.c | 2 +- builtin/merge-tree.c | 2 +- builtin/notes.c | 2 +- builtin/pull.c| 2 +- builtin/receive-pack.c| 2 +- common-main.c | 2 +- config.c | 2 +- contrib/update-unicode/README | 6 +++--- contrib/update-unicode/update_unicode.sh | 2 +- exec_cmd.c => exec-cmd.c | 2 +- exec_cmd.h => exec-cmd.h | 0 fetch-pack.c | 2 +- git.c | 2 +- help.c| 2 +- http-backend.c| 2 +- http-fetch.c | 2 +- http-push.c | 2 +- imap-send.c | 2 +- list-objects-filter.c | 2 +- object.h | 2 +- remote-curl.c | 2 +- remote-testsvn.c | 2 +- replace_object.c => replace-object.c | 0 run-command.c | 2 +- sequencer.c | 2 +- sha1_file.c => sha1-file.c| 0 sha1_name.c => sha1-name.c| 0 shell.c | 2 +- unicode_width.h => unicode-width.h| 0 upload-pack.c | 2 +- utf8.c| 2 +- write_or_die.c => write-or-die.c | 0 42 files changed, 45 insertions(+), 45 deletions(-) rename exec_cmd.c => exec-cmd.c (99%) rename exec_cmd.h => exec-cmd.h (100%) rename replace_object.c => replace-object.c (100%) rename sha1_file.c => sha1-file.c (100%) rename sha1_name.c => sha1-name.c (100%) rename unicode_width.h => unicode-width.h (100%) rename write_or_die.c => write-or-die.c (100%) -- 2.17.0.484.g0c8726318c-goog
[PATCH 2/6] unicode_width.h: rename to use dash in file name
This is more consistent with the project style. The majority of Git's source files use dashes in preference to underscores in their file names. Also adjust contrib/update-unicode as well. Signed-off-by: Stefan Beller --- contrib/update-unicode/README| 6 +++--- contrib/update-unicode/update_unicode.sh | 2 +- unicode_width.h => unicode-width.h | 0 utf8.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename unicode_width.h => unicode-width.h (100%) diff --git a/contrib/update-unicode/README b/contrib/update-unicode/README index b9e2fc8540..151a197041 100644 --- a/contrib/update-unicode/README +++ b/contrib/update-unicode/README @@ -1,10 +1,10 @@ TL;DR: Run update_unicode.sh after the publication of a new Unicode -standard and commit the resulting unicode_widths.h file. +standard and commit the resulting unicode-widths.h file. The long version -The Git source code ships the file unicode_widths.h which contains +The Git source code ships the file unicode-widths.h which contains tables of zero and double width Unicode code points, respectively. These tables are generated using update_unicode.sh in this directory. update_unicode.sh itself uses a third-party tool, uniset, to query two @@ -16,5 +16,5 @@ This requires a current-ish version of autoconf (2.69 works per December On each run, update_unicode.sh checks whether more recent Unicode data files are available from the Unicode consortium, and rebuilds the header -unicode_widths.h with the new data. The new header can then be +unicode-widths.h with the new data. The new header can then be committed. diff --git a/contrib/update-unicode/update_unicode.sh b/contrib/update-unicode/update_unicode.sh index e05db92d3f..aa90865bef 100755 --- a/contrib/update-unicode/update_unicode.sh +++ b/contrib/update-unicode/update_unicode.sh @@ -6,7 +6,7 @@ #Cf Format a format control character # cd "$(dirname "$0")" -UNICODEWIDTH_H=$(git rev-parse --show-toplevel)/unicode_width.h +UNICODEWIDTH_H=$(git rev-parse --show-toplevel)/unicode-width.h wget -N http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt \ http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt && diff --git a/unicode_width.h b/unicode-width.h similarity index 100% rename from unicode_width.h rename to unicode-width.h diff --git a/utf8.c b/utf8.c index 2c27ce0137..4419055b48 100644 --- a/utf8.c +++ b/utf8.c @@ -81,7 +81,7 @@ static int git_wcwidth(ucs_char_t ch) /* * Sorted list of non-overlapping intervals of non-spacing characters, */ -#include "unicode_width.h" +#include "unicode-width.h" /* test for 8-bit control characters */ if (ch == 0) -- 2.17.0.484.g0c8726318c-goog
Re: [PATCH 6/6] doc: add note about shell quoting to revision.txt
Andreas Heiduk writes: > Signed-off-by: Andreas Heiduk > --- > Documentation/revisions.txt | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt > index dfcc49c72c..c1d3a40a90 100644 > --- a/Documentation/revisions.txt > +++ b/Documentation/revisions.txt > @@ -7,6 +7,10 @@ syntax. Here are various ways to spell object names. The > ones listed near the end of this list name trees and > blobs contained in a commit. > > +NOTE: This document shows the "raw" syntax as seen by git. The shell > +and other UIs might require additional quoting to protect special > +characters and to avoid word splitting. > + > '', e.g. 'dae86e1950b1277e545cee180551750029cfe735', 'dae86e':: >The full SHA-1 object name (40-byte hexadecimal string), or >a leading substring that is unique within the repository. > @@ -186,6 +190,8 @@ existing tag object. >is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a >literal '!' character, followed by 'foo'. Any other sequence beginning with >':/!' is reserved for now. > + Depending on the given text the shell's word splitting rules might > + require additional quoting. > > ':', e.g. 'HEAD:README', ':README', 'master:./README':: >A suffix ':' followed by a path names the blob or tree I've seen this suggested before and thought it is a good idea. GOod to see it is finally happening ;-) Thanks.
Re: [PATCH 1/6] doc: fix formatting inconsistency in githooks.txt
Martin Ågren writes: > Your reading seems correct, so I was wrong in my speculation. My guess > is such a patch would be welcome. I checked a couple of man-pages and > this one seems particularly heavy on 'git foo' as opposed to `git foo`. > I think that's a reason to fix it, not to leave it behind. Sounds sensible. Hopefully there isn't a topic in flight that wants to change this file, so it may be a good time to do a wholesale cleanup of it.
Re: [PATCH 2/6] doc: align 'diff --no-index' in text with synopsis
Andreas Heiduk writes: > > -'git diff' --no-index [--options] [--] [...]:: > +'git diff' [--options] [--no-index] [--] :: > > This form is to compare the given two paths on the > filesystem. You can omit the `--no-index` option when It definitely is a good change to show two (and only two) on the command line as non-optional arguments. I however find the change to mark that -"-no-index" is optional is inviting more confusion in the form presented in this patch. It is optional under specific conditions, and that is not conveyed with these two path arguments named very genericly (as opposed to making it clear that they are paths that are not managed by Git) on the example command line. I have a suspicion that it would be safer to have the description say under what condition "--no-index" becomes optional (which our text already does), without marking it as if it is always optional like this patch does (i.e. do not lose [] around it from this line). I dunno.
[PATCH v1 1/2] fsexcludes: add a programmatic way to exclude files from git's working directory traversal logic
The File System Excludes module is a new programmatic way to exclude files and
folders from git's traversal of the working directory. fsexcludes_init() should
be called with a string buffer that contains a NUL separated list of path names
of the files and/or directories that should be included. Any path not listed
will be excluded. The paths should be relative to the root of the working
directory and be separated by a single NUL.
The excludes logic in dir.c has been updated to honor the results of
fsexcludes_is_excluded_from(). If fsexcludes does not exclude the file, the
normal excludes logic is also checked as it could further reduce the set of
files that should be included.
Signed-off-by: Ben Peart
---
Makefile | 1 +
fsexcludes.c | 210 +++
fsexcludes.h | 27 +++
3 files changed, 238 insertions(+)
create mode 100644 fsexcludes.c
create mode 100644 fsexcludes.h
diff --git a/Makefile b/Makefile
index 96f6138f63..c102d2f75a 100644
--- a/Makefile
+++ b/Makefile
@@ -819,6 +819,7 @@ LIB_OBJS += exec_cmd.o
LIB_OBJS += fetch-object.o
LIB_OBJS += fetch-pack.o
LIB_OBJS += fsck.o
+LIB_OBJS += fsexcludes.o
LIB_OBJS += fsmonitor.o
LIB_OBJS += gettext.o
LIB_OBJS += gpg-interface.o
diff --git a/fsexcludes.c b/fsexcludes.c
new file mode 100644
index 00..07bfe376a0
--- /dev/null
+++ b/fsexcludes.c
@@ -0,0 +1,210 @@
+#include "cache.h"
+#include "fsexcludes.h"
+#include "hashmap.h"
+#include "strbuf.h"
+
+static int fsexcludes_initialized = 0;
+static struct strbuf fsexcludes_data = STRBUF_INIT;
+static struct hashmap fsexcludes_hashmap;
+static struct hashmap parent_directory_hashmap;
+
+struct fsexcludes {
+ struct hashmap_entry ent; /* must be the first member! */
+ const char *pattern;
+ int patternlen;
+};
+
+static unsigned int(*fsexcludeshash)(const void *buf, size_t len);
+static int(*fsexcludescmp)(const char *a, const char *b, size_t len);
+
+static int fsexcludes_hashmap_cmp(const void *unused_cmp_data,
+ const void *a, const void *b, const void *key)
+{
+ const struct fsexcludes *fse1 = a;
+ const struct fsexcludes *fse2 = b;
+
+ return fsexcludescmp(fse1->pattern, fse2->pattern, fse1->patternlen);
+}
+
+static int check_fsexcludes_hashmap(struct hashmap *map, const char *pattern,
int patternlen)
+{
+ struct strbuf sb = STRBUF_INIT;
+ struct fsexcludes fse;
+ char *slash;
+
+ /* Check straight mapping */
+ strbuf_reset(&sb);
+ strbuf_add(&sb, pattern, patternlen);
+ fse.pattern = sb.buf;
+ fse.patternlen = sb.len;
+ hashmap_entry_init(&fse, fsexcludeshash(fse.pattern, fse.patternlen));
+ if (hashmap_get(map, &fse, NULL)) {
+ strbuf_release(&sb);
+ return 0;
+ }
+
+ /*
+* Check to see if it matches a directory or any path
+* underneath it. In other words, 'a/b/foo.txt' will match
+* '/', 'a/', and 'a/b/'.
+*/
+ slash = strchr(sb.buf, '/');
+ while (slash) {
+ fse.pattern = sb.buf;
+ fse.patternlen = slash - sb.buf + 1;
+ hashmap_entry_init(&fse, fsexcludeshash(fse.pattern,
fse.patternlen));
+ if (hashmap_get(map, &fse, NULL)) {
+ strbuf_release(&sb);
+ return 0;
+ }
+ slash = strchr(slash + 1, '/');
+ }
+
+ strbuf_release(&sb);
+ return 1;
+}
+
+static void fsexcludes_hashmap_add(struct hashmap *map, const char *pattern,
const int patternlen)
+{
+ struct fsexcludes *fse;
+
+ fse = xmalloc(sizeof(struct fsexcludes));
+ fse->pattern = pattern;
+ fse->patternlen = patternlen;
+ hashmap_entry_init(fse, fsexcludeshash(fse->pattern, fse->patternlen));
+ hashmap_add(map, fse);
+}
+
+static void initialize_fsexcludes_hashmap(struct hashmap *map, struct strbuf
*fsexcludes_data)
+{
+ char *buf, *entry;
+ size_t len;
+ int i;
+
+ /*
+* Build a hashmap of the fsexcludes data we can use to look
+* for cache entry matches quickly
+*/
+ fsexcludeshash = ignore_case ? memihash : memhash;
+ fsexcludescmp = ignore_case ? strncasecmp : strncmp;
+ hashmap_init(map, fsexcludes_hashmap_cmp, NULL, 0);
+
+ entry = buf = fsexcludes_data->buf;
+ len = fsexcludes_data->len;
+ for (i = 0; i < len; i++) {
+ if (buf[i] == '\0') {
+ fsexcludes_hashmap_add(map, entry, buf + i - entry);
+ entry = buf + i + 1;
+ }
+ }
+}
+
+static void parent_directory_hashmap_add(struct hashmap *map, const char
*pattern, const int patternlen)
+{
+ char *slash;
+ struct fsexcludes *fse;
+
+ /*
+* Add any directories leading up to the file as the excludes logic
+* needs to match directories leading up to the files as we
Re: [PATCH 6/8] gpg-interface: find the last gpg signature line
Ben Toews writes: >> H, what vintage of our codebase is this patch based on? Did I >> miss a patch that removes these lines >> >> >> printf ' ' >sigblanknonlfile >> get_tag_header blanknonlfile-signed-tag $commit commit $time >expect >> echo '-BEGIN PGP SIGNATURE-' >>expect >> test_expect_success GPG \ >> 'creating a signed tag with spaces and no newline should >> succeed' ' >> git tag -s -F sigblanknonlfile blanknonlfile-signed-tag && >> get_tag_msg blanknonlfile-signed-tag >actual && >> test_cmp expect actual && >> git tag -v signed-tag >> ' >> >> which appear between the pre- and post- context of the lines you are >> inserting? They date back to 2007-2009. >> > > That test was fixed a week ago: > https://github.com/git/git/commit/a99d903f21d102a5768f19157085a9733aeb68dd Well, you cannot expect any reviewer to know about a change that has never been sent to the list and has never been part of even 'pu' branch, no matter how old such a private "fix" is. What other unpublished things that are not even on 'pu' do these patches depend on?
[PATCH v1 0/2] fsexcludes: Add programmatic way to exclude files
In git repos with large working directories an external file system monitor (like fsmonitor or gvfs) can track what files in the working directory have been modified. This information can be used to speed up git operations that scale based on the size of the working directory so that they become O(# of modified files) vs O(# of files in the working directory). The fsmonitor patch series added logic to limit what files git had to stat() to the set of modified files provided by the fsmonitor hook proc. It also used the untracked cache (if enabled) to limit the files/folders git had to scan looking for new/untracked files. GVFS is another external file system model that also speeds up git working directory based operations that has been using a different mechanism (programmatically generating an excludes file) to enable git to be O(# of modified files). This patch series will introduce a new way to limit git�s traversal of the working directory that does not require the untracked cache (fsmonitor) or using the excludes feature (GVFS). It does this by enhancing the existing excludes logic in dir.c to support a new �File System Excludes� or fsexcludes API that is better tuned to these programmatic applications. Base Ref: master Web-Diff: https://github.com/benpeart/git/commit/2ccbcd6360 Checkout: git fetch https://github.com/benpeart/git fsexcludes-v1 && git checkout 2ccbcd6360 Ben Peart (2): fsexcludes: add a programmatic way to exclude files from git's working directory traversal logic fsmonitor: switch to use new fsexcludes logic and remove unused untracked cache based logic Makefile| 1 + dir.c | 33 -- dir.h | 2 - fsexcludes.c| 210 fsexcludes.h| 27 + fsmonitor.c | 21 +--- fsmonitor.h | 10 +- t/t7519-status-fsmonitor.sh | 14 +-- 8 files changed, 270 insertions(+), 48 deletions(-) create mode 100644 fsexcludes.c create mode 100644 fsexcludes.h base-commit: 0b0cc9f86731f894cff8dd25299a9b38c254569e -- 2.17.0.windows.1
[PATCH v1 2/2] fsmonitor: switch to use new fsexcludes logic and remove unused untracked cache based logic
Update fsmonitor to utilize the new fsexcludes based logic for excluding paths
that do not need to be scaned for new or modified files. Remove the old logic
in dir.c that utilized the untracked cache (if enabled) to accomplish the same
goal.
Signed-off-by: Ben Peart
---
dir.c | 33 -
dir.h | 2 --
fsmonitor.c | 21 ++---
fsmonitor.h | 10 +++---
t/t7519-status-fsmonitor.sh | 14 +++---
5 files changed, 32 insertions(+), 48 deletions(-)
diff --git a/dir.c b/dir.c
index 63a917be45..28c2c83f76 100644
--- a/dir.c
+++ b/dir.c
@@ -18,7 +18,7 @@
#include "utf8.h"
#include "varint.h"
#include "ewah/ewok.h"
-#include "fsmonitor.h"
+#include "fsexcludes.h"
/*
* Tells read_directory_recursive how a file or directory should be treated.
@@ -1102,6 +1102,12 @@ int is_excluded_from_list(const char *pathname,
struct exclude_list *el, struct index_state *istate)
{
struct exclude *exclude;
+
+ if (*dtype == DT_UNKNOWN)
+ *dtype = get_dtype(NULL, istate, pathname, pathlen);
+ if (fsexcludes_is_excluded_from(istate, pathname, pathlen, *dtype) > 0)
+ return 1;
+
exclude = last_exclude_matching_from_list(pathname, pathlen, basename,
dtype, el, istate);
if (exclude)
@@ -1317,8 +1323,15 @@ struct exclude *last_exclude_matching(struct dir_struct
*dir,
int is_excluded(struct dir_struct *dir, struct index_state *istate,
const char *pathname, int *dtype_p)
{
- struct exclude *exclude =
- last_exclude_matching(dir, istate, pathname, dtype_p);
+ struct exclude *exclude;
+ int pathlen = strlen(pathname);
+
+ if (*dtype_p == DT_UNKNOWN)
+ *dtype_p = get_dtype(NULL, istate, pathname, pathlen);
+ if (fsexcludes_is_excluded_from(istate, pathname, pathlen, *dtype_p) >
0)
+ return 1;
+
+ exclude = last_exclude_matching(dir, istate, pathname, dtype_p);
if (exclude)
return exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;
return 0;
@@ -1671,6 +1684,9 @@ static enum path_treatment treat_one_path(struct
dir_struct *dir,
if (dtype != DT_DIR && has_path_in_index)
return path_none;
+ if (fsexcludes_is_excluded_from(istate, path->buf, path->len, dtype) >
0)
+ return path_excluded;
+
/*
* When we are looking at a directory P in the working tree,
* there are three cases:
@@ -1810,12 +1826,9 @@ static int valid_cached_dir(struct dir_struct *dir,
if (!untracked)
return 0;
- /*
-* With fsmonitor, we can trust the untracked cache's valid field.
-*/
- refresh_fsmonitor(istate);
- if (!(dir->untracked->use_fsmonitor && untracked->valid)) {
- if (lstat(path->len ? path->buf : ".", &st)) {
+ if (!untracked->valid) {
+ if (stat(path->len ? path->buf : ".", &st)) {
+ invalidate_directory(dir->untracked, untracked);
memset(&untracked->stat_data, 0,
sizeof(untracked->stat_data));
return 0;
}
@@ -2011,6 +2024,8 @@ static enum path_treatment
read_directory_recursive(struct dir_struct *dir,
/* add the path to the appropriate result list */
switch (state) {
case path_excluded:
+ if (fsexcludes_is_excluded_from(istate, path.buf,
path.len, DTYPE(cdir.de)) > 0)
+ break;
if (dir->flags & DIR_SHOW_IGNORED)
dir_add_name(dir, istate, path.buf, path.len);
else if ((dir->flags & DIR_SHOW_IGNORED_TOO) ||
diff --git a/dir.h b/dir.h
index b0758b82a2..e67ccfbb29 100644
--- a/dir.h
+++ b/dir.h
@@ -139,8 +139,6 @@ struct untracked_cache {
int gitignore_invalidated;
int dir_invalidated;
int dir_opened;
- /* fsmonitor invalidation data */
- unsigned int use_fsmonitor : 1;
};
struct dir_struct {
diff --git a/fsmonitor.c b/fsmonitor.c
index 6d7bcd5d0e..dd67eef851 100644
--- a/fsmonitor.c
+++ b/fsmonitor.c
@@ -2,6 +2,7 @@
#include "config.h"
#include "dir.h"
#include "ewah/ewok.h"
+#include "fsexcludes.h"
#include "fsmonitor.h"
#include "run-command.h"
#include "strbuf.h"
@@ -125,12 +126,7 @@ static void fsmonitor_refresh_callback(struct index_state
*istate, const char *n
ce->ce_flags &= ~CE_FSMONITOR_VALID;
}
- /*
-* Mark the untracked cache dirty even if it wasn't found in the index
-* as it could be a new untracked file.
-*/
trace_printf_key(&trace_fsmonitor, "fsmonitor_refresh_callback '%s'",
name);
- untracked_cache_invalidate_path
git-gui branches, was Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
Hi Junio, On Mon, 9 Apr 2018, Junio C Hamano wrote: > * bb/git-gui-ssh-key-files (2018-03-02) 2 commits > - Merge branch 'bb/ssh-key-files' of git-gui into bb/git-gui-ssh-key-files > - git-gui: search for all current SSH key types > > "git gui" learned that "~/.ssh/id_ecdsa.pub" and > "~/.ssh/id_ed25519.pub" are also possible SSH key files. > > > * bp/git-gui-bind-kp-enter (2018-03-02) 2 commits > - Merge branch 'bp/bind-kp-enter' of git-gui into bp/git-gui-bind-kp-enter > - git-gui: bind CTRL/CMD+numpad ENTER to do_commit > > "git gui" performs commit upon CTRL/CMD+ENTER but the > CTRL/CMD+KP_ENTER (i.e. enter key on the numpad) did not have the > same key binding. It now does. > > > * cb/git-gui-ttk-style (2018-03-05) 2 commits > - Merge branch 'cb/ttk-style' of git-gui into cb/git-gui-ttk-style > - git-gui: workaround ttk:style theme use > > "git gui" has been taught to work with old versions of tk (like > 8.5.7) that do not support "ttk::style theme use" as a way to query > the current theme. What is your plan with those? I thought they were on track for v2.17.0, but now I see that they are not even in `next`... Ciao, Dscho
Re: [PATCH 1/6] doc: fix formatting inconsistency in githooks.txt
On 10 April 2018 at 22:04, Andreas Heiduk wrote:
> Am 10.04.2018 um 21:13 schrieb Martin Ågren:
>> On 10 April 2018 at 20:32, Andreas Heiduk wrote:
>> Hmm, I wonder if that is actually intentional. `git commit --amend`
>> could be run exactly like that and would do what this paragraph expects
>> of it. The 'git-rebase' is a Git subcommand name, i.e., not some
>> copy-paste command-line ready for use. If it were something like `git
>> rebase -i HEAD~5`, I would expect the backticks.
>
> That page mostly uses single quotes and no dash ('git send-email')for
> formatting. Reading 'CodingGuidelines' my understanding is, that git
> commands should be typeset with backticks, no dash (`git send-email`).
> So 'git-rebase' (an similar) *should* be typeset as `git rebase`. But
> doing so consistently would be a full-diff for this manual page.
>
> Should I do this?
Your reading seems correct, so I was wrong in my speculation. My guess
is such a patch would be welcome. I checked a couple of man-pages and
this one seems particularly heavy on 'git foo' as opposed to `git foo`.
I think that's a reason to fix it, not to leave it behind.
Martin
Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
On 10/04/18 21:22, Ramsay Jones wrote: > > > On 10/04/18 20:35, Derrick Stolee wrote: >> On 4/10/2018 3:21 PM, Ramsay Jones wrote: >>> >>> On 10/04/18 13:57, Derrick Stolee wrote: On 4/9/2018 6:08 PM, Junio C Hamano wrote: > I guess we'd want a final cleaned-up round after all ;-) Thanks. v8 sent [1]. Thanks. >>> I just tried to 'git am' this series and I could not get it >>> to apply cleanly to current 'master', 'next' or 'pu'. I fixed >>> up a few patches, but just got bored ... ;-) >>> >> >> In my cover letter, I did mention that my patch started here (using >> 'format-patch --base'): >> >> base-commit: 468165c1d8a442994a825f3684528361727cd8c0 >> >> >> This corresponds to v2.17.0. > > Yep, I forgot to mention that I had already tried that too: > > $ git log --oneline -1 > 468165c1d (HEAD -> master-graph, tag: v2.17.0, origin/maint, maint, build) > Git 2.17 > $ git am patches/* > Applying: csum-file: rename hashclose() to finalize_hashfile() > Applying: csum-file: refactor finalize_hashfile() method > Applying: commit-graph: add format document > Applying: graph: add commit graph design document > Applying: commit-graph: create git-commit-graph builtin > Applying: commit-graph: create git-commit-graph builtin > error: patch failed: .gitignore:34 > error: .gitignore: patch does not apply > error: Documentation/git-commit-graph.txt: already exists in index > error: patch failed: Makefile:952 > error: Makefile: patch does not apply > error: patch failed: builtin.h:149 > error: builtin.h: patch does not apply > error: builtin/commit-graph.c: already exists in index > error: patch failed: command-list.txt:34 > error: command-list.txt: patch does not apply > error: patch failed: contrib/completion/git-completion.bash:878 > error: contrib/completion/git-completion.bash: patch does not apply > error: patch failed: git.c:388 > error: git.c: patch does not apply > Patch failed at 0006 commit-graph: create git-commit-graph builtin > Use 'git am --show-current-patch' to see the failed patch > When you have resolved this problem, run "git am --continue". > If you prefer to skip this patch, run "git am --skip" instead. > To restore the original branch and stop patching, run "git am --abort". > $ Ah, OK, it helps if you don't have an edited patch backup file in the patches directory! patches now apply cleanly. sparse is also now clean - thanks! ATB, Ramsay Jones
Inbox SMTP, Inbox Webmail, I Sell Sure Spamming Toolz
I Sell Sure Spamming Toolz What we have on Stock Daily Inbox Webmail Inbox SMTP Fresh USA email leads Fresh Canada email leads Fresh Loan email leads Fresh Business emails leads Real Eastate email leads Conference delegates email leads Fresh Job Seaker emails cPanel HTTP and HTTPs Shell Zip/Unzipp Mailer RDP All ScamPages Bank ScamPage Add me on whatsapp or call me Watsapp: +2348107268246 Only Real buyers
[PATCH v2 1/1] completion: load completion file for external subcommand
Adding external subcommands to Git is as easy as to put an executable
file git-foo into PATH. Packaging such subcommands for a Linux
distribution can be achieved by unpacking the executable into /usr/bin
of the user's system. Adding system-wide completion scripts for new
subcommands, however, can be a bit tricky.
Since bash-completion started to use dynamical loading of completion
scripts since v1.90 (preview of v2.0), it is no longer sufficient to
drop a completion script of a subcommand into the standard completions
path, /usr/share/bash-completion/completions, since this script will not
be loaded if called as a git subcommand.
For example, look at https://bugs.gentoo.org/544722. To give a short
summary: The popular git-flow subcommand provides a completion script,
which gets installed as /usr/share/bash-completion/completions/git-flow.
If you now type into a Bash shell:
git flow
You will not get any completions, because bash-completion only loads
completions for git and git has no idea that git-flow is defined in
another file. You have to load this script manually or trigger the
dynamic loader with:
git-flow # Please notice the dash instead of whitespace
This will not complete anything either, because it only defines a Bash
function, without generating completions. But now the correct completion
script has been loaded and the first command can use the completions.
So, the goal is now to teach the git completion script to consider the
possibility of external completion scripts for subcommands, but of
course without breaking current workflows.
I think the easiest method is to use a function that is defined by
bash-completion v2.0+, namely __load_completion. It will take care of
loading the correct script if present. Afterwards, the git completion
script behaves as usual.
This way we can leverage bash-completion's dynamic loading for git
subcommands and make it easier for developers to distribute custom
completion scripts.
Signed-off-by: Florian Gamböck
---
contrib/completion/git-completion.bash | 10 ++
1 file changed, 10 insertions(+)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index b09c8a236..09a820990 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3096,12 +3096,22 @@ __git_main ()
fi
local completion_func="_git_${command//-/_}"
+ if ! declare -f $completion_func >/dev/null 2>/dev/null &&
+ declare -f __load_completion >/dev/null 2>/dev/null
+ then
+ __load_completion "git-$command"
+ fi
declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
&& return
local expansion=$(__git_aliased_command "$command")
if [ -n "$expansion" ]; then
words[1]=$expansion
completion_func="_git_${expansion//-/_}"
+ if ! declare -f $completion_func >/dev/null 2>/dev/null &&
+ declare -f __load_completion >/dev/null 2>/dev/null
+ then
+ __load_completion "git-$expansion"
+ fi
declare -f $completion_func >/dev/null 2>/dev/null &&
$completion_func
fi
}
--
2.16.1
[PATCH v2 0/1] completion: dynamic completion loading
In this small patch I want to introduce a way to dynamically load completion scripts for external subcommands. A few years ago, you would put a completion script (which defines a Bash function _git_foo for a custom git subcommand foo) into /etc/bash_completion.d, or save it somewhere in your $HOME and source it manually in your .bashrc. Starting with bash-completion v2.0 (or, to be absolutely exact, the preview versions started at v1.90), completion scripts are not sourced at bash startup anymore. Rather, when typing in a command and trigger completion by pressing the TAB key, the new bash-completion's main script looks for a script with the same name as the typed command in the completion directory, sources it, and provides the completions defined in this script. However, that only works for top level commands. After a completion script has been found, the logic is delegated to this script. This means, that the completion of subcommands must be handled by the corresponding completion script. As it is now, git is perfectly able to call custom completion functions, iff they are already defined when calling the git completion. With my patch, git uses an already defined loader function of bash-completion (or continues silently if this function is not found), loads an external completion script, and provides its completions. An example for an external completion script would be /usr/share/bash-completion/completions/git-foo for a git subcommand foo. Changes since v1 (RFC): - Re-arranged if-fi statement to increase readability (suggested by Junio C Hamano) - Re-worded commit message to include the exakt version of bashcomp that introduced dynamic loading (suggested by Stefan Beller) Florian Gamböck (1): completion: load completion file for external subcommand contrib/completion/git-completion.bash | 10 ++ 1 file changed, 10 insertions(+) -- 2.16.1
Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
On 10/04/18 20:35, Derrick Stolee wrote: > On 4/10/2018 3:21 PM, Ramsay Jones wrote: >> >> On 10/04/18 13:57, Derrick Stolee wrote: >>> On 4/9/2018 6:08 PM, Junio C Hamano wrote: I guess we'd want a final cleaned-up round after all ;-) Thanks. >>> v8 sent [1]. Thanks. >> I just tried to 'git am' this series and I could not get it >> to apply cleanly to current 'master', 'next' or 'pu'. I fixed >> up a few patches, but just got bored ... ;-) >> > > In my cover letter, I did mention that my patch started here (using > 'format-patch --base'): > > base-commit: 468165c1d8a442994a825f3684528361727cd8c0 > > > This corresponds to v2.17.0. Yep, I forgot to mention that I had already tried that too: $ git log --oneline -1 468165c1d (HEAD -> master-graph, tag: v2.17.0, origin/maint, maint, build) Git 2.17 $ git am patches/* Applying: csum-file: rename hashclose() to finalize_hashfile() Applying: csum-file: refactor finalize_hashfile() method Applying: commit-graph: add format document Applying: graph: add commit graph design document Applying: commit-graph: create git-commit-graph builtin Applying: commit-graph: create git-commit-graph builtin error: patch failed: .gitignore:34 error: .gitignore: patch does not apply error: Documentation/git-commit-graph.txt: already exists in index error: patch failed: Makefile:952 error: Makefile: patch does not apply error: patch failed: builtin.h:149 error: builtin.h: patch does not apply error: builtin/commit-graph.c: already exists in index error: patch failed: command-list.txt:34 error: command-list.txt: patch does not apply error: patch failed: contrib/completion/git-completion.bash:878 error: contrib/completion/git-completion.bash: patch does not apply error: patch failed: git.c:388 error: git.c: patch does not apply Patch failed at 0006 commit-graph: create git-commit-graph builtin Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". $ ATB, Ramsay Jones
Re: [PATCH v1] fsmonitor: fix incorrect buffer size when printing version number
On Tue, Apr 10, 2018 at 2:43 PM, Ben Peart wrote: > This is a trivial bug fix for passing the incorrect size to snprintf() when > outputing the version. It should be passing the size of the destination > buffer s/outputing/outputting/ > rather than the size of the value being printed. > > Signed-off-by: Ben Peart
Re: [PATCH 1/6] doc: fix formatting inconsistency in githooks.txt
Am 10.04.2018 um 21:13 schrieb Martin Ågren:
> On 10 April 2018 at 20:32, Andreas Heiduk wrote:
>> The section 'post-rewrite' in 'githooks.txt' renders only one command
>> using backticks (`git commit`) but the other commands using single quotes
>> ('git-rebase'). Align this formatting to use single quotes.
>>
>> Signed-off-by: Andreas Heiduk
>> ---
>> Documentation/githooks.txt | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index f877f7b7cd..070e745b41 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -417,8 +417,8 @@ to abort.
>> post-rewrite
>>
>>
>> -This hook is invoked by commands that rewrite commits (`git commit
>> ---amend`, 'git-rebase'; currently 'git-filter-branch' does 'not' call
>> +This hook is invoked by commands that rewrite commits ('git commit
>> +--amend', 'git-rebase'; currently 'git-filter-branch' does 'not' call
>> it!). Its first argument denotes the command it was invoked by:
>> currently one of `amend` or `rebase`. Further command-dependent
>> arguments may be passed in the future.
>
> Hmm, I wonder if that is actually intentional. `git commit --amend`
> could be run exactly like that and would do what this paragraph expects
> of it. The 'git-rebase' is a Git subcommand name, i.e., not some
> copy-paste command-line ready for use. If it were something like `git
> rebase -i HEAD~5`, I would expect the backticks.
That page mostly uses single quotes and no dash ('git send-email')for
formatting. Reading 'CodingGuidelines' my understanding is, that git
commands should be typeset with backticks, no dash (`git send-email`).
So 'git-rebase' (an similar) *should* be typeset as `git rebase`. But
doing so consistently would be a full-diff for this manual page.
Should I do this?
>
> A second discrepancy is the dash in "git commit" vs "git-rebase" and
> "git-ls-remote". That could perhaps be explained by the same reasoning.
>
> Martin
>
Re: [PATCH 4/6] doc: added '-d' and '-q' for 'git push'
On 10 April 2018 at 21:38, Andreas Heiduk wrote: > Can I add "Reviewed-by: $YOU" to this one and 2/6? Sure!
Re: [PATCH 4/6] doc: added '-d' and '-q' for 'git push'
Am 10.04.2018 um 21:17 schrieb Martin Ågren:
> On 10 April 2018 at 20:32, Andreas Heiduk wrote:
>> Add the missing `-o` shortcut for `--push-option` to the synposis.
>> Add the missing `-d` shortcut for `--delete` in the main section.
>
> s/synposis/synopsis/
>
> The subject of this patch says -q, which should be -o. The subject
> could also be in imperative ("doc: add ...", or "doc: add missing ...").
> The diff LGTM.
>
> Martin
>
ACK & Thanks,
Can I add "Reviewed-by: $YOU" to this one and 2/6?
Andreas
Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
On 4/10/2018 3:21 PM, Ramsay Jones wrote: On 10/04/18 13:57, Derrick Stolee wrote: On 4/9/2018 6:08 PM, Junio C Hamano wrote: I guess we'd want a final cleaned-up round after all ;-) Thanks. v8 sent [1]. Thanks. I just tried to 'git am' this series and I could not get it to apply cleanly to current 'master', 'next' or 'pu'. I fixed up a few patches, but just got bored ... ;-) In my cover letter, I did mention that my patch started here (using 'format-patch --base'): base-commit: 468165c1d8a442994a825f3684528361727cd8c0 This corresponds to v2.17.0. Thanks, -Stolee
Re: [PATCH 2/6] doc: align 'diff --no-index' in text with synopsis
Am 10.04.2018 um 21:14 schrieb Martin Ågren: > On 10 April 2018 at 20:32, Andreas Heiduk wrote: >> Comparing > > That first line should probably not be there. The diff LGTM. > > Martin > ACK, Thanks
Re: Great Investment Offer
Hello In my search for a business partner i got your contact in google search. My client is willing to invest $10 Million to $500 million but my client said he need a trusted partner who he can have a meeting at the point of releasing his funds. I told my client that you have a good profile with your company which i got details about you on my search on google lookup. Can we trust you. Can we make a plan for a long term business relationship. Please reply. Regards, Gagum Melvin Sikze Kakha Tel: 703197576
Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
On 10/04/18 13:57, Derrick Stolee wrote: > On 4/9/2018 6:08 PM, Junio C Hamano wrote: >> >> I guess we'd want a final cleaned-up round after all ;-) Thanks. > > v8 sent [1]. Thanks. I just tried to 'git am' this series and I could not get it to apply cleanly to current 'master', 'next' or 'pu'. I fixed up a few patches, but just got bored ... ;-) ATB, Ramsay Jones
Re: [PATCH v8 03/14] commit-graph: add format document
On 4/10/2018 3:10 PM, Stefan Beller wrote:
Hi Derrick,
On Tue, Apr 10, 2018 at 5:55 AM, Derrick Stolee wrote:
+ OID Fanout (ID: {'O', 'I', 'D', 'F'}) (256 * 4 bytes)
+ The ith entry, F[i], stores the number of OIDs with first
+ byte at most i. Thus F[255] stores the total
+ number of commits (N).
I was about to give this series one last read not expecting any questions
to come up (this series has had a lot of feedback already!)
Although I just did.
What were your design considerations for the fanout table?
Did you include it as the pack index has one or did you come up with
them from first principles?
Have you measured the performance impact of the fanout table
(maybe even depending on the size of the fanout) ?
context:
https://public-inbox.org/git/CAJo=hJsto1ik=gtc8c3+2_jbuuqcapl0uwp-1uoyympgblb...@mail.gmail.com/
(side note: searching the web for fanout makes it seem
as if it is git-lingo, apparently the term is not widely used)
I don't think we want to restart the design discussion,
I am just curious.
I knew that I wanted some amount of a fanout table, and the 256-entry
one was used for IDX files (and in my MIDX RFC). With the recent
addition of "packfile: refactor hash search with fanout table" [1] it is
probably best to keep the 256-entry table to reduce code clones.
As for speed, we have the notion of 'graph_pos' which gives random
access into the commit-graph after a commit is loaded as a parent of a
commit from the commit-graph file. Thus, we are spending time in the
binary search only for commits that do not exist in the commit-graph
file and those that are first found in the file. Thus, running profilers
on long commit-graph walks do not show any measurable time spent in
'bsearch_graph()'.
Thanks,
-Stolee
[1]
https://github.com/gitster/git/commit/b4e00f7306a160639f047b3421985e8f3d0c6fb1
Re: [PATCH 4/6] doc: added '-d' and '-q' for 'git push'
On 10 April 2018 at 20:32, Andreas Heiduk wrote:
> Add the missing `-o` shortcut for `--push-option` to the synposis.
> Add the missing `-d` shortcut for `--delete` in the main section.
s/synposis/synopsis/
The subject of this patch says -q, which should be -o. The subject
could also be in imperative ("doc: add ...", or "doc: add missing ...").
The diff LGTM.
Martin
Re: [PATCH 2/6] doc: align 'diff --no-index' in text with synopsis
On 10 April 2018 at 20:32, Andreas Heiduk wrote: > Comparing > The two '' parameters are not optional but the option > '--no-index' is. Also move the `--options` part to the same > place where the other variants show them. That first line should probably not be there. The diff LGTM. Martin
Re: [PATCH 1/6] doc: fix formatting inconsistency in githooks.txt
On 10 April 2018 at 20:32, Andreas Heiduk wrote:
> The section 'post-rewrite' in 'githooks.txt' renders only one command
> using backticks (`git commit`) but the other commands using single quotes
> ('git-rebase'). Align this formatting to use single quotes.
>
> Signed-off-by: Andreas Heiduk
> ---
> Documentation/githooks.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index f877f7b7cd..070e745b41 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -417,8 +417,8 @@ to abort.
> post-rewrite
>
>
> -This hook is invoked by commands that rewrite commits (`git commit
> ---amend`, 'git-rebase'; currently 'git-filter-branch' does 'not' call
> +This hook is invoked by commands that rewrite commits ('git commit
> +--amend', 'git-rebase'; currently 'git-filter-branch' does 'not' call
> it!). Its first argument denotes the command it was invoked by:
> currently one of `amend` or `rebase`. Further command-dependent
> arguments may be passed in the future.
Hmm, I wonder if that is actually intentional. `git commit --amend`
could be run exactly like that and would do what this paragraph expects
of it. The 'git-rebase' is a Git subcommand name, i.e., not some
copy-paste command-line ready for use. If it were something like `git
rebase -i HEAD~5`, I would expect the backticks.
A second discrepancy is the dash in "git commit" vs "git-rebase" and
"git-ls-remote". That could perhaps be explained by the same reasoning.
Martin
Re: What's cooking in git.git (Apr 2018, #01; Mon, 9)
On Tue, Apr 10, 2018 at 5:57 AM, Derrick Stolee wrote: > On 4/9/2018 6:08 PM, Junio C Hamano wrote: >> >> >> I guess we'd want a final cleaned-up round after all ;-) Thanks. > > > v8 sent [1]. Thanks. > > -Stolee I gave the series another read and think it is ready. Stefan
Re: [PATCH v8 03/14] commit-graph: add format document
Hi Derrick,
On Tue, Apr 10, 2018 at 5:55 AM, Derrick Stolee wrote:
> + OID Fanout (ID: {'O', 'I', 'D', 'F'}) (256 * 4 bytes)
> + The ith entry, F[i], stores the number of OIDs with first
> + byte at most i. Thus F[255] stores the total
> + number of commits (N).
I was about to give this series one last read not expecting any questions
to come up (this series has had a lot of feedback already!)
Although I just did.
What were your design considerations for the fanout table?
Did you include it as the pack index has one or did you come up with
them from first principles?
Have you measured the performance impact of the fanout table
(maybe even depending on the size of the fanout) ?
context:
https://public-inbox.org/git/CAJo=hJsto1ik=gtc8c3+2_jbuuqcapl0uwp-1uoyympgblb...@mail.gmail.com/
(side note: searching the web for fanout makes it seem
as if it is git-lingo, apparently the term is not widely used)
I don't think we want to restart the design discussion,
I am just curious.
Thanks,
Stefan
Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page
On 10 April 2018 at 14:30, Johannes Schindelin wrote: > The --rebase-merges mode is probably not half as intuitive to use as > its inventor hopes, so let's document it some. I quite like this documentation. Well-structured and well-paced. Already after the first reading, I believe I understand how to use this. > +The `label` command puts a label to whatever will be the current > +revision when that command is executed. Internally, these labels are > +worktree-local refs that will be deleted when the rebase finishes or > +when it is aborted. That way, rebase operations in multiple worktrees > +linked to the same repository do not interfere with one another. In the above paragraph, you say "internally". > +At this time, the `merge` command will *always* use the `recursive` > +merge strategy, with no way to choose a different one. To work around > +this, an `exec` command can be used to call `git merge` explicitly, > +using the fact that the labels are worktree-local refs (the ref > +`refs/rewritten/onto` would correspond to the label `onto`). This sort of encourages use of that "internal" detail, which made me a little bit surprised at first. But if we can't come up with a reason why we would want to change the "refs/rewritten/"-concept later (I can't) and if we think the gain this paragraph gives is significant (it basically gives access to `git merge` in its entirety), then providing this hint might be the correct thing to do. > +Note: the first command (`reset onto`) labels the revision onto which > +the commits are rebased; The name `onto` is just a convention, as a nod > +to the `--onto` option. s/reset onto/label onto/ Martin
[PATCH v1] fsmonitor: fix incorrect buffer size when printing version number
This is a trivial bug fix for passing the incorrect size to snprintf() when outputing the version. It should be passing the size of the destination buffer rather than the size of the value being printed. Signed-off-by: Ben Peart --- Notes: Base Ref: v2.17.0 Web-Diff: https://github.com/benpeart/git/commit/0bc3fc3b74 Checkout: git fetch https://github.com/benpeart/git fsmonitor-version-v1 && git checkout 0bc3fc3b74 fsmonitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fsmonitor.c b/fsmonitor.c index 6d7bcd5d0e..eb4e642256 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -104,7 +104,7 @@ static int query_fsmonitor(int version, uint64_t last_update, struct strbuf *que if (!(argv[0] = core_fsmonitor)) return -1; - snprintf(ver, sizeof(version), "%d", version); + snprintf(ver, sizeof(ver), "%d", version); snprintf(date, sizeof(date), "%" PRIuMAX, (uintmax_t)last_update); argv[1] = ver; argv[2] = date; base-commit: 468165c1d8a442994a825f3684528361727cd8c0 -- 2.17.0.windows.1
[PATCH 6/6] doc: add note about shell quoting to revision.txt
Signed-off-by: Andreas Heiduk --- Documentation/revisions.txt | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index dfcc49c72c..c1d3a40a90 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -7,6 +7,10 @@ syntax. Here are various ways to spell object names. The ones listed near the end of this list name trees and blobs contained in a commit. +NOTE: This document shows the "raw" syntax as seen by git. The shell +and other UIs might require additional quoting to protect special +characters and to avoid word splitting. + '', e.g. 'dae86e1950b1277e545cee180551750029cfe735', 'dae86e':: The full SHA-1 object name (40-byte hexadecimal string), or a leading substring that is unique within the repository. @@ -186,6 +190,8 @@ existing tag object. is matched. ':/!-foo' performs a negative match, while ':/!!foo' matches a literal '!' character, followed by 'foo'. Any other sequence beginning with ':/!' is reserved for now. + Depending on the given text the shell's word splitting rules might + require additional quoting. ':', e.g. 'HEAD:README', ':README', 'master:./README':: A suffix ':' followed by a path names the blob or tree -- 2.16.2
[PATCH 5/6] git-svn: commit-diff does not support --add-author-from
Signed-off-by: Andreas Heiduk --- Documentation/git-svn.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt index 636e09048e..11aefadf7a 100644 --- a/Documentation/git-svn.txt +++ b/Documentation/git-svn.txt @@ -700,7 +700,7 @@ creating the branch or tag. config key: svn.useLogAuthor --add-author-from:: - When committing to svn from Git (as part of 'commit-diff', 'set-tree' or 'dcommit' + When committing to svn from Git (as part of 'set-tree' or 'dcommit' operations), if the existing log message doesn't already have a `From:` or `Signed-off-by:` line, append a `From:` line based on the Git commit's author string. If you use this, then `--use-log-author` -- 2.16.2
Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
Hi Michael, On Tue, Apr 10, 2018 at 7:02 AM, Michael Haggerty wrote: > This also makes sense to me, as far as it goes. I have a few comments > and questions: > > Why do you call the new member `main_ref_store`? Is there expected to be > some other `ref_store` associated with a repository? I'll rename it in a reroll. > > I think the origin of the name `main_ref_store` was to distinguish it > from submodule ref stores. But presumably those will soon become the > "main" ref stores for their respective submodule repository objects, > right? I hope so. > So maybe calling things `repository.ref_store` and > `get_ref_store(repository)` would be appropriate. ok. > > There are some places in the reference code that only work with the main > repository. The ones that I can think of are: > > * `ref_resolves_to_object()` depends on an object store. > > * `peel_object()` and `ref_iterator_peel()` also have to look up objects > (in this case, tag objects). > > * Anything that calls `files_assert_main_repository()` or depends on > `REF_STORE_MAIN` isn't implemented for other reference stores (usually, > I think, these are functions that depend on the object store). > > Some of these things might be easy to generalize to non-main > repositories, but I didn't bother because AFAIK currently only the main > repository store is ever mutated. > > You can move a now-obsolete comment above the definition of `struct > files_ref_store` if you haven't in some other patch (search for > "libification"). ok, I'll have a look at that. My plan was to remove the submodule accessors from the refs API, and mandate the access via repo_submodule_init(&submodule_repo, superproject_repo, path); sub_ref_store = get_ref_store(submodule_repo); instead of also having sub_ref_store = get_submodule_ref_store(path); as that would ease the refs API (and its internals potentially) as well as avoiding errors with mixing up repositories. As the construction of a submodule repository struct requires its superproject repo, it helps avoiding pitfalls with nested submodules IMO. Thanks for the comments, Stefan > > Hope that helps, > Michael
[PATCH 1/6] doc: fix formatting inconsistency in githooks.txt
The section 'post-rewrite' in 'githooks.txt' renders only one command
using backticks (`git commit`) but the other commands using single quotes
('git-rebase'). Align this formatting to use single quotes.
Signed-off-by: Andreas Heiduk
---
Documentation/githooks.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index f877f7b7cd..070e745b41 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -417,8 +417,8 @@ to abort.
post-rewrite
-This hook is invoked by commands that rewrite commits (`git commit
---amend`, 'git-rebase'; currently 'git-filter-branch' does 'not' call
+This hook is invoked by commands that rewrite commits ('git commit
+--amend', 'git-rebase'; currently 'git-filter-branch' does 'not' call
it!). Its first argument denotes the command it was invoked by:
currently one of `amend` or `rebase`. Further command-dependent
arguments may be passed in the future.
--
2.16.2
[PATCH 2/6] doc: align 'diff --no-index' in text with synopsis
Comparing The two '' parameters are not optional but the option '--no-index' is. Also move the `--options` part to the same place where the other variants show them. All three items are already correct in the synopsis. Signed-off-by: Andreas Heiduk --- Documentation/git-diff.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index b0c1bb95c8..ee1c509bd3 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -29,7 +29,7 @@ two blob objects, or changes between two files on disk. further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. -'git diff' --no-index [--options] [--] [...]:: +'git diff' [--options] [--no-index] [--] :: This form is to compare the given two paths on the filesystem. You can omit the `--no-index` option when -- 2.16.2
[PATCH 3/6] doc: clarify ignore rules for git ls-files
Explain that `git ls-files --ignored` requires at least one of the `--exclude*` options to do its job. Signed-off-by: Andreas Heiduk --- Documentation/git-ls-files.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt index 3ac3e3a77d..f3474b2ede 100644 --- a/Documentation/git-ls-files.txt +++ b/Documentation/git-ls-files.txt @@ -53,7 +53,8 @@ OPTIONS Show only ignored files in the output. When showing files in the index, print only those matched by an exclude pattern. When showing "other" files, show only those matched by an exclude - pattern. + pattern. Standard ignore rules are not automatically activated, + therefore at least one of the `--exclude*` options is required. -s:: --stage:: -- 2.16.2
[PATCH 4/6] doc: added '-d' and '-q' for 'git push'
Add the missing `-o` shortcut for `--push-option` to the synposis. Add the missing `-d` shortcut for `--delete` in the main section. Signed-off-by: Andreas Heiduk --- Documentation/git-push.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 5b08302fc2..f2bbda6e32 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -11,7 +11,7 @@ SYNOPSIS [verse] 'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | --dry-run] [--receive-pack=] [--repo=] [-f | --force] [-d | --delete] [--prune] [-v | --verbose] - [-u | --set-upstream] [--push-option=] + [-u | --set-upstream] [-o | --push-option=] [--[no-]signed|--signed=(true|false|if-asked)] [--force-with-lease[=[:]]] [--no-verify] [ [...]] @@ -123,6 +123,7 @@ already exists on the remote side. will be tab-separated and sent to stdout instead of stderr. The full symbolic names of the refs will be given. +-d:: --delete:: All listed refs are deleted from the remote repository. This is the same as prefixing all refs with a colon. -- 2.16.2
[PATCH 0/6] Some doc-fixes
I'm flushing a queue of small fixes to the docs. Handling these indiviually is just to much hassle - at least I hope so :-) Andreas Heiduk (6): doc: fix formatting inconsistency in githooks.txt doc: align 'diff --no-index' in text with synopsis doc: clarify ignore rules for git ls-files doc: added '-d' and '-q' for 'git push' git-svn: commit-diff does not support --add-author-from doc: add note about shell quoting to revision.txt Documentation/git-diff.txt | 2 +- Documentation/git-ls-files.txt | 3 ++- Documentation/git-push.txt | 3 ++- Documentation/git-svn.txt | 2 +- Documentation/githooks.txt | 4 ++-- Documentation/revisions.txt| 6 ++ 6 files changed, 14 insertions(+), 6 deletions(-) -- 2.16.2
Re: [PATCH 07/16] refs: add repository argument to get_main_ref_store
Hi Michael, On Tue, Apr 10, 2018 at 6:36 AM, Michael Haggerty wrote: > On 04/10/2018 12:45 AM, Stefan Beller wrote: >> Add a repository argument to allow the get_main_ref_store caller >> to be more specific about which repository to handle. This is a small >> mechanical change; it doesn't change the implementation to handle >> repositories other than the_repository yet. >> >> As with the previous commits, use a macro to catch callers passing a >> repository other than the_repository at compile time. > > This seems OK to me from a refs perspective. > > The macro trick is surprising. I guess it gets you a compile-time check, > under the assumption that nothing else is called `the_repository`. Yes. Credit goes to Jonathan Tan for this trick. > But > why actually commit the macro, as opposed to compiling once locally to > check for correctness, then maybe add something like `assert(r == > the_repository)` for the actual commit? The eternal struggle of contributing patches that are easy to review. ;) With the assert we'll have a run time check, which is not desirable compared to a compile time check. And from a reviewers point of view running a "rebase -x make" on the series that Junio queued is easier than to reason about the "assert(r = the_repository)" IMHO. > But I don't care either way, since the macro disappears again soon. Glad you're ok with this approach. Thanks for looking at the refs specific code, Stefan
[PATCH v1] fsmonitor: force index write after full scan
fsmonitor currently only flags the index as dirty if the extension is being added or removed. This is a performance optimization that recognizes you can stat() a lot of files in less time than it takes to write out an updated index. This patch makes a small enhancement and flags the index dirty if we end up having to stat() all files and scan the entire working directory. The assumption being that must be expensive or you would not have turned on the feature. Signed-off-by: Ben Peart --- Notes: Base Ref: Web-Diff: https://github.com/benpeart/git/commit/4d39ddc2f9 Checkout: git fetch https://github.com/benpeart/git fsmonitor-perf-v1 && git checkout 4d39ddc2f9 fsmonitor.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fsmonitor.c b/fsmonitor.c index eb4e642256..ed3d1a074d 100644 --- a/fsmonitor.c +++ b/fsmonitor.c @@ -185,6 +185,9 @@ void refresh_fsmonitor(struct index_state *istate) for (i = 0; i < istate->cache_nr; i++) istate->cache[i]->ce_flags &= ~CE_FSMONITOR_VALID; + /* If we're going to check every file, ensure we save the results */ + istate->cache_changed |= FSMONITOR_CHANGED; + if (istate->untracked) istate->untracked->use_fsmonitor = 0; } base-commit: 0ac4dfac5000ef8de7966a3b7229275567d2d707 -- 2.17.0.windows.1
Re: [PATCH 02/16] replace_object.c: rename to use dash in file name
Hi Junio,
On Mon, Apr 9, 2018 at 8:00 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> This is more consistent with the project style. The majority of
>> Git's source files use dashes in preference to underscores in their file
>> names.
>>
>> Noticed while adding a header corresponding to this file.
>>
>> Signed-off-by: Jonathan Nieder
>> Signed-off-by: Stefan Beller
>> ---
>
> Hmph, is this authored by Jonathan?
This one (as well as other patches that are also signed off by Jonathan)
was cherry-picked from the long series[1], which was done partially
in a pair programming session or by passing patches back and forth.
Most of the mechanical changes were done by one author and we
just added the others sign off to have the whole series look like pair
programming.
This patch is [2], as-is, so I did not mess up the original authorship.
We decided to not use another trailer, such as co-authored-by or such
as we'd have to sign off anyway.
[1] https://public-inbox.org/git/20180205235508.216277-1-sbel...@google.com/
[2] https://public-inbox.org/git/20180205235735.216710-19-sbel...@google.com/
>
> There are sha1_{file,name}.c, exec_cmd.[ch], and unicode_width.h
> remaining, though ;-)
Noted.
Thanks,
Stefan
Re: [PATCH 8/8] gpg-interface: handle alternative signature types
On Tue, Apr 10, 2018 at 3:35 AM, Junio C Hamano wrote: > Ben Toews writes: > >> From: Ben Toews >> >> Currently you can only sign commits and tags using "gpg". >> ... >> have asked before on the list about using OpenBSD signify). >> --- > > Missing sign-off. > >> -gpg.program:: >> - Use this custom program instead of "`gpg`" found on `$PATH` when >> - making or verifying a PGP signature. The program must support the >> - same command-line interface as GPG, namely, to verify a detached >> - signature, "`gpg --verify $file - <$signature`" is run, and the >> - program is expected to signal a good signature by exiting with >> - code 0, and to generate an ASCII-armored detached signature, the >> - standard input of "`gpg -bsau $key`" is fed with the contents to be >> +signingtool..program:: >> + The name of the program on `$PATH` to execute when making or >> + verifying a signature. > > I think you do not want "on `$PATH`", as you should be able to > specify a full path /opt/some/where/not/on/my/path/pgp and have it > work just fine. The mention of "found on `$PATH`" in the original > is talking about the behaviour _WITHOUT_ the configuration, i.e. by > default we just invoke "gpg" and expect that it is found in the > usual measure, i.e. being on user's $PATH. What you are describing > in this updated explanation is what happens _WITH_ the configuration. > >> + This program will be used for making >> + signatures if `` is configured as `signingtool.default`. >> + This program will be used for verifying signatures whose PEM >> + block type matches `signingtool..pemtype` (see below). The >> + program must support the same command-line interface as GPG. >> + To verify a detached signature, >> + "`gpg --verify $file - <$signature`" is run, and the program is >> + expected to signal a good signature by exiting with code 0. >> + To generate an ASCII-armored detached signature, the standard >> + input of "`gpg -bsau $key`" is fed with the contents to be >> signed, and the program is expected to send the result to its >> - standard output. >> + standard output. By default, `signingtool.gpg.program` is set to >> + `gpg`. > > I do not think the description is wrong per-se, but reading it made > me realize that with this "custom" program, you still require that > the "custom" program MUST accept the command line options as if it > were an implementation of GPG. Most likely you'd write a thin > wrapper to call your custom program with whatever options that are > appropriate when asked to --verify or -bsau (aka "sign")? If that > is the case, I have to wonder if such a wrappper program can also > trivially reformat the --- BEGIN WHATEVER --- block and behave as if > it were an implementation of GPG. That makes the primary point of > this long series somewhat moot, as we won't need that pemtype thing > at all, no? > Just because a signature is PEM encoded and claims to be a "PGP SIGNATURE", doesn't mean it can be understood or verified by a PGP implementation. Without different tools specifying different PEM types we would have no way of knowing which tool to route the signature to for verification. >> +signingtool..pemtype:: >> + The PEM block type associated with the signing tool named >> + ``. For example, the block type of a GPG signature >> + starting with `-BEGIN PGP SIGNATURE-` is `PGP >> + SIGNATURE`. When verifying a signature with this PEM block type >> + the program specified in `signingtool..program` will be >> + used. By default `signingtool.gpg.pemtype` contains `PGP >> + SIGNATURE` and `PGP MESSAGE`. > > As Eric noted elsewhere, I suspect that it is cleaner and more > useful if this were *NOT* "pemtype" but were "boundary", i.e. > letting "-BEGIN PGP SIGNATURE-\n" string be specified. > >> +signingtool.default:: >> + The `` of the signing tool to use when creating >> + signatures (e.g., setting it to "foo" will use use the program >> + specified by `signingtool.foo.program`). Defaults to `gpg`. > > Will there be a command line option to say "I may usually be using > whatever I configured with signingtool.default, but for this single > invocation only, let me use something else"? Without such a command > line option that overrides such a default, I do not quite get the > point of adding this configuration variable. > > Thanks. -- -Ben Toews
[PATCH v8 5/5] mingw/msvc: use the new-style RUNTIME_PREFIX helper
From: Johannes Schindelin This change also allows us to stop overriding argv[0] with the absolute path of the executable, allowing us to preserve e.g. the case of the executable's file name. This fixes https://github.com/git-for-windows/git/issues/1496 partially. Signed-off-by: Johannes Schindelin --- compat/mingw.c | 5 ++--- config.mak.uname | 2 ++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index a67872bab..6ded1c859 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2221,7 +2221,7 @@ void mingw_startup(void) die_startup(); /* determine size of argv and environ conversion buffer */ - maxlen = wcslen(_wpgmptr); + maxlen = wcslen(wargv[0]); for (i = 1; i < argc; i++) maxlen = max(maxlen, wcslen(wargv[i])); for (i = 0; wenv[i]; i++) @@ -2241,8 +2241,7 @@ void mingw_startup(void) buffer = malloc_startup(maxlen); /* convert command line arguments and environment to UTF-8 */ - __argv[0] = wcstoutfdup_startup(buffer, _wpgmptr, maxlen); - for (i = 1; i < argc; i++) + for (i = 0; i < argc; i++) __argv[i] = wcstoutfdup_startup(buffer, wargv[i], maxlen); for (i = 0; wenv[i]; i++) environ[i] = wcstoutfdup_startup(buffer, wenv[i], maxlen); diff --git a/config.mak.uname b/config.mak.uname index e1cfe5e5e..a6e734c5d 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -357,6 +357,7 @@ ifeq ($(uname_S),Windows) SNPRINTF_RETURNS_BOGUS = YesPlease NO_SVN_TESTS = YesPlease RUNTIME_PREFIX = YesPlease + HAVE_WPGMPTR = YesWeDo NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease NO_NSEC = YesPlease USE_WIN32_MMAP = YesPlease @@ -506,6 +507,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_SVN_TESTS = YesPlease NO_PERL_MAKEMAKER = YesPlease RUNTIME_PREFIX = YesPlease + HAVE_WPGMPTR = YesWeDo NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease NO_NSEC = YesPlease USE_WIN32_MMAP = YesPlease -- 2.15.0.chromium12
[PATCH v8 1/5] Makefile: generate Perl header from template file
Currently, the generated Perl script headers are emitted by commands in
the Makefile. This mechanism restricts options to introduce alternative
header content, needed by Perl runtime prefix support, and obscures the
origin of the Perl script header.
Change the Makefile to generate a header by processing a template file and
move the header content into the "perl/" subdirectory. The generated
header content will now be stored in the "GIT-PERL-HEADER" file. This
allows the content of the Perl header to be controlled by changing the path
of the template in the Makefile.
Signed-off-by: Dan Jacques
Thanks-to: Ævar Arnfjörð Bjarmason
Thanks-to: Johannes Schindelin
---
.gitignore | 1 +
Makefile | 27 +++---
perl/header_templates/fixed_prefix.template.pl | 1 +
3 files changed, 18 insertions(+), 11 deletions(-)
create mode 100644 perl/header_templates/fixed_prefix.template.pl
diff --git a/.gitignore b/.gitignore
index 833ef3b0b..89bd7bd8a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,7 @@
/GIT-LDFLAGS
/GIT-PREFIX
/GIT-PERL-DEFINES
+/GIT-PERL-HEADER
/GIT-PYTHON-VARS
/GIT-SCRIPT-DEFINES
/GIT-USER-AGENT
diff --git a/Makefile b/Makefile
index 96f6138f6..ec7cf5a0f 100644
--- a/Makefile
+++ b/Makefile
@@ -1984,20 +1984,15 @@ git.res: git.rc GIT-VERSION-FILE
$(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS
ifndef NO_PERL
-$(SCRIPT_PERL_GEN):
-
+PERL_HEADER_TEMPLATE = perl/header_templates/fixed_prefix.template.pl
PERL_DEFINES = $(PERL_PATH_SQ):$(PERLLIB_EXTRA_SQ):$(perllibdir_SQ)
-$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-VERSION-FILE
+
+$(SCRIPT_PERL_GEN): % : %.perl GIT-PERL-DEFINES GIT-PERL-HEADER
GIT-VERSION-FILE
$(QUIET_GEN)$(RM) $@ $@+ && \
- INSTLIBDIR='$(perllibdir_SQ)' && \
- INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
- INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
sed -e '1{' \
-e 's|#!.*perl|#!$(PERL_PATH_SQ)|' \
- -e 'h' \
- -e 's=.*=use lib (split(/$(pathsep)/, $$ENV{GITPERLLIB} ||
"'"$$INSTLIBDIR"'"));=' \
- -e 'H' \
- -e 'x' \
+ -e 'rGIT-PERL-HEADER' \
+ -e 'G' \
-e '}' \
-e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
$< >$@+ && \
@@ -2011,6 +2006,16 @@ GIT-PERL-DEFINES: FORCE
echo "$$FLAGS" >$@; \
fi
+GIT-PERL-HEADER: $(PERL_HEADER_TEMPLATE) GIT-PERL-DEFINES Makefile
+ $(QUIET_GEN)$(RM) $@ && \
+ INSTLIBDIR='$(perllibdir_SQ)' && \
+ INSTLIBDIR_EXTRA='$(PERLLIB_EXTRA_SQ)' && \
+ INSTLIBDIR="$$INSTLIBDIR$${INSTLIBDIR_EXTRA:+:$$INSTLIBDIR_EXTRA}" && \
+ sed -e 's=@@PATHSEP@@=$(pathsep)=g' \
+ -e 's=@@INSTLIBDIR@@='$$INSTLIBDIR'=g' \
+ -e 's=@@PERLLIBDIR@@='$(perllibdir_SQ)'=g' \
+ $< >$@+ && \
+ mv $@+ $@
.PHONY: gitweb
gitweb:
@@ -2788,7 +2793,7 @@ ifndef NO_TCLTK
endif
$(RM) GIT-VERSION-FILE GIT-CFLAGS GIT-LDFLAGS GIT-BUILD-OPTIONS
$(RM) GIT-USER-AGENT GIT-PREFIX
- $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PYTHON-VARS
+ $(RM) GIT-SCRIPT-DEFINES GIT-PERL-DEFINES GIT-PERL-HEADER
GIT-PYTHON-VARS
.PHONY: all install profile-clean clean strip
.PHONY: shell_compatibility_test please_set_SHELL_PATH_to_a_more_modern_shell
diff --git a/perl/header_templates/fixed_prefix.template.pl
b/perl/header_templates/fixed_prefix.template.pl
new file mode 100644
index 0..857b4391a
--- /dev/null
+++ b/perl/header_templates/fixed_prefix.template.pl
@@ -0,0 +1 @@
+use lib (split(/@@PATHSEP@@/, $ENV{GITPERLLIB} || '@@INSTLIBDIR@@'));
--
2.15.0.chromium12
[PATCH v8 3/5] exec_cmd: RUNTIME_PREFIX on some POSIX systems
Enable Git to resolve its own binary location using a variety of OS-specific and generic methods, including: - procfs via "/proc/self/exe" (Linux) - _NSGetExecutablePath (Darwin) - KERN_PROC_PATHNAME sysctl on BSDs. - argv0, if absolute (all, including Windows). This is used to enable RUNTIME_PREFIX support for non-Windows systems, notably Linux and Darwin. When configured with RUNTIME_PREFIX, Git will do a best-effort resolution of its executable path and automatically use this as its "exec_path" for relative helper and data lookups, unless explicitly overridden. Small incidental formatting cleanup of "exec_cmd.c". Signed-off-by: Dan Jacques Thanks-to: Robbie Iannucci Thanks-to: Junio C Hamano --- Makefile | 28 +- cache.h| 1 + common-main.c | 4 +- config.mak.uname | 7 ++ exec_cmd.c | 236 +++-- exec_cmd.h | 4 +- gettext.c | 8 +- git.c | 2 +- t/t0061-run-command.sh | 2 +- 9 files changed, 253 insertions(+), 39 deletions(-) diff --git a/Makefile b/Makefile index 13fb0e19a..960541e77 100644 --- a/Makefile +++ b/Makefile @@ -448,6 +448,18 @@ all:: # can be moved to arbitrary filesystem locations. RUNTIME_PREFIX also causes # Perl scripts to use a modified entry point header allowing them to resolve # support files at runtime. +# +# When using RUNTIME_PREFIX, define HAVE_BSD_KERN_PROC_SYSCTL if your platform +# supports the KERN_PROC BSD sysctl function. +# +# When using RUNTIME_PREFIX, define PROCFS_EXECUTABLE_PATH if your platform +# mounts a "procfs" filesystem capable of resolving the path of the current +# executable. If defined, this must be the canonical path for the "procfs" +# current executable path. +# +# When using RUNTIME_PREFIX, define HAVE_NS_GET_EXECUTABLE_PATH if your platform +# supports calling _NSGetExecutablePath to retrieve the path of the running +# executable. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1671,10 +1683,23 @@ ifdef HAVE_BSD_SYSCTL BASIC_CFLAGS += -DHAVE_BSD_SYSCTL endif +ifdef HAVE_BSD_KERN_PROC_SYSCTL + BASIC_CFLAGS += -DHAVE_BSD_KERN_PROC_SYSCTL +endif + ifdef HAVE_GETDELIM BASIC_CFLAGS += -DHAVE_GETDELIM endif +ifneq ($(PROCFS_EXECUTABLE_PATH),) + procfs_executable_path_SQ = $(subst ','\'',$(PROCFS_EXECUTABLE_PATH)) + BASIC_CFLAGS += '-DPROCFS_EXECUTABLE_PATH="$(procfs_executable_path_SQ)"' +endif + +ifdef HAVE_NS_GET_EXECUTABLE_PATH + BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH +endif + ifeq ($(TCLTK_PATH),) NO_TCLTK = NoThanks endif @@ -2223,6 +2248,7 @@ endif exec_cmd.sp exec_cmd.s exec_cmd.o: GIT-PREFIX exec_cmd.sp exec_cmd.s exec_cmd.o: EXTRA_CPPFLAGS = \ '-DGIT_EXEC_PATH="$(gitexecdir_SQ)"' \ + '-DGIT_LOCALE_PATH="$(localedir_relative_SQ)"' \ '-DBINDIR="$(bindir_relative_SQ)"' \ '-DPREFIX="$(prefix_SQ)"' @@ -2240,7 +2266,7 @@ attr.sp attr.s attr.o: EXTRA_CPPFLAGS = \ gettext.sp gettext.s gettext.o: GIT-PREFIX gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \ - -DGIT_LOCALE_PATH='"$(localedir_SQ)"' + -DGIT_LOCALE_PATH='"$(localedir_relative_SQ)"' http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS += \ -DCURL_DISABLE_TYPECHECK diff --git a/cache.h b/cache.h index 6e45c1b53..4f8754969 100644 --- a/cache.h +++ b/cache.h @@ -428,6 +428,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS" #define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH" #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS" +#define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR" /* * Environment variable used in handshaking the wire protocol. diff --git a/common-main.c b/common-main.c index 7d716d5a5..b2e5a86df 100644 --- a/common-main.c +++ b/common-main.c @@ -32,14 +32,14 @@ int main(int argc, const char **argv) */ sanitize_stdfds(); + git_resolve_executable_dir(argv[0]); + git_setup_gettext(); initialize_the_repository(); attr_start(); - git_extract_argv0_path(argv[0]); - restore_sigpipe_to_default(); return cmd_main(argc, argv); diff --git a/config.mak.uname b/config.mak.uname index 6a1d0de0c..e1cfe5e5e 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -37,6 +37,7 @@ ifeq ($(uname_S),Linux) HAVE_GETDELIM = YesPlease SANE_TEXT_GREP=-a FREAD_READS_DIRECTORIES = UnfortunatelyYes + PROCFS_EXECUTABLE_PATH = /proc/self/exe endif ifeq ($(uname_S),GNU/kFreeBSD) HAVE_ALLOCA_H = YesPlease @@ -111,6 +112,7 @@ ifeq ($(uname_S),Darwin) BASIC_CFLAGS += -DPROTECT_HFS_DEFAULT=1 HAVE_BSD_SYSCTL = YesPlease FREAD_READS_DIRECTORIES = UnfortunatelyYes + HAVE_NS_GET_EXECUTABLE_PATH = YesPlease endif ifeq ($(uname_S),S
[PATCH v8 4/5] exec_cmd: provide a new-style RUNTIME_PREFIX helper for Windows
From: Johannes Schindelin
The RUNTIME_PREFIX feature comes from Git for Windows, but it was
enhanced to allow support for other platforms. While changing the
original idea, the concept was also improved by not forcing argv[0] to
be adjusted.
Let's allow the same for Windows by implementing a helper just as for
the other platforms.
Signed-off-by: Johannes Schindelin
---
Makefile | 8
exec_cmd.c | 22 ++
2 files changed, 30 insertions(+)
diff --git a/Makefile b/Makefile
index 960541e77..8fc5559c7 100644
--- a/Makefile
+++ b/Makefile
@@ -460,6 +460,10 @@ all::
# When using RUNTIME_PREFIX, define HAVE_NS_GET_EXECUTABLE_PATH if your
platform
# supports calling _NSGetExecutablePath to retrieve the path of the running
# executable.
+#
+# When using RUNTIME_PREFIX, define HAVE_WPGMPTR if your platform offers
+# the global variable _wpgmptr containing the absolute path of the current
+# executable (this is the case on Windows).
GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1700,6 +1704,10 @@ ifdef HAVE_NS_GET_EXECUTABLE_PATH
BASIC_CFLAGS += -DHAVE_NS_GET_EXECUTABLE_PATH
endif
+ifdef HAVE_WPGMPTR
+ BASIC_CFLAGS += -DHAVE_WPGMPTR
+endif
+
ifeq ($(TCLTK_PATH),)
NO_TCLTK = NoThanks
endif
diff --git a/exec_cmd.c b/exec_cmd.c
index 38d52d90a..6e114f8b3 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -144,6 +144,24 @@ static int git_get_exec_path_darwin(struct strbuf *buf)
}
#endif /* HAVE_NS_GET_EXECUTABLE_PATH */
+#ifdef HAVE_WPGMPTR
+/*
+ * Resolves the executable path by using the global variable _wpgmptr.
+ *
+ * Returns 0 on success, -1 on failure.
+ */
+static int git_get_exec_path_wpgmptr(struct strbuf *buf)
+{
+ int len = wcslen(_wpgmptr) * 3 + 1;
+ strbuf_grow(buf, len);
+ len = xwcstoutf(buf->buf, _wpgmptr, len);
+ if (len < 0)
+ return -1;
+ buf->len += len;
+ return 0;
+}
+#endif /* HAVE_WPGMPTR */
+
/*
* Resolves the absolute path of the current executable.
*
@@ -178,6 +196,10 @@ static int git_get_exec_path(struct strbuf *buf, const
char *argv0)
git_get_exec_path_procfs(buf) &&
#endif /* PROCFS_EXECUTABLE_PATH */
+#ifdef HAVE_WPGMPTR
+ git_get_exec_path_wpgmptr(buf) &&
+#endif /* HAVE_WPGMPTR */
+
git_get_exec_path_from_argv0(buf, argv0)) {
return -1;
}
--
2.15.0.chromium12
[PATCH v8 2/5] Makefile: add Perl runtime prefix support
Broaden the RUNTIME_PREFIX flag to configure Git's Perl scripts to locate the Git installation's Perl support libraries by resolving against the script's path, rather than hard-coding that path at build-time. Hard-coding at build time worked on previous RUNTIME_PREFIX configurations (i.e., Windows) because the Perl scripts were run within a virtual filesystem whose paths were consistent regardless of the location of the actual installation. This will no longer be the case for non-Windows RUNTIME_PREFIX users. When enabled, RUNTIME_PREFIX now requires Perl's system paths to be expressed relative to a common installation directory in the Makefile, and uses that relationship to locate support files based on the known starting point of the script being executed, much like RUNTIME_PREFIX does for the Git binary. This change enables Git's Perl scripts to work when their Git installation is relocated or moved to another system, even when they are not in a virtual filesystem environment. Signed-off-by: Dan Jacques Thanks-to: Ævar Arnfjörð Bjarmason Thanks-to: Johannes Schindelin --- Makefile | 65 +++- perl/Git/I18N.pm | 2 +- perl/header_templates/runtime_prefix.template.pl | 42 +++ 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 perl/header_templates/runtime_prefix.template.pl diff --git a/Makefile b/Makefile index ec7cf5a0f..13fb0e19a 100644 --- a/Makefile +++ b/Makefile @@ -441,6 +441,13 @@ all:: # # When cross-compiling, define HOST_CPU as the canonical name of the CPU on # which the built Git will run (for instance "x86_64"). +# +# Define RUNTIME_PREFIX to configure Git to resolve its ancillary tooling and +# support files relative to the location of the runtime binary, rather than +# hard-coding them into the binary. Git installations built with RUNTIME_PREFIX +# can be moved to arbitrary filesystem locations. RUNTIME_PREFIX also causes +# Perl scripts to use a modified entry point header allowing them to resolve +# support files at runtime. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -478,6 +485,8 @@ ARFLAGS = rcs # mandir # infodir # htmldir +# localedir +# perllibdir # This can help installing the suite in a relocatable way. prefix = $(HOME) @@ -502,7 +511,9 @@ bindir_relative = $(patsubst $(prefix)/%,%,$(bindir)) mandir_relative = $(patsubst $(prefix)/%,%,$(mandir)) infodir_relative = $(patsubst $(prefix)/%,%,$(infodir)) gitexecdir_relative = $(patsubst $(prefix)/%,%,$(gitexecdir)) +localedir_relative = $(patsubst $(prefix)/%,%,$(localedir)) htmldir_relative = $(patsubst $(prefix)/%,%,$(htmldir)) +perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir)) export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir @@ -1748,11 +1759,13 @@ mandir_relative_SQ = $(subst ','\'',$(mandir_relative)) infodir_relative_SQ = $(subst ','\'',$(infodir_relative)) perllibdir_SQ = $(subst ','\'',$(perllibdir)) localedir_SQ = $(subst ','\'',$(localedir)) +localedir_relative_SQ = $(subst ','\'',$(localedir_relative)) gitexecdir_SQ = $(subst ','\'',$(gitexecdir)) gitexecdir_relative_SQ = $(subst ','\'',$(gitexecdir_relative)) template_dir_SQ = $(subst ','\'',$(template_dir)) htmldir_relative_SQ = $(subst ','\'',$(htmldir_relative)) prefix_SQ = $(subst ','\'',$(prefix)) +perllibdir_relative_SQ = $(subst ','\'',$(perllibdir_relative)) gitwebdir_SQ = $(subst ','\'',$(gitwebdir)) SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) @@ -1763,6 +1776,31 @@ TCLTK_PATH_SQ = $(subst ','\'',$(TCLTK_PATH)) DIFF_SQ = $(subst ','\'',$(DIFF)) PERLLIB_EXTRA_SQ = $(subst ','\'',$(PERLLIB_EXTRA)) +# RUNTIME_PREFIX's resolution logic requires resource paths to be expressed +# relative to each other and share an installation path. +# +# This is a dependency in: +# - Git's binary RUNTIME_PREFIX logic in (see "exec_cmd.c"). +# - The runtime prefix Perl header (see +# "perl/header_templates/runtime_prefix.template.pl"). +ifdef RUNTIME_PREFIX + +ifneq ($(filter /%,$(firstword $(gitexecdir_relative))),) +$(error RUNTIME_PREFIX requires a relative gitexecdir, not: $(gitexecdir)) +endif + +ifneq ($(filter /%,$(firstword $(localedir_relative))),) +$(error RUNTIME_PREFIX requires a relative localedir, not: $(localedir)) +endif + +ifndef NO_PERL +ifneq ($(filter /%,$(firstword $(perllibdir_relative))),) +$(error RUNTIME_PREFIX requires a relative perllibdir, not: $(perllibdir)) +endif +endif + +endif + # We must filter out any object files from $(GITLIBS), # as it is typically used like: # @@ -1983,10 +2021,31 @@ git.res: git.rc GIT-VERSION-FILE # This makes sure we depend on the NO_PERL setting itself. $(SCRIPT_PERL_GEN): GIT-BUILD-OPTIONS +# Used for substitution in Perl modules. Disabled when using RUNTIME_PREFIX +# since the locale directory is injected. +perl_localedir_SQ = $(localedir_SQ) + ifndef NO_PERL PERL_HEADER_TEM
[PATCH v8 0/5] RUNTIME_PREFIX relocatable Git
This is a minor update based on comments from the v6 series. I'm hoping this set is good to go! This patch set expands support for the RUNTIME_PREFIX configuration flag, currently only used on Windows builds, to include Linux, Darwin, and FreeBSD. When Git is built with RUNTIME_PREFIX enabled, it resolves its ancillary paths relative to the runtime location of its executable rather than hard-coding them at compile-time, allowing a Git installation to be deployed to a path other than the one in which it was built/installed. Note that RUNTIME_PREFIX is not currently used outside of Windows. This patch set should not have an impact on default Git builds. Previous threads: v1: https://public-inbox.org/git/20171116170523.28696-1-...@google.com/ v2: https://public-inbox.org/git/20171119173141.4896-1-...@google.com/ v3: https://public-inbox.org/git/20171127164055.93283-1-...@google.com/ v4: https://public-inbox.org/git/20171129223807.91343-1-...@google.com/ v5: https://public-inbox.org/git/20180108030239.92036-1-...@google.com/ v5: https://public-inbox.org/git/20180108030239.92036-1-...@google.com/ v6: https://public-inbox.org/git/20180319025046.58052-1-...@google.com/ v7: https://public-inbox.org/git/20180325205120.17730-1-...@google.com/ Changes in v8 from v7: - Add Johannes's Windows patch series to the end (see v7 thread). - Fix more typos and formatting nits. - Rebased on top of "master". === Testing === The latest patch set is available for testing on my GitHub fork, including "travis.ci" testing. The "runtime-prefix" branch includes a "config.mak" commit that enables runtime prefix for the Travis build; the "runtime-prefix-no-config" omits this file, testing this patch without runtime prefix enabled: - https://github.com/danjacques/git/tree/runtime-prefix - https://github.com/danjacques/git/tree/runtime-prefix-no-config - https://travis-ci.org/danjacques/git/branches Built/tested locally using this "config.mak" w/ autoconf: === Example config.mak === ## (BEGIN config.mak) RUNTIME_PREFIX = YesPlease RUNTIME_PREFIX_PERL = YesPlease gitexecdir = libexec/git-core template_dir = share/git-core/templates sysconfdir = etc ## (END config.mak) === Revision History === Changes in v7 from v6: - Change Perl header based on avarab@'s suggestion. - Fix typos in commit messages and comments. Changes in v6 from v5: - Rebased on top of "master". - Updated commit messages. - Updated runtime prefix Perl header comment and code to clarify when and why FindBin is used. - With Johannes' blessing on Git-for-Windows, folded "RUNTIME_PREFIX_PERL" functionality into "RUNTIME_PREFIX". - Updated "run-command" test to accommodate RUNTIME_PREFIX trace messages. Changes in v5 from v4: - Rebase on top of "next", notably incorporating the "ab/simplify-perl-makefile" branch. - Cleaner Makefile relative path enforcement. - Update Perl header template path now that the "perl/" directory has fewer build-related files in it. - Update Perl runtime prefix header to use a general system path resolution function. - Implemented the injection of the locale directory into Perl's "Git/I18N.pm" module from the runtime prefix Perl script header. - Updated Perl's "Git/I18N.pm" module to accept injected locale directory. - Added more content to some comments. Changes in v4 from v3: - Incorporated some quoting and Makefile dependency fixes, courtesy of . Changes in v3 from v2: - Broken into multiple patches now that Perl is isolated in its own RUNTIME_PREFIX_PERL flag. - Working with avarab@, several changes to Perl script runtime prefix support: - Moved Perl header body content from Makefile into external template file(s). - Added generic "perllibdir" variable to override Perl installation path. - RUNTIME_PREFIX_PERL generated script header is more descriptive and consistent with how the C version operates. - Fixed Generated Perl header Makefile dependency, should rebuild when dependent files and flags change. - Changed some of the new RUNTIME_PREFIX trace strings to use consistent formatting and terminology. Changes in v2 from v1: - Added comments and formatting to improve readability of platform-sepecific executable path resolution sleds in `git_get_exec_path`. - Consolidated "cached_exec_path" and "argv_exec_path" globals into "exec_path_value". - Use `strbuf_realpath` instead of `realpath` for procfs resolution. - Removed new environment variable exports. Git with RUNTIME_PREFIX no longer exports or consumes any additional environment information. - Updated Perl script resolution strategy: rather than having Git export the relative executable path to the Perl scripts, they now resolve it independently when RUNTIME_PREFIX_PERL is enabled. - Updated resolution strategy for "gettext()": use system_path() instead of special environment variable. - Added `sysctl` executable resolution support for BSDs that don't mount "procfs" by default (most of them). Dan Jacques (3):
Re: git-gui ignores core.hooksPath
Hello, using git 2.16 the same problem is still present. I see that the pull request https://github.com/patthoyts/git-gui/pull/12 (along with 15 other pull requests) are lying around since about one year without any sign of life from patthoyts. Is there any hope from here that anyone will pick up this / these changes? Will anyone else be assigned the main responsible for this git-gui repository? Just hoping to revive the discussion here, since the https://github.com/patthoyts/git-gui/ repository seems quite dead. sincerely, Chris Maes. -- Macq nv Luchtschipstraat, 2 - 1140 Brussel - België T +32 (0) 2 610 15 57 chris.m...@macq.eu - www.macq.eu
Re: [PATCH 8/8] gpg-interface: handle alternative signature types
On Tue, Apr 10, 2018 at 2:24 AM, Eric Sunshine wrote: > On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews wrote: >> [...] >> This patch introduces a set of configuration options for >> defining a "signing tool", of which gpg may be just one. >> With this patch you can: >> >> - define a new tool "foo" with signingtool.foo.program >> >> - map PEM strings to it for verification using >> signingtool.foo.pemtype >> >> - set it as your default tool for creating signatures >> using using signingtool.default > > s/using using/using/ > >> This subsumes the existing gpg config, as we have baked-in >> config for signingtool.gpg that works just like the current >> hard-coded behavior. And setting gpg.program becomes an >> alias for signingtool.gpg.program. >> >> This is enough to plug in gpgsm like: >> >> [signingtool "gpgsm"] >> program = gpgsm >> pemtype = "SIGNED MESSAGE" > > How confident are we that _all_ possible signing programs will conform > to the "-BEGIN %s-" pattern? If we're not confident, then > perhaps the user should be providing the full string here, not just > the '%s' part? These changes are only intended to work with PEM encoded signatures. The new config format leaves room for working with other signature formats in the future, though this would require further code changes. Requiring the user to specify the whole PEM start/end markers in the config doesn't make sense to me, since it assumes that non-PEM encodings would have similarly simple start/end delimiters. > > Further, I can infer from PGP itself, as well as from reading the code > that the 'pemtype' key can be specified multiple times; it would be > nice to mention that in the commit message. > >> [...] >> The implementation is still in gpg-interface.c, and most of >> the code internally refers to this as "gpg". I've left it >> this way to keep the diff sane, and to make it clear that >> we're not breaking anything gpg-related. This is probably OK >> for now, as our tools must be gpg-like anyway. But renaming >> everything would be an obvious next-step refactoring if we >> want to offer support for more exotic tools (e.g., people >> have asked before on the list about using OpenBSD signify). > > I can't decide if this paragraph belongs in the commit message proper > (as it currently is) or if it is commentary which should be placed > below the "---" line just above the diffstat. It feels more like > commentary, but not a big deal, and not itself worth a re-roll. > >> --- >> Documentation/config.txt | 40 +--- >> builtin/fmt-merge-msg.c | 6 +- >> builtin/receive-pack.c | 7 +- >> builtin/tag.c| 2 +- >> commit.c | 2 +- >> gpg-interface.c | 165 >> ++- >> gpg-interface.h | 24 ++- >> log-tree.c | 7 +- >> ref-filter.c | 2 +- >> t/lib-gpg.sh | 26 >> t/t7510-signed-commit.sh | 32 - >> tag.c| 2 +- >> 12 files changed, 272 insertions(+), 43 deletions(-) > > Due to its length, this patch is painfully time-consuming to review, > and asks reviewers to keep track of too many details at one time. As a > consequence, it's likely to scare away potential reviewers and limit > the quality of those reviews it does receive. If you break the changes > down into a series of much smaller patches which hold the hands of > reviewers, then you are likely to attract more and better reviews. > Please consider doing so. > > Each patch should be reasonably small and easy to digest. Each patch > should build logically upon the patch or patches preceding it, thus > incrementally building up a picture of the complete change. A (very) > rough idea of a series of smaller patches corresponding to this > feature might be: > > 1. introduce 'struct signing_tool' and the bare machinery for managing them > > 2. convert PGP from a hard-coded signer to a 'signing_tool' signer > > 3. add support for the new configuration variables > > It's likely that these steps can be broken into even smaller, more > reviewer-friendly ones; exactly how to do so may become apparent as > you think about how the patch series should be structured. For > instance, perhaps step 3 could be divided into: > > 3.1. add support for "signingtool.foo" variables > 3.2. retrofit "gpg.program" to be alias of "signingtool.gpg.program" > >> diff --git a/Documentation/config.txt b/Documentation/config.txt >> @@ -1727,16 +1727,38 @@ grep.fallbackToNoIndex:: >> -gpg.program:: >> - Use this custom program instead of "`gpg`" found on `$PATH` when >> - making or verifying a PGP signature. The program must support the >> - same command-line interface as GPG, namely, to verify a detached >> - signature, "`gpg --verify $file - <$signature`" is run, and the >> - program is expected to signal a good signature by exiting with >> - code 0, and to generate an ASCII
Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology
Hi Johannes, Johannes Schindelin writes: > Once upon a time, I dreamt of an interactive rebase that would not > flatten branch structure, but instead recreate the commit topology > faithfully. [...] > Think of --rebase-merges as "--preserve-merges done right". Both option names seem to miss the primary point of the mode of operation that you've formulated in the first sentence. I suggest to rather call the new option in accordance to your description, say, --no-flatten, --keep-topology, or --preserve-shape. Besides, this way the option name will only specify one thing: _what_ it is about, leaving out the _how_ part, that could vary and could then be specified as option value or as another companion option(s), that is usually considered to be an indication of a good design. -- Sergey
Re: [PATCH 6/8] gpg-interface: find the last gpg signature line
On Tue, Apr 10, 2018 at 3:44 AM, Junio C Hamano wrote: > Ben Toews writes: > >> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh >> index ee093b393d..e3f1e014aa 100755 >> --- a/t/t7004-tag.sh >> +++ b/t/t7004-tag.sh >> @@ -1059,6 +1059,17 @@ test_expect_success GPG \ >> git tag -v blanknonlfile-signed-tag >> ' >> >> +test_expect_success GPG 'signed tag with embedded PGP message' ' >> + cat >msg <<-\EOF && >> + -BEGIN PGP MESSAGE- >> + >> + this is not a real PGP message >> + -END PGP MESSAGE- >> + EOF >> + git tag -s -F msg confusing-pgp-message && >> + git tag -v confusing-pgp-message >> +' >> + >> # messages with commented lines for signed tags: >> >> cat >sigcommentsfile < > H, what vintage of our codebase is this patch based on? Did I > miss a patch that removes these lines > > > printf ' ' >sigblanknonlfile > get_tag_header blanknonlfile-signed-tag $commit commit $time >expect > echo '-BEGIN PGP SIGNATURE-' >>expect > test_expect_success GPG \ > 'creating a signed tag with spaces and no newline should succeed' > ' > git tag -s -F sigblanknonlfile blanknonlfile-signed-tag && > get_tag_msg blanknonlfile-signed-tag >actual && > test_cmp expect actual && > git tag -v signed-tag > ' > > which appear between the pre- and post- context of the lines you are > inserting? They date back to 2007-2009. > That test was fixed a week ago: https://github.com/git/git/commit/a99d903f21d102a5768f19157085a9733aeb68dd -- -Ben Toews
Re: core.fsmonitor always perform rescans
On 3/26/2018 12:41 AM, Tatsuyuki Ishi wrote: Hello, I'm facing issue with core.fsmonitor. I'm currently using the provided watchman hook, but it doesn't seem to record the fact that it has queried the fsmonitor backend, and as a result the timestamp passed to the hook doesn't seem to change. As it always pass a timestamp before watchman has crawled the directories, watchman will always return all files inside the directory. This happens everytime I run a git command, resulting in slowness. Is the timestamp not being updated an intended behavior, or is this a bug? As a performance optimization, the fsmonitor code doesn't flag the index as dirty and force it to be written out with every command. Can you try performing a git operation (add, rm, commit, etc) that will write out an updated index and see if that fixes the issue you're seeing? I'm considering adding a special case to force the index to be written out the first time fsmonitor is invoked and am interested to know if this would have avoided the issue you are seeing. Thanks! Tatsuyuki Ishi
Re: [PATCH 1/8] gpg-interface: handle bool user.signingkey
On Mon, Apr 09, 2018 at 04:55:26PM -0400, Eric Sunshine wrote: > Peff's Signed-off-by: is missing. Also, since you're forwarding this > patch on Peff's behalf, your Signed-off-by: should follow his. Same > comment applies to all patches by Peff in this series. I usually sign-off as I send to the list, but I passed these patches to Ben via a repository. For the record, it's OK to add my s-o-b to the whole series. -Peff
Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
On 04/10/2018 12:45 AM, Stefan Beller wrote:
> Signed-off-by: Stefan Beller
> ---
> refs.c | 13 +
> refs.h | 4 +---
> repository.h | 3 +++
> 3 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index f58b9fb7df..b5be754a97 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry
> *alloc_ref_store_hash_entry(
> return entry;
> }
>
> -/* A pointer to the ref_store for the main repository: */
> -static struct ref_store *main_ref_store;
> -
> /* A hashmap of ref_stores, stored by submodule name: */
> static struct hashmap submodule_ref_stores;
>
> @@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char
> *gitdir,
> return refs;
> }
>
> -struct ref_store *get_main_ref_store_the_repository(void)
> +struct ref_store *get_main_ref_store(struct repository *r)
> {
> - if (main_ref_store)
> - return main_ref_store;
> + if (r->main_ref_store)
> + return r->main_ref_store;
>
> - main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
> - return main_ref_store;
> + r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
> + return r->main_ref_store;
> }
>
> /*
> diff --git a/refs.h b/refs.h
> index ab3d2bec2f..f5ab68c0ed 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct
> object_id *oid,
>
> int ref_storage_backend_exists(const char *name);
>
> -#define get_main_ref_store(r) \
> - get_main_ref_store_##r()
> -struct ref_store *get_main_ref_store_the_repository(void);
> +struct ref_store *get_main_ref_store(struct repository *r);
> /*
> * Return the ref_store instance for the specified submodule. For the
> * main repository, use submodule==NULL; such a call cannot fail. For
> diff --git a/repository.h b/repository.h
> index 09df94a472..7d0710b273 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -26,6 +26,9 @@ struct repository {
>*/
> struct raw_object_store *objects;
>
> + /* The store in which the refs are held. */
> + struct ref_store *main_ref_store;
> +
> /*
>* Path to the repository's graft file.
>* Cannot be NULL after initialization.
>
This also makes sense to me, as far as it goes. I have a few comments
and questions:
Why do you call the new member `main_ref_store`? Is there expected to be
some other `ref_store` associated with a repository?
I think the origin of the name `main_ref_store` was to distinguish it
from submodule ref stores. But presumably those will soon become the
"main" ref stores for their respective submodule repository objects,
right? So maybe calling things `repository.ref_store` and
`get_ref_store(repository)` would be appropriate.
There are some places in the reference code that only work with the main
repository. The ones that I can think of are:
* `ref_resolves_to_object()` depends on an object store.
* `peel_object()` and `ref_iterator_peel()` also have to look up objects
(in this case, tag objects).
* Anything that calls `files_assert_main_repository()` or depends on
`REF_STORE_MAIN` isn't implemented for other reference stores (usually,
I think, these are functions that depend on the object store).
Some of these things might be easy to generalize to non-main
repositories, but I didn't bother because AFAIK currently only the main
repository store is ever mutated.
You can move a now-obsolete comment above the definition of `struct
files_ref_store` if you haven't in some other patch (search for
"libification").
Hope that helps,
Michael
