* Re: [PATCH 1/2] asciidoctor: fix user-manual to be built by `asciidoctor`
From: Junio C Hamano @ 2017-01-07 22:03 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, 마누엘
In-Reply-To: <20170104080852.bmlmtzxhjx4qt74f@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Jan 02, 2017 at 05:03:57PM +0100, Johannes Schindelin wrote:
>
>> From: =?UTF-8?q?=EB=A7=88=EB=88=84=EC=97=98?= <nalla@hamal.uberspace.de>
>>
>> The `user-manual.txt` is designed as a `book` but the `Makefile` wants
>> to build it as an `article`. This seems to be a problem when building
>> the documentation with `asciidoctor`. Furthermore the parts *Git
>> Glossary* and *Appendix B* had no subsections which is not allowed when
>> building with `asciidoctor`. So lets add a *dummy* section.
>
> The git-scm.com site uses asciidoctor, too, and I think I have seen some
> oddness with the rendering though. So in general I am in favor of making
> things work under both asciidoc and asciidoctor.
>
> I diffed the results of "make user-manual.html" before and after this
> patch. A lot of "h3" chapter titles get bumped to "h2", which is OK. The
> chapter titles now say "Chapter 1. Repositories and Branches" rather
> than just "Repositories and Branches". Likewise, references now say
>
> Chapter 1, _Repositories and Branches_
>
> rather than:
>
> the section called "Repositories and Branches".
>
> which is probably OK, though the whole thing is short enough that
> calling the sections chapters feels a bit over-important.
Is that a longer way to say that the claim "... is designed as a
book" is false?
> So I dunno. I really do think "article" is conceptually the most
> appropriate style, but I agree that there are some book-like things
> (like appendices).
... Yeah, I should have read forward first before starting to insert
my comments.
^ permalink raw reply
* Re: [PATCH 2/2] giteveryday: unbreak rendering with AsciiDoctor
From: Junio C Hamano @ 2017-01-07 22:05 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170104081544.5htofa3zpgkvty7x@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Jan 02, 2017 at 05:04:05PM +0100, Johannes Schindelin wrote:
>
>> The "giteveryday" document has a callout list that contains a code
>> block. This is not a problem for AsciiDoc, but AsciiDoctor sadly was
>> explicitly designed *not* to render this correctly [*1*]. The symptom is
>> an unhelpful
>>
>> line 322: callout list item index: expected 1 got 12
>> line 325: no callouts refer to list item 1
>> line 325: callout list item index: expected 2 got 13
>> line 327: no callouts refer to list item 2
>>
>> In Git for Windows, we rely on the speed improvement of AsciiDoctor (on
>> this developer's machine, `make -j15 html` takes roughly 30 seconds with
>> AsciiDoctor, 70 seconds with AsciiDoc), therefore we need a way to
>> render this correctly.
>>
>> The easiest way out is to simplify the callout list, as suggested by
>> AsciiDoctor's author, even while one may very well disagree with him
>> that a code block hath no place in a callout list.
>
> This looks like a good re-write to avoid the issue while maintaining the
> meaning and flow of the original.
OK. Ack.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] asciidoctor: fix user-manual to be built by `asciidoctor`
From: brian m. carlson @ 2017-01-07 22:08 UTC (permalink / raw)
To: Jeff King
Cc: Lars Schneider, Johannes Schindelin, git,
마누엘, Junio C Hamano
In-Reply-To: <20170105164556.b3bzeqqzx4pvni4z@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 2330 bytes --]
On Thu, Jan 05, 2017 at 11:45:57AM -0500, Jeff King wrote:
> On Thu, Jan 05, 2017 at 11:05:29AM +0100, Lars Schneider wrote:
>
> > > The git-scm.com site uses asciidoctor, too, and I think I have seen some
> > > oddness with the rendering though. So in general I am in favor of making
> > > things work under both asciidoc and asciidoctor.
> >
> > I am not familiar with both tools but it sounds to me as if "asciidoctor"
> > is kind of the "lowest common denominator". Is this true? If yes, would it
> > make sense to switch TravisCI [1] to asciidocter if this change gets merged?
>
> I don't think that's quite true.
>
> The two programs produce different output for certain inputs. We tend to
> see the cases where asciidoc produces the desired output and asciidoctor
> doesn't, because traditionally the documentation was written _for_
> asciidoc. So whenever asciidoctor diverges, it looks like a bug.
This is indeed the case. Asciidoctor is a bit stricter on some inputs
because it provides significant performance improvements, but it also
has features that AsciiDoc does not. It also has some bugs that
AsciiDoc does not (again, usually due to performance concerns).
For example, Debian's reproducible builds project would probably like it
if we had better support for Asciidoctor.
> [1] I think we've also traditionally considered asciidoc to be the
> definitive toolchain, and people using asciidoctor are free to
> submit patches to make things work correctly in both places. I'm not
> opposed to changing that attitude, as it seems like asciidoctor is
> faster and more actively maintained these days. But I suspect our
> build chain would need some improvements. Last time I tried building
> with AsciiDoctor it involved a lot manual tweaking of Makefile
> variables. It sounds like Dscho is doing it regularly, though. It
> should probably work out of the box (with something like
> USE_ASCIIDOCTOR=Yes) if we expect people to actually rely on it.
Yes, that would probably be beneficial. I'll see if I can come up with
some patches based on Dscho's work.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [PATCH 0/3] fix ^C killing pager when running alias
From: Jacob Keller @ 2017-01-07 23:26 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Sixt, Trygve Aaberge,
Nguyễn Thái Ngọc Duy, Git mailing list
In-Reply-To: <20170107011445.3e4fv6vdtimrwhgv@sigill.intra.peff.net>
On Fri, Jan 6, 2017 at 5:14 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 06, 2017 at 06:20:42PM -0500, Jeff King wrote:
>
>> > In general, I think it is wrong to wait for child processes when a signal
>> > was received. After all, it is the purpose of a (deadly) signal to have the
>> > process go away. There may be programs that know it better, like less, but
>> > git should not attempt to know better in general.
>> >
>> > We do apply some special behavior for certain cases like we do for the
>> > pager. And now the case with aliases is another special situation. The
>> > parent git process only delegates to the child, and as such it is reasonable
>> > that it binds its life time to the first child, which executes the expanded
>> > alias.
>>
>> Yeah, I think I agree. That binding is something you want in many cases,
>> but not necessarily all. The original purpose of clean_on_exit was to
>> create a binding like that, but of course it can be (and with the
>> smudge-filter stuff, arguably has been) used for other cases, too.
>>
>> I'll work up a patch that makes it a separate option, which should be
>> pretty easy.
>
> Yeah, this did turn out to be really easy. I spent most of the time
> trying to explain the issue in the commit message in a sane way.
> Hopefully it didn't end up _too_ long. :)
>
> The interesting bit is in the third one. The first is a necessary
> preparatory step, and the second is a cleanup I noticed in the
> neighborhood.
>
> [1/3]: execv_dashed_external: use child_process struct
> [2/3]: execv_dashed_external: stop exiting with negative code
> [3/3]: execv_dashed_external: wait for child on signal death
>
> git.c | 36 +++++++++++++++---------------------
> run-command.c | 19 +++++++++++++++++++
> run-command.h | 1 +
> 3 files changed, 35 insertions(+), 21 deletions(-)
>
> -Peff
I don't see the rest of the patches on the list..?
Thanks,
Jake
^ permalink raw reply
* Re: [PATCH 0/3] fix ^C killing pager when running alias
From: Jacob Keller @ 2017-01-07 23:27 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Sixt, Trygve Aaberge,
Nguyễn Thái Ngọc Duy, Git mailing list
In-Reply-To: <CA+P7+xqEqKWmQGgAyrmdzOZgO0CXFOGfcp=0otJ_nQPS13wFWg@mail.gmail.com>
On Sat, Jan 7, 2017 at 3:26 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Fri, Jan 6, 2017 at 5:14 PM, Jeff King <peff@peff.net> wrote:
>> On Fri, Jan 06, 2017 at 06:20:42PM -0500, Jeff King wrote:
>>
>>> > In general, I think it is wrong to wait for child processes when a signal
>>> > was received. After all, it is the purpose of a (deadly) signal to have the
>>> > process go away. There may be programs that know it better, like less, but
>>> > git should not attempt to know better in general.
>>> >
>>> > We do apply some special behavior for certain cases like we do for the
>>> > pager. And now the case with aliases is another special situation. The
>>> > parent git process only delegates to the child, and as such it is reasonable
>>> > that it binds its life time to the first child, which executes the expanded
>>> > alias.
>>>
>>> Yeah, I think I agree. That binding is something you want in many cases,
>>> but not necessarily all. The original purpose of clean_on_exit was to
>>> create a binding like that, but of course it can be (and with the
>>> smudge-filter stuff, arguably has been) used for other cases, too.
>>>
>>> I'll work up a patch that makes it a separate option, which should be
>>> pretty easy.
>>
>> Yeah, this did turn out to be really easy. I spent most of the time
>> trying to explain the issue in the commit message in a sane way.
>> Hopefully it didn't end up _too_ long. :)
>>
>> The interesting bit is in the third one. The first is a necessary
>> preparatory step, and the second is a cleanup I noticed in the
>> neighborhood.
>>
>> [1/3]: execv_dashed_external: use child_process struct
>> [2/3]: execv_dashed_external: stop exiting with negative code
>> [3/3]: execv_dashed_external: wait for child on signal death
>>
>> git.c | 36 +++++++++++++++---------------------
>> run-command.c | 19 +++++++++++++++++++
>> run-command.h | 1 +
>> 3 files changed, 35 insertions(+), 21 deletions(-)
>>
>> -Peff
>
> I don't see the rest of the patches on the list..?
>
> Thanks,
> Jake
They showed up on public inbox so I assume it must be some spam filter
on my end..
Hmm,
Jake
^ permalink raw reply
* Re: [PATCH v2 05/34] sequencer (rebase -i): learn about the 'verbose' mode
From: Junio C Hamano @ 2017-01-08 1:57 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Kevin Daudt, Dennis Kaarsemaker
In-Reply-To: <xmqqk2a635it.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> ...
>>
>> I guess my only defence is that I tried to be a little lazy.
>
> I actually was alluding to going the other way around, spawning
> "diff-tree -p" in the other codepath like this one does.
Pre-emptively, because I expect it will take a while for me to clear
the backlog to get to your reroll already on the mailing list, I do
not mean to say that you must rewrite the other one to spawn to
match this one. I meant that I wouldn't have minded if the series
were done that way, as this kind of being "little lazy" would reduce
the chance of regression by reducing the number of exchanged parts
and it is not necessarily a bad thing.
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-08 2:33 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Segev Finer
In-Reply-To: <2ff29a4d00e0e13d460122d8008e762361ca90aa.1483358673.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> + if (ssh) {
> + char *split_ssh = xstrdup(ssh);
> + const char **ssh_argv;
> +
> + if (split_cmdline(split_ssh, &ssh_argv))
> + ssh_argv0 = xstrdup(ssh_argv[0]);
> + free(split_ssh);
> + free((void *)ssh_argv);
> + } else {
> /*
> * GIT_SSH is the no-shell version of
> * GIT_SSH_COMMAND (and must remain so for
> @@ -807,8 +813,11 @@ struct child_process *git_connect(int fd[2], const char *url,
> if (!ssh)
> ssh = "ssh";
>
> - ssh_dup = xstrdup(ssh);
> - base = basename(ssh_dup);
> + ssh_argv0 = xstrdup(ssh);
> + }
> +
> + if (ssh_argv0) {
> + const char *base = basename(ssh_argv0);
>
> tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
> !strcasecmp(base, "tortoiseplink.exe");
I suspect that this will break when GIT_SSH_COMMAND, which is meant
to be processed by the shell, hence the user can write anything,
begins with a one-shot environment variable assignment, e.g.
[core] sshcommand = VAR1=VAL1 VAR2=VAL2 //path/to/tortoiseplink
One possible unintended side effect of this patch is when VAL1 ends
with /plink (or /tortoiseplink) and the command is not either of
these, in which case the "tortoiseplink" and "putty" variables will
tweak the command line for an SSH implementation that does not want
such a tweak to be made. There may be other unintended fallouts.
Sorry, no concrete suggestion to get this to work comes to my mind
offhand.
Perhaps give an explicit way to force "tortoiseplink" and "putty"
variables without looking at and guessing from the pathname, so that
the solution does not have to split and peek the command line?
connect.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/connect.c b/connect.c
index 8cb93b0720..1768122456 100644
--- a/connect.c
+++ b/connect.c
@@ -691,6 +691,23 @@ static const char *get_ssh_command(void)
return NULL;
}
+static void get_ssh_variant(int *tortoiseplink, int *putty)
+{
+ const char *variant;
+ if (!git_config_get_string_const("ssh.variant", &variant))
+ return;
+ if (!strcmp(variant, "tortoiseplink")) {
+ *tortoiseplink = 1;
+ *putty = 1;
+ } else if (!strcmp(variant, "putty")) {
+ *tortoiseplink = 0;
+ *putty = 1;
+ } else {
+ *tortoiseplink = 0;
+ *putty = 0;
+ }
+}
+
/*
* This returns a dummy child_process if the transport protocol does not
* need fork(2), or a struct child_process object if it does. Once done,
@@ -824,6 +841,9 @@ struct child_process *git_connect(int fd[2], const char *url,
argv_array_push(&conn->args, "-4");
else if (flags & CONNECT_IPV6)
argv_array_push(&conn->args, "-6");
+
+ get_ssh_variant(&tortoiseplink, &putty);
+
if (tortoiseplink)
argv_array_push(&conn->args, "-batch");
if (port) {
^ permalink raw reply related
* Re: [PATCH] diff: add interhunk context config option
From: Junio C Hamano @ 2017-01-08 3:02 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Vegard Nossum, Git List, René Scharfe
In-Reply-To: <CAFZEwPMLk5KTeLR9zygU6ZH5zN7TLnQzmJVpyCNig8FrYcOazw@mail.gmail.com>
Pranit Bauva <pranit.bauva@gmail.com> writes:
>> diff --git a/diff.c b/diff.c
>> index 84dba60c4..40b4e6afe 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -33,6 +33,7 @@ static int diff_rename_limit_default = 400;
>> static int diff_suppress_blank_empty;
>> static int diff_use_color_default = -1;
>> static int diff_context_default = 3;
>> +static int diff_interhunk_context_default = 0;
Do not explicitly initialize BSS variables to 0 (or NULL).
>> static const char *diff_word_regex_cfg;
>> static const char *external_diff_cmd_cfg;
>> static const char *diff_order_file_cfg;
>> @@ -248,6 +249,12 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
>> return -1;
>> return 0;
>> }
>> + if (!strcmp(var, "diff.interhunkcontext")) {
>> + diff_interhunk_context_default = git_config_int(var, value);
>> + if (diff_interhunk_context_default < 0)
>> + return -1;
>> + return 0;
>> + }
>> if (!strcmp(var, "diff.renames")) {
>> diff_detect_rename_default = git_config_rename(var, value);
>> return 0;
>> @@ -3371,6 +3378,7 @@ void diff_setup(struct diff_options *options)
>> options->rename_limit = -1;
>> options->dirstat_permille = diff_dirstat_permille_default;
>> options->context = diff_context_default;
>> + options->interhunkcontext = diff_interhunk_context_default;
Will this receive -1 if diff.interhunkcontext configuration variable
is misconfigured and the *_default variable receives the error return
value from git_config_int()?
>> options->ws_error_highlight = ws_error_highlight_default;
>> DIFF_OPT_SET(options, RENAME_EMPTY);
>
> On a first look, it seems that we can overwrite the default config
> values by using a different command line argument which is good.
>
> Also, tests are missing. It seems that t/t4032 might be a good place
> to add those tests.
>
> Rest all is quite good! :)
>
> Regards,
> Pranit Bauva
^ permalink raw reply
* Re: [PATCH v5 0/5] road to reentrant real_path
From: Junio C Hamano @ 2017-01-08 3:09 UTC (permalink / raw)
To: Brandon Williams
Cc: git, sbeller, peff, jacob.keller, ramsay, tboegi, j6t, pclouds,
larsxschneider
In-Reply-To: <20170104220124.145808-1-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> changes in v5:
> * set errno to ELOOP when MAXSYMLINKS is exceded.
> * revert to use MAXSYMLINKS instead of MAXDEPTH.
> * If the OS hasn't defined MAXSYMLINKS, use a fallback value of 32.
>
> Brandon Williams (4):
> real_path: resolve symlinks by hand
> real_path: convert real_path_internal to strbuf_realpath
> real_path: create real_pathdup
> real_path: have callers use real_pathdup and strbuf_realpath
>
> Johannes Sixt (1):
> real_path: canonicalize directory separators in root parts
>
How does this relate to the 5-patch real_path: series that has been
on 'next' since last year?
^ permalink raw reply
* Re: [PATCH] rebase--interactive: count squash commits above 10 correctly
From: Junio C Hamano @ 2017-01-08 3:26 UTC (permalink / raw)
To: Jeff King; +Cc: Brandon Tolsch, Johannes Schindelin, Vasco Almeida, git
In-Reply-To: <20170107082318.jj3izthx2ylscrns@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> git-rebase--interactive.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index b0a6f2b7ba..4734094a3f 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -425,7 +425,7 @@ update_squash_messages () {
> if test -f "$squash_msg"; then
> mv "$squash_msg" "$squash_msg".bak || exit
> count=$(($(sed -n \
> - -e "1s/^$comment_char.*\([0-9][0-9]*\).*/\1/p" \
> + -e "1s/^$comment_char[^0-9]*\([0-9][0-9]*\).*/\1/p" \
I would have written it to match ".*[^1-9]\([1-9][0-9]*\)", though.
Even if a foreign language expresses all its words as digits (or has
a digit as the last letter of the last word before the number), a
translation into it must have a non-digit before the number to
disambiguate---but I guess I am being ultra-pedantic, and I think
what you wrote would be sufficient in practice.
As you guys discussed, this code will hopefully disappear in a cycle
or two anyway ;-) Let's queue this as-is.
Thanks.
> -e "q" < "$squash_msg".bak)+1))
> {
> printf '%s\n' "$comment_char $(eval_ngettext \
^ permalink raw reply
* Re: [PATCH 1/2] asciidoctor: fix user-manual to be built by `asciidoctor`
From: Jeff King @ 2017-01-08 3:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, 마누엘
In-Reply-To: <xmqqbmvi34ul.fsf@gitster.mtv.corp.google.com>
On Sat, Jan 07, 2017 at 02:03:30PM -0800, Junio C Hamano wrote:
> Is that a longer way to say that the claim "... is designed as a
> book" is false?
>
> > So I dunno. I really do think "article" is conceptually the most
> > appropriate style, but I agree that there are some book-like things
> > (like appendices).
>
> ... Yeah, I should have read forward first before starting to insert
> my comments.
To be honest, I'm not sure whether "book" versus "article" was really
considered in the original writing. I think we can call it whichever
produces the output we find most pleasing. I was mostly just pointing at
there are some tradeoffs in the end result in flipping the type.
-Peff
^ permalink raw reply
* Re: [PATCH 0/3] fixing some random blame corner cases
From: Junio C Hamano @ 2017-01-08 3:39 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170106041541.rjzzofal5hscv6yi@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> [1/3]: blame: fix alignment with --abbrev=40
> [2/3]: blame: handle --no-abbrev
> [3/3]: blame: output porcelain "previous" header for each file
Thanks. 1 & 2 obviously look correct. I'd need to look at 3 when I
am not exhausted, even though I expect it to be also fine from a
cursory read.
^ permalink raw reply
* Re: "git fsck" not detecting garbage at the end of blob object files...
From: Jeff King @ 2017-01-08 5:26 UTC (permalink / raw)
To: Dennis Kaarsemaker; +Cc: John Szakmeister, git
In-Reply-To: <1483825623.31837.9.camel@kaarsemaker.net>
On Sat, Jan 07, 2017 at 10:47:03PM +0100, Dennis Kaarsemaker wrote:
> On Sat, 2017-01-07 at 07:50 -0500, John Szakmeister wrote:
> > I was perusing StackOverflow this morning and ran across this
> > question: http://stackoverflow.com/questions/41521143/git-fsck-full-only-checking-directories/
> >
> > It was a simple question about why "checking objects" was not
> > appearing, but in it was another issue. The user purposefully
> > corrupted a blob object file to see if `git fsck` would catch it by
> > tacking extra data on at the end. `git fsck` happily said everything
> > was okay, but when I played with things locally I found out that `git
> > gc` does not like that extra garbage. I'm not sure what the trade-off
> > needs to be here, but my expectation is that if `git fsck` says
> > everything is okay, then all operations using that object (file)
> > should work too.
> >
> > Is that unreasonable? What would be the impact of fixing this issue?
>
> If you do this with a commit object or tree object, fsck does complain.
> I think it's sensible to do so for blob objects as well.
The existing extra-garbage check is in unpack_sha1_rest(), which is
called as part of read_sha1_file(). And that's what we hit for commits
and trees. However, we check the sha1 of blobs using the streaming
interface (in case they're large). I think you'd want to put a similar
check into read_istream_loose(). But note if you are grepping for it, it
is hidden behind a macro; look for read_method_decl(loose).
I'm actually not sure if this should be downgrade to a warning. It's
true that it's a form of corruption, but it doesn't actually prohibit us
from getting the data we need to complete the operation. Arguably fsck
should be more picky, but it is just relying on the same parse_object()
code path that the rest of git uses.
I doubt anybody cares too much either way, though. It's not like this is
a common thing.
I did notice another interesting case when looking at this. Fsck ends up
in fsck_loose(), which has the sha1 and path of the loose object. It
passes the sha1 to fsck_sha1(), and ignores the path entirely!
So if you have a duplicate copy of the object in a pack, we'd actually
find and check the duplicate. This can happen, e.g., if you had a loose
object and fetched a thin-pack which made a copy of the loose object to
complete the pack).
Probably fsck_loose() should be more picky about making sure we are
reading the data from the loose version we found.
-Peff
^ permalink raw reply
* [PATCH] Makefile: put LIBS after LDFLAGS for imap-send
From: Steven Penny @ 2017-01-08 6:12 UTC (permalink / raw)
To: git; +Cc: Steven Penny
This matches up with the targets git-%, git-http-fetch, git-http-push and
git-remote-testsvn. It must be done this way on Windows else lcrypto cannot find
lgdi32 and lws2_32
Signed-off-by: Steven Penny <svnpenn@gmail.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index d861bd9..f0f65ea 100644
--- a/Makefile
+++ b/Makefile
@@ -2046,7 +2046,7 @@ git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
- $(LIBS) $(IMAP_SEND_LDFLAGS)
+ $(IMAP_SEND_LDFLAGS) $(LIBS)
git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
--
2.8.3
^ permalink raw reply related
* [PATCH 0/6] git worktree move/remove
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
This version is the same as nd/worktree-move but with the recursive
directory copy code removed. We rely on rename() to move directories.
Nguyễn Thái Ngọc Duy (6):
worktree.c: add validate_worktree()
worktree.c: add update_worktree_location()
worktree move: new command
worktree move: accept destination as directory
worktree move: refuse to move worktrees with submodules
worktree remove: new command
Documentation/git-worktree.txt | 28 ++++--
builtin/worktree.c | 162 +++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 5 +-
t/t2028-worktree-move.sh | 56 ++++++++++++
worktree.c | 84 +++++++++++++++++
worktree.h | 11 +++
6 files changed, 335 insertions(+), 11 deletions(-)
--
2.8.2.524.g6ff3d78
^ permalink raw reply
* [PATCH 1/6] worktree.c: add validate_worktree()
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
This function is later used by "worktree move" and "worktree remove"
to ensure that we have a good connection between the repository and
the worktree. For example, if a worktree is moved manually, the
worktree location recorded in $GIT_DIR/worktrees/.../gitdir is
incorrect and we should not move that one.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
worktree.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
worktree.h | 5 +++++
2 files changed, 68 insertions(+)
diff --git a/worktree.c b/worktree.c
index eb61212..929072a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -291,6 +291,69 @@ const char *is_worktree_locked(struct worktree *wt)
return wt->lock_reason;
}
+static int report(int quiet, const char *fmt, ...)
+{
+ va_list params;
+
+ if (quiet)
+ return -1;
+
+ va_start(params, fmt);
+ vfprintf(stderr, fmt, params);
+ fputc('\n', stderr);
+ va_end(params);
+ return -1;
+}
+
+int validate_worktree(const struct worktree *wt, int quiet)
+{
+ struct strbuf sb = STRBUF_INIT;
+ const char *path;
+ int err;
+
+ if (is_main_worktree(wt)) {
+ /*
+ * Main worktree using .git file to point to the
+ * repository would make it impossible to know where
+ * the actual worktree is if this function is executed
+ * from another worktree. No .git file support for now.
+ */
+ strbuf_addf(&sb, "%s/.git", wt->path);
+ if (!is_directory(sb.buf)) {
+ strbuf_release(&sb);
+ return report(quiet, _("'%s/.git' at main worktree is not the repository directory"),
+ wt->path);
+ }
+ return 0;
+ }
+
+ /*
+ * Make sure "gitdir" file points to a real .git file and that
+ * file points back here.
+ */
+ if (!is_absolute_path(wt->path))
+ return report(quiet, _("'%s' file does not contain absolute path to the worktree location"),
+ git_common_path("worktrees/%s/gitdir", wt->id));
+
+ strbuf_addf(&sb, "%s/.git", wt->path);
+ if (!file_exists(sb.buf)) {
+ strbuf_release(&sb);
+ return report(quiet, _("'%s/.git' does not exist"), wt->path);
+ }
+
+ path = read_gitfile_gently(sb.buf, &err);
+ strbuf_release(&sb);
+ if (!path)
+ return report(quiet, _("'%s/.git' is not a .git file, error code %d"),
+ wt->path, err);
+
+ if (fspathcmp(path, real_path(git_common_path("worktrees/%s", wt->id))))
+ return report(quiet, _("'%s' does not point back to"),
+ wt->path, git_common_path("worktrees/%s", wt->id));
+
+ return 0;
+}
+
int is_worktree_being_rebased(const struct worktree *wt,
const char *target)
{
diff --git a/worktree.h b/worktree.h
index d59ce1f..4433db2 100644
--- a/worktree.h
+++ b/worktree.h
@@ -52,6 +52,11 @@ extern int is_main_worktree(const struct worktree *wt);
*/
extern const char *is_worktree_locked(struct worktree *wt);
+/*
+ * Return zero if the worktree is in good condition.
+ */
+extern int validate_worktree(const struct worktree *wt, int quiet);
+
/*
* Free up the memory for worktree(s)
*/
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 2/6] worktree.c: add update_worktree_location()
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:39 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
worktree.c | 21 +++++++++++++++++++++
worktree.h | 6 ++++++
2 files changed, 27 insertions(+)
diff --git a/worktree.c b/worktree.c
index 929072a..7684951 100644
--- a/worktree.c
+++ b/worktree.c
@@ -354,6 +354,27 @@ int validate_worktree(const struct worktree *wt, int quiet)
return 0;
}
+int update_worktree_location(struct worktree *wt, const char *path_)
+{
+ struct strbuf path = STRBUF_INIT;
+ int ret = 0;
+
+ if (is_main_worktree(wt))
+ return 0;
+
+ strbuf_add_absolute_path(&path, path_);
+ if (fspathcmp(wt->path, path.buf)) {
+ write_file(git_common_path("worktrees/%s/gitdir",
+ wt->id),
+ "%s/.git", real_path(path.buf));
+ free(wt->path);
+ wt->path = strbuf_detach(&path, NULL);
+ ret = 0;
+ }
+ strbuf_release(&path);
+ return ret;
+}
+
int is_worktree_being_rebased(const struct worktree *wt,
const char *target)
{
diff --git a/worktree.h b/worktree.h
index 4433db2..1ee03f4 100644
--- a/worktree.h
+++ b/worktree.h
@@ -57,6 +57,12 @@ extern const char *is_worktree_locked(struct worktree *wt);
*/
extern int validate_worktree(const struct worktree *wt, int quiet);
+/*
+ * Update worktrees/xxx/gitdir with the new path.
+ */
+extern int update_worktree_location(struct worktree *wt,
+ const char *path_);
+
/*
* Free up the memory for worktree(s)
*/
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 3/6] worktree move: new command
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
There are two options to move the main worktree, but both have
complications, so it's not implemented yet. Anyway the options are:
- convert the main worktree to a linked one and move it away, leave the
git repository where it is. The repo essentially becomes bare after
this move.
- move the repository with the main worktree. The tricky part is make
sure all file descriptors to the repository are closed, or it may
fail on Windows.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-worktree.txt | 7 +++++-
builtin/worktree.c | 43 ++++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 2 +-
t/t2028-worktree-move.sh | 30 ++++++++++++++++++++++++
4 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e257c19..1310513 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -12,6 +12,7 @@ SYNOPSIS
'git worktree add' [-f] [--detach] [--checkout] [-b <new-branch>] <path> [<branch>]
'git worktree list' [--porcelain]
'git worktree lock' [--reason <string>] <worktree>
+'git worktree move' <worktree> <new-path>
'git worktree prune' [-n] [-v] [--expire <expire>]
'git worktree unlock' <worktree>
@@ -71,6 +72,11 @@ files from being pruned automatically. This also prevents it from
being moved or deleted. Optionally, specify a reason for the lock
with `--reason`.
+move::
+
+Move a working tree to a new location. Note that the main working tree
+cannot be moved yet.
+
prune::
Prune working tree information in $GIT_DIR/worktrees.
@@ -252,7 +258,6 @@ performed manually, such as:
- `remove` to remove a linked working tree and its administrative files (and
warn if the working tree is dirty)
-- `mv` to move or rename a working tree and update its administrative files
GIT
---
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9a97e37..0d8b57c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -15,6 +15,7 @@ static const char * const worktree_usage[] = {
N_("git worktree add [<options>] <path> [<branch>]"),
N_("git worktree list [<options>]"),
N_("git worktree lock [<options>] <path>"),
+ N_("git worktree move <worktree> <new-path>"),
N_("git worktree prune [<options>]"),
N_("git worktree unlock <path>"),
NULL
@@ -524,6 +525,46 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
return ret;
}
+static int move_worktree(int ac, const char **av, const char *prefix)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ struct strbuf dst = STRBUF_INIT;
+ const char *reason;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac != 2)
+ usage_with_options(worktree_usage, options);
+
+ strbuf_addstr(&dst, prefix_filename(prefix,
+ strlen(prefix),
+ av[1]));
+ if (file_exists(dst.buf))
+ die(_("target '%s' already exists"), av[1]);
+
+ worktrees = get_worktrees(0);
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working directory"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("'%s' is a main working directory"), av[0]);
+ reason = is_worktree_locked(wt);
+ if (reason) {
+ if (*reason)
+ die(_("already locked, reason: %s"), reason);
+ die(_("already locked, no reason"));
+ }
+ if (validate_worktree(wt, 0))
+ return -1;
+
+ if (rename(wt->path, dst.buf) == -1)
+ die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
+
+ return update_worktree_location(wt, dst.buf);
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -546,5 +587,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
return lock_worktree(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "unlock"))
return unlock_worktree(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "move"))
+ return move_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6721ff8..e80392b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
_git_worktree ()
{
- local subcommands="add list lock prune unlock"
+ local subcommands="add list lock move prune unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 8298aaf..74070bd 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -59,4 +59,34 @@ test_expect_success 'unlock worktree twice' '
test_path_is_missing .git/worktrees/source/locked
'
+test_expect_success 'move non-worktree' '
+ mkdir abc &&
+ test_must_fail git worktree move abc def
+'
+
+test_expect_success 'move locked worktree' '
+ git worktree lock source &&
+ test_must_fail git worktree move source destination &&
+ git worktree unlock source
+'
+
+test_expect_success 'move worktree' '
+ git worktree move source destination &&
+ test_path_is_missing source &&
+ git worktree list --porcelain | grep "^worktree" >actual &&
+ cat <<-EOF >expected &&
+ worktree $TRASH_DIRECTORY
+ worktree $TRASH_DIRECTORY/destination
+ worktree $TRASH_DIRECTORY/elsewhere
+ EOF
+ test_cmp expected actual &&
+ git -C destination log --format=%s >actual2 &&
+ echo init >expected2 &&
+ test_cmp expected2 actual2
+'
+
+test_expect_success 'move main worktree' '
+ test_must_fail git worktree move . def
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 4/6] worktree move: accept destination as directory
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Similar to "mv a b/", which is actually "mv a b/a", we extract basename
of source worktree and create a directory of the same name at
destination if dst path is a directory.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 0d8b57c..900b68b 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -541,7 +541,13 @@ static int move_worktree(int ac, const char **av, const char *prefix)
strbuf_addstr(&dst, prefix_filename(prefix,
strlen(prefix),
av[1]));
- if (file_exists(dst.buf))
+ if (is_directory(dst.buf))
+ /*
+ * keep going, dst will be appended after we get the
+ * source's absolute path
+ */
+ ;
+ else if (file_exists(dst.buf))
die(_("target '%s' already exists"), av[1]);
worktrees = get_worktrees(0);
@@ -559,6 +565,17 @@ static int move_worktree(int ac, const char **av, const char *prefix)
if (validate_worktree(wt, 0))
return -1;
+ if (is_directory(dst.buf)) {
+ const char *sep = find_last_dir_sep(wt->path);
+
+ if (!sep)
+ die(_("could not figure out destination name from '%s'"),
+ wt->path);
+ strbuf_addstr(&dst, sep);
+ if (file_exists(dst.buf))
+ die(_("target '%s' already exists"), dst.buf);
+ }
+
if (rename(wt->path, dst.buf) == -1)
die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 5/6] worktree move: refuse to move worktrees with submodules
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Submodules contains .git files with relative paths. After a worktree
move, these files need to be updated or they may point to nowhere.
This is a bandage patch to make sure "worktree move" don't break
people's worktrees by accident. When .git file update code is in
place, this validate_no_submodules() could be removed.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/worktree.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 900b68b..64d0264 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -525,6 +525,27 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
return ret;
}
+static void validate_no_submodules(const struct worktree *wt)
+{
+ struct index_state istate = {0};
+ int i, found_submodules = 0;
+
+ if (read_index_from(&istate, worktree_git_path(wt, "index")) > 0) {
+ for (i = 0; i < istate.cache_nr; i++) {
+ struct cache_entry *ce = istate.cache[i];
+
+ if (S_ISGITLINK(ce->ce_mode)) {
+ found_submodules = 1;
+ break;
+ }
+ }
+ }
+ discard_index(&istate);
+
+ if (found_submodules)
+ die(_("This working tree contains submodules and cannot be moved yet"));
+}
+
static int move_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -565,6 +586,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)
if (validate_worktree(wt, 0))
return -1;
+ validate_no_submodules(wt);
+
if (is_directory(dst.buf)) {
const char *sep = find_last_dir_sep(wt->path);
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* [PATCH 6/6] worktree remove: new command
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 9:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy
In-Reply-To: <20170108094003.637-1-pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Documentation/git-worktree.txt | 21 +++++----
builtin/worktree.c | 79 ++++++++++++++++++++++++++++++++++
contrib/completion/git-completion.bash | 5 ++-
t/t2028-worktree-move.sh | 26 +++++++++++
4 files changed, 121 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 1310513..bbde6b8 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -14,6 +14,7 @@ SYNOPSIS
'git worktree lock' [--reason <string>] <worktree>
'git worktree move' <worktree> <new-path>
'git worktree prune' [-n] [-v] [--expire <expire>]
+'git worktree remove' [--force] <worktree>
'git worktree unlock' <worktree>
DESCRIPTION
@@ -81,6 +82,13 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+remove::
+
+Remove a working tree. Only clean working trees (no untracked files
+and no modification in tracked files) can be removed. Unclean working
+trees can be removed with `--force`. The main working tree cannot be
+removed.
+
unlock::
Unlock a working tree, allowing it to be pruned, moved or deleted.
@@ -90,9 +98,10 @@ OPTIONS
-f::
--force::
- By default, `add` refuses to create a new working tree when `<branch>`
- is already checked out by another working tree. This option overrides
- that safeguard.
+ By default, `add` refuses to create a new working tree when
+ `<branch>` is already checked out by another working tree and
+ `remove` refuses to remove an unclean working tree. This option
+ overrides that safeguard.
-b <new-branch>::
-B <new-branch>::
@@ -253,12 +262,6 @@ Multiple checkout in general is still experimental, and the support
for submodules is incomplete. It is NOT recommended to make multiple
checkouts of a superproject.
-git-worktree could provide more automation for tasks currently
-performed manually, such as:
-
-- `remove` to remove a linked working tree and its administrative files (and
- warn if the working tree is dirty)
-
GIT
---
Part of the linkgit:git[1] suite
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 64d0264..339c622e 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -17,6 +17,7 @@ static const char * const worktree_usage[] = {
N_("git worktree lock [<options>] <path>"),
N_("git worktree move <worktree> <new-path>"),
N_("git worktree prune [<options>]"),
+ N_("git worktree remove [<options>] <worktree>"),
N_("git worktree unlock <path>"),
NULL
};
@@ -605,6 +606,82 @@ static int move_worktree(int ac, const char **av, const char *prefix)
return update_worktree_location(wt, dst.buf);
}
+static int remove_worktree(int ac, const char **av, const char *prefix)
+{
+ int force = 0;
+ struct option options[] = {
+ OPT_BOOL(0, "force", &force,
+ N_("force removing even if the worktree is dirty")),
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ struct strbuf sb = STRBUF_INIT;
+ const char *reason;
+ int ret = 0;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac != 1)
+ usage_with_options(worktree_usage, options);
+
+ worktrees = get_worktrees(0);
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working directory"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("'%s' is a main working directory"), av[0]);
+ reason = is_worktree_locked(wt);
+ if (reason) {
+ if (*reason)
+ die(_("already locked, reason: %s"), reason);
+ die(_("already locked, no reason"));
+ }
+ if (validate_worktree(wt, 0))
+ return -1;
+
+ if (!force) {
+ struct argv_array child_env = ARGV_ARRAY_INIT;
+ struct child_process cp;
+ char buf[1];
+
+ argv_array_pushf(&child_env, "%s=%s/.git",
+ GIT_DIR_ENVIRONMENT, wt->path);
+ argv_array_pushf(&child_env, "%s=%s",
+ GIT_WORK_TREE_ENVIRONMENT, wt->path);
+ memset(&cp, 0, sizeof(cp));
+ argv_array_pushl(&cp.args, "status", "--porcelain", NULL);
+ cp.env = child_env.argv;
+ cp.git_cmd = 1;
+ cp.dir = wt->path;
+ cp.out = -1;
+ ret = start_command(&cp);
+ if (ret)
+ die_errno(_("failed to run git-status on '%s', code %d"),
+ av[0], ret);
+ ret = xread(cp.out, buf, sizeof(buf));
+ if (ret)
+ die(_("'%s' is dirty, use --force to delete it"), av[0]);
+ close(cp.out);
+ ret = finish_command(&cp);
+ if (ret)
+ die_errno(_("failed to run git-status on '%s', code %d"),
+ av[0], ret);
+ }
+ strbuf_addstr(&sb, wt->path);
+ if (remove_dir_recursively(&sb, 0)) {
+ error_errno(_("failed to delete '%s'"), sb.buf);
+ ret = -1;
+ }
+ strbuf_reset(&sb);
+ strbuf_addstr(&sb, git_common_path("worktrees/%s", wt->id));
+ if (remove_dir_recursively(&sb, 0)) {
+ error_errno(_("failed to delete '%s'"), sb.buf);
+ ret = -1;
+ }
+ strbuf_release(&sb);
+ free_worktrees(worktrees);
+ return ret;
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -629,5 +706,7 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
return unlock_worktree(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "move"))
return move_worktree(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "remove"))
+ return remove_worktree(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index e80392b..f413431 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2715,7 +2715,7 @@ _git_whatchanged ()
_git_worktree ()
{
- local subcommands="add list lock move prune unlock"
+ local subcommands="add list lock move prune remove unlock"
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
@@ -2733,6 +2733,9 @@ _git_worktree ()
prune,--*)
__gitcomp "--dry-run --expire --verbose"
;;
+ remove,--*)
+ __gitcomp "--force"
+ ;;
*)
;;
esac
diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 74070bd..084acc6 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh
@@ -89,4 +89,30 @@ test_expect_success 'move main worktree' '
test_must_fail git worktree move . def
'
+test_expect_success 'remove main worktree' '
+ test_must_fail git worktree remove .
+'
+
+test_expect_success 'remove locked worktree' '
+ git worktree lock destination &&
+ test_must_fail git worktree remove destination &&
+ git worktree unlock destination
+'
+
+test_expect_success 'remove worktree with dirty tracked file' '
+ echo dirty >>destination/init.t &&
+ test_must_fail git worktree remove destination
+'
+
+test_expect_success 'remove worktree with untracked file' '
+ git -C destination checkout init.t &&
+ : >destination/untracked &&
+ test_must_fail git worktree remove destination
+'
+
+test_expect_success 'force remove worktree with untracked file' '
+ git worktree remove --force destination &&
+ test_path_is_missing destination
+'
+
test_done
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [ANNOUNCE] git-test: run automated tests against a range of Git commits
From: Michael Haggerty @ 2017-01-08 9:52 UTC (permalink / raw)
To: Jeff King; +Cc: git discussion list
In-Reply-To: <20170107071832.2rucap3rskzmkgq4@sigill.intra.peff.net>
On 01/07/2017 08:18 AM, Jeff King wrote:
> On Fri, Jan 06, 2017 at 04:52:16PM +0100, Michael Haggerty wrote:
>
>> I just released `git test`, a script for running automated
>> tests across a range of Git commits and keeping track of the results in
>> git notes:
>>
>> https://github.com/mhagger/git-test
>>
>> This is a script that I've been using in one form or another for years
>> and I find it really handy [1].
>
> Neat. I usually "git rebase -x 'make -j8 test' @{u}" after finishing a
> topic to make sure the intermediate steps are good. But it would be neat
> to have this running continuously in the background to alert me to
> problems sooner (and the key thing there is that it remembers
> already-run tests, so it should be safe to basically for new commits
> every 10 seconds or so).
>
> I did hit a few interesting cases trying out "git test". So here's a
> narrative, and you can pick out where there may be room for improvement
> in the tool, and where I'm just being dumb. :)
>
> I tried it out first on a topic I finished earlier today, which has 3
> commits. So I did:
>
> $ git test add 'make -j8 test'
> $ git test range @{u}..HEAD
>
> It barfed on the first commit, because the script expects "git co" to
> work, but I don't have that alias. No big deal (and I already submitted
> a PR to fix it).
I make the same mistake in most of my scripts :-/ Thanks for the PR; I
merged it.
> So then I reinvoked it like:
>
> $ git test range @{u}..HEAD
>
> and it actually ran some tests. Yay.
>
> And then of course I wanted to prove to myself how cool the notes
> feature is, so I ran it again. It didn't run any tests this time. Yay
> again. But there were a few surprises:
>
> $ git test range @{u}..HEAD
> setup_test default
> Using test default; command: make -j8 test
> Old status: bad
> Tree 9fcdbd5c78^{tree} is already known to be bad!
> Old status: good
> Tree c22f4f6624^{tree} is already known to be good.
> Old status: good
> Tree 19e2e62e5e^{tree} is already known to be good.
> Already on 'jk/wait-for-child-cleanup'
> Your branch is ahead of 'origin/master' by 3 commits.
>
> ALL TESTS SUCCESSFUL
>
> My initial run with "git co" had left the first commit marked as "bad".
> That's not _too_ surprising, since it did indeed fail. I think it's
> probably a bug to record a failure note, though, if checking out fails.
> It's not necessarily an immutable property of the tree.
I definitely agree. This was an oversight which I just fixed.
> [...]
>
> The second thing that surprised me was "ALL TESTS SUCCESSFUL", when
> clearly one of them was known-bad. :)
Replayed results weren't being treated internally as failures. That's
fixed, too.
> So at this point I knew I needed to re-run the test. Looks like there's
> a "--force" option. Let's try it. There's no need to re-run the other
> two, so let's just give it one commit:
>
> $ git test range -f HEAD~2
> ...
> Object 95649d6cf9ec68f05d1dc57ec1b989b8d263a7ae^{tree} has no note
> Object e1970ce43abfbf625bce68516857e910748e5965^{tree} has no note
> Object 368f99d57e8ed17243f2e164431449d48bfca2fb^{tree} has no note
> Object ceede59ea90cebad52ba9c8263fef3fb6ef17593^{tree} has no note
> Object dfe070511c652f2b8e1bf6540f238c9ca9ba41d3^{tree} has no note
> Object 902d960b382a0cd424618ff4e1316da40e4be2f6^{tree} has no note
> ...
>
> This started spewing out many lines like the one above, until I hit ^C.
> Yikes!
>
> [...]
> I see the symmetry and simplicity in allowing the user to specify a full
> range. But it also seems like it's easy to make a mistake that's going
> to want to test a lot of commits. I wonder if it should complain when
> there's no lower bound to the commit range. Or alternatively, if there's
> a single positive reference, treat it as a lower bound, with HEAD as the
> upper bound (which is vaguely rebase-like).
I see how this might be unexpected, and it's definitely inconvenient at
some times (like when you want to test a single commit). I thought it
would be nice to allow arbitrary `rev-list` expressions (albeit
currently only a single word), but I think that you are right that other
semantics would be more convenient.
I'm thinking of maybe
* If an argument matches `*..*`, pass it to `rev-list` (like now).
* Otherwise, treat each argument as a single commit/tree (i.e., pass it
to `rev-parse`).
* If no argument is specified, test `@{u}..` (assuming that an
upstream is configured). Though actually, this won't be as
convenient as it sounds, because (a) `git test` is often run
in a separate worktree, and (2) on errors, it currently leaves the
repository with a detached `HEAD`.
* Support a `--stdin` option, to read a list of commits/trees to test
from standard input. By this mechanism, users could use arbitrary
`rev-list` commands to choose what to test.
> A few other observations about the note deletion:
>
> - The "has no note" message should perhaps be suppressed. We're just
> trying to overwrite the value if there is one (alternatively,
> instead of removing it, just overwrite it, so the old note stays
> until we get a result one way or the other).
Yes. That's a one-time problem that I haven't seen in a long time. The
script is overly chatty in general.
Aside: It would be nice if `git notes` had a subcommand to initialize a
note reference with an empty tree. (I know how to do it longhand, but
it's awkward and it should be possible to do it via the `notes` interface.)
I think ideally `git notes add` would look for pre-existing notes, and:
* If none are found, create an empty notes reference.
* If pre-existing notes are found and there was no existing test with
that name, probably just leave the old notes in place.
* If pre-existing notes are found and there was already a test with
that name but a different command, perhaps insist that the user
decide explicitly whether to forget the old results or continue using
them. This might help users avoid the mistake of re-using old results
even if they change the manner of testing.
> - It was sufficiently slow that it looks like we invoke "git notes
> remove" once per commit. It would be a lot more efficient to batch
> them (not just in terms of process startup, but because you're going
> to write a _ton_ of intermediate notes trees).
>
> Of course none of that matters if you don't do something stupid like
> trying to "git test" 45,000 commits. :)
Yeah, I've never experienced that problem myself :-P But I see that
notes supports `git notes remove --ignore-missing --stdin`, so that will
be easy to implement.
> [...]
> It would be even easier if I could just repeat my range and only re-test
> the "bad" commits. It was then that I decided to actually read the rest
> of "git test help range" and see that you already wrote such an option,
> cleverly hidden under the name "--retest".
I think you were being ironic, but if not, would this have been easier
to find under another name?
> And one final nit. I notice there is also a "--keep-going" option. Which
> made me surprised that we bothered to test HEAD~1 and HEAD, when we knew
> that HEAD~2 was bogus. I suspect this is related to the "ALL TESTS
> SUCCESSFUL" issue.
Yes, that's part of the same bug from above.
> So those were all little cosmetic things. The other big thing I wanted
> to see was what it's like to fix a bug deep in a topic. So I used "git
> rebase -i" to inset a compile error into the first commit of my 3-patch
> series. And then I tested it:
>
> $ git test add -t compile 'make -j8'
> $ git test range -t compile HEAD~3..
>
> As predicted, it stopped at the first commit and told me it was buggy.
> But I'm dumped onto a detached HEAD, and I'm on my own to actually get
> the working tree to a state where I can test and fix on my actual
> branch.
Yeah, this is awkward, not only because many people don't know what to
make of detached HEAD, but also because it makes it awkward in general
to use `git test` in your main working directory. I didn't model this
behavior on `git rebase --interactive`'s `edit` command, because I
rarely use that. But I can see how they would fit together pretty well
for people who like that workflow.
I've considered that rather than leave you in a detached HEAD state,
maybe `git test` should always restore your old branch. But it seems
like it would more often be useful to be in the directory with the
broken commit checked out and any test results, coredumps, etc intact.
I would definitely like to implement a `git test reset` command that
returns you to your initial branch (like `git bisect reset`).
I like your idea of a `git test fix` command:
> [...]
> I think it should be possible to script the next steps, though.
> Something like like "git test fix foo", which would:
>
> - expand the range of foo@{u}..foo to get the list of commits
>
> - see which ones were marked as broken
>
> - kick off an interactive rebase, but override GIT_EDITOR to mark any
> broken ones as "edit" instead of "pick"
>
> That lets you separate the act of testing from the act of fixing. You
> can let the tester run continuously in the background, and only stop to
> fix when you're at an appropriate point in your work.
I think you would usually only want to mark only the *first* broken
commit as "edit", because often errors cascade to descendant commits.
Thanks for all the great feedback!
Michael
^ permalink raw reply
* [PATCH v3] log --graph: customize the graph lines with config log.graphColors
From: Nguyễn Thái Ngọc Duy @ 2017-01-08 10:13 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy
In-Reply-To: <xmqqzijjd34j.fsf@gitster.mtv.corp.google.com>
If you have a 256 colors terminal (or one with true color support), then
the predefined 12 colors seem limited. On the other hand, you don't want
to draw graph lines with every single color in this mode because the two
colors could look extremely similar. This option allows you to hand pick
the colors you want.
Even with standard terminal, if your background color is neither black
or white, then the graph line may match your background and become
hidden. You can exclude your background color (or simply the colors you
hate) with this.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Compared to v2:
* set_column_colors_by_config() is renamed to set_column_colors()
* no memory leak if the function is called the second time
* proper space trimming since color_parse_mem expects so
* fixed the warning message, giving some context
* at least one test to exercise the code
* I'm not going with the cumulative behavior because I think that's
just harder to manage colors, and we would need a way to remove
colors from the config too.
Documentation/config.txt | 4 ++++
graph.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
t/t4202-log.sh | 22 ++++++++++++++++++++++
3 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..33a007b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2003,6 +2003,10 @@ log.follow::
i.e. it cannot be used to follow multiple files and does not work well
on non-linear history.
+log.graphColors::
+ A list of colors, separated by commas, that can be used to draw
+ history lines in `git log --graph`.
+
log.showRoot::
If true, the initial commit will be shown as a big creation event.
This is equivalent to a diff against an empty tree.
diff --git a/graph.c b/graph.c
index dd17201..048f5cb 100644
--- a/graph.c
+++ b/graph.c
@@ -62,6 +62,49 @@ enum graph_state {
static const char **column_colors;
static unsigned short column_colors_max;
+static void set_column_colors(void)
+{
+ static char **colors;
+ static int colors_max, colors_alloc;
+ char *string = NULL;
+ const char *end, *start;
+ int i;
+
+ for (i = 0; i < colors_max; i++)
+ free(colors[i]);
+ if (colors)
+ free(colors[colors_max]);
+ colors_max = 0;
+
+ if (git_config_get_string("log.graphcolors", &string)) {
+ graph_set_column_colors(column_colors_ansi,
+ column_colors_ansi_max);
+ return;
+ }
+
+ start = string;
+ end = string + strlen(string);
+ while (start < end) {
+ const char *comma = strchrnul(start, ',');
+ char color[COLOR_MAXLEN];
+
+ while (start < comma && isspace(*start))
+ start++;
+
+ if (!color_parse_mem(start, comma - start, color)) {
+ ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+ colors[colors_max++] = xstrdup(color);
+ } else
+ warning(_("ignore invalid color '%.*s' in log.graphColors"),
+ (int)(comma - start), start);
+ start = comma + 1;
+ }
+ free(string);
+ ALLOC_GROW(colors, colors_max + 1, colors_alloc);
+ colors[colors_max] = xstrdup(GIT_COLOR_RESET);
+ graph_set_column_colors((const char **)colors, colors_max);
+}
+
void graph_set_column_colors(const char **colors, unsigned short colors_max)
{
column_colors = colors;
@@ -208,8 +251,7 @@ struct git_graph *graph_init(struct rev_info *opt)
struct git_graph *graph = xmalloc(sizeof(struct git_graph));
if (!column_colors)
- graph_set_column_colors(column_colors_ansi,
- column_colors_ansi_max);
+ set_column_colors();
graph->commit = NULL;
graph->revs = opt;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index e2db47c..afe0715 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -313,6 +313,28 @@ test_expect_success 'log --graph with merge' '
test_cmp expect actual
'
+cat > expect.colors <<\EOF
+* Merge branch 'side'
+<BLUE>|<RESET><CYAN>\<RESET>
+<BLUE>|<RESET> * side-2
+<BLUE>|<RESET> * side-1
+* <CYAN>|<RESET> Second
+* <CYAN>|<RESET> sixth
+* <CYAN>|<RESET> fifth
+* <CYAN>|<RESET> fourth
+<CYAN>|<RESET><CYAN>/<RESET>
+* third
+* second
+* initial
+EOF
+
+test_expect_success 'log --graph with merge with log.graphColors' '
+ test_config log.graphColors " blue , cyan , red " &&
+ git log --color=always --graph --date-order --pretty=tformat:%s |
+ test_decode_color | sed "s/ *\$//" >actual &&
+ test_cmp expect.colors actual
+'
+
test_expect_success 'log --raw --graph -m with merge' '
git log --raw --graph --oneline -m master | head -n 500 >actual &&
grep "initial" actual
--
2.8.2.524.g6ff3d78
^ permalink raw reply related
* Re: [PATCH] Makefile: put LIBS after LDFLAGS for imap-send
From: Johannes Schindelin @ 2017-01-08 11:54 UTC (permalink / raw)
To: Steven Penny; +Cc: git
In-Reply-To: <20170108061238.2604-1-svnpenn@gmail.com>
Hi Steven,
On Sun, 8 Jan 2017, Steven Penny wrote:
> This matches up with the targets git-%, git-http-fetch, git-http-push
> and git-remote-testsvn. It must be done this way on Windows else lcrypto
> cannot find lgdi32 and lws2_32
I am curious: how do you build Git? I ask because I build Git on Windows
many times a day, and I did not encounter any link problems. This hints at
a difference of build environment (I use the Git for Windows SDK) that
needs to be mentioned in the commit message.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] Makefile: put LIBS after LDFLAGS for imap-send
From: Steven Penny @ 2017-01-08 15:12 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1701081250580.3469@virtualbox>
On Sun, Jan 8, 2017 at 5:54 AM, Johannes Schindelin wrote:
> I am curious: how do you build Git? I ask because I build Git on Windows
> many times a day, and I did not encounter any link problems.
My end goal is to build static native Windows Git via Cygwin and the
mingw64-x86_64-gcc-core package. This is certainly possible but definitely not
considered in the current Git codebase. I have a patch to config.mak.uname
coming as well.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox