* Re: [PATCH v2 0/5] nd/worktree-list-fixup
From: Junio C Hamano @ 2016-11-28 21:25 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, rappazzo
In-Reply-To: <20161128093656.15744-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This version
>
> * changes get_worktrees() to take a flag, and adds one flag for
> sorting.
>
> * adds tests for both the 'main worktree always present' and the
> sorting problems.
>
> * reworks 3/5 a bit, keep changes closer, easier to see the cause and
> consequence.
>
> Nguyễn Thái Ngọc Duy (5):
> worktree.c: zero new 'struct worktree' on allocation
> worktree: reorder an if statement
> get_worktrees() must return main worktree as first item even on error
> worktree.c: get_worktrees() takes a new flag argument
> worktree list: keep the list sorted
>
> branch.c | 2 +-
> builtin/branch.c | 2 +-
> builtin/worktree.c | 14 ++++++++------
> t/t2027-worktree-list.sh | 40 ++++++++++++++++++++++++++++++++++++++++
> worktree.c | 42 +++++++++++++++++++++---------------------
> worktree.h | 4 +++-
> 6 files changed, 74 insertions(+), 30 deletions(-)
Thanks for a pleasant read. Will replace what has been queued.
^ permalink raw reply
* Re: [PATCH v2 00/11] git worktree (re)move
From: Johannes Sixt @ 2016-11-28 21:25 UTC (permalink / raw)
To: Junio C Hamano, Nguyễn Thái Ngọc Duy
Cc: Johannes Schindelin, git
In-Reply-To: <xmqqshqbicga.fsf@gitster.mtv.corp.google.com>
Am 28.11.2016 um 21:20 schrieb Junio C Hamano:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Does this round address the issue raised in
>>
>> http://public-inbox.org/git/alpine.DEB.2.20.1611161041040.3746@virtualbox
>>
>> by Dscho?
>>
>> Even if you are not tracking a fifo, for example, your working tree
>> may have one created in t/trash* directory during testing, and
>> letting platform "cp -r" taking care of it (if that is possible---I
>> didn't look at the code that calls busybox copy to see if you are
>> doing something exotic or just blindly copying everything in the
>> directory) may turn out to be a more portable way to do this, and I
>> suspect that the cost of copying one whole-tree would dominate the
>> run_command() overhead.
>
> Please do not take the above as me saying "you must spawn the
> platform cp -r".
copy_dir_recursively is used in 'worktree move' when the move is across
file systems. My stance on it is to punt in this case. *I* would not
trust Git, or any other program that is not *specifically* made to copy
a whole directory structure, to get all cases right when a simple
rename() is not sufficent. And, uh, oh, it does a
remove_dir_recursively() after it has finshed copying. No, Git is not a
tool to move directories, thank you very much!
-- Hannes
^ permalink raw reply
* Partial fetch?
From: Dāvis Mosāns @ 2016-11-28 20:34 UTC (permalink / raw)
To: git
I'm trying to fetch a remote repository over https but sadly it
timeouts too soon.
$ git fetch -v upstream
POST git-upload-pack (gzip 1148 to 641 bytes)
POST git-upload-pack (gzip 1148 to 644 bytes)
POST git-upload-pack (gzip 1948 to 1038 bytes)
POST git-upload-pack (gzip 3548 to 1845 bytes)
POST git-upload-pack (gzip 6748 to 3431 bytes)
POST git-upload-pack (gzip 13148 to 6659 bytes)
POST git-upload-pack (gzip 25948 to 13084 bytes)
POST git-upload-pack (gzip 51548 to 25997 bytes)
POST git-upload-pack (gzip 102748 to 51375 bytes)
POST git-upload-pack (gzip 205148 to 101973 bytes)
POST git-upload-pack (gzip 409948 to 203200 bytes)
POST git-upload-pack (gzip 82248 to 41185 bytes)
POST git-upload-pack (gzip 90448 to 45257 bytes)
POST git-upload-pack (gzip 99448 to 49719 bytes)
POST git-upload-pack (gzip 109348 to 54623 bytes)
POST git-upload-pack (gzip 120248 to 59937 bytes)
POST git-upload-pack (gzip 132248 to 65839 bytes)
POST git-upload-pack (gzip 145448 to 72450 bytes)
POST git-upload-pack (gzip 159948 to 79682 bytes)
POST git-upload-pack (gzip 175898 to 87436 bytes)
POST git-upload-pack (gzip 193448 to 96171 bytes)
POST git-upload-pack (gzip 212748 to 105783 bytes)
POST git-upload-pack (gzip 233998 to 116207 bytes)
POST git-upload-pack (gzip 257398 to 127793 bytes)
POST git-upload-pack (gzip 283098 to 140511 bytes)
POST git-upload-pack (gzip 311348 to 154447 bytes)
POST git-upload-pack (gzip 342448 to 169815 bytes)
POST git-upload-pack (gzip 376648 to 186850 bytes)
POST git-upload-pack (gzip 414298 to 205408 bytes)
POST git-upload-pack (gzip 455698 to 225879 bytes)
POST git-upload-pack (gzip 501248 to 248389 bytes)
POST git-upload-pack (gzip 551298 to 273131 bytes)
POST git-upload-pack (gzip 606398 to 300334 bytes)
POST git-upload-pack (gzip 666998 to 330414 bytes)
POST git-upload-pack (gzip 733698 to 363387 bytes)
POST git-upload-pack (gzip 807048 to 399647 bytes)
POST git-upload-pack (gzip 689453 to 341449 bytes)
error: RPC failed; HTTP 504 curl 22 The requested URL returned error:
504 Gateway Time-out
fatal: The remote end hung up unexpectedly
Is there some way to fetch partially by smaller chunks and then repeat
that again till everything is fetched?
Thanks!
^ permalink raw reply
* Re: [PATCH v2 00/11] git worktree (re)move
From: Junio C Hamano @ 2016-11-28 20:20 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: Johannes Schindelin, git
In-Reply-To: <xmqqwpfnidxm.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Does this round address the issue raised in
>
> http://public-inbox.org/git/alpine.DEB.2.20.1611161041040.3746@virtualbox
>
> by Dscho?
>
> Even if you are not tracking a fifo, for example, your working tree
> may have one created in t/trash* directory during testing, and
> letting platform "cp -r" taking care of it (if that is possible---I
> didn't look at the code that calls busybox copy to see if you are
> doing something exotic or just blindly copying everything in the
> directory) may turn out to be a more portable way to do this, and I
> suspect that the cost of copying one whole-tree would dominate the
> run_command() overhead.
Please do not take the above as me saying "you must spawn the
platform cp -r".
A more traditional alternative solution seen on this list is to work
together, leveraging expertise of each participant. From the build
log Dscho gave us, it seems that his Windows port lack at least
POSIX emulation for lchown, mknod, utimes and chown. It is hard to
decide without involving Windows expert what the best way to deal
with it in the code (e.g. To stub or #ifdef out these calls?
Provide suitable emulation in compat/? Something else?), and what
things other than these four are still missing.
^ permalink raw reply
* Re: [PATCH v2 00/11] git worktree (re)move
From: Junio C Hamano @ 2016-11-28 19:48 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, Johannes Schindelin; +Cc: git
In-Reply-To: <20161128094319.16176-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> v2 contains some style fix and adapts to the new get_worktrees() api
> from nd/worktree-list-fixup (which means it can't be built without
> that series).
>
> Nguyễn Thái Ngọc Duy (11):
> copy.c: import copy_file() from busybox
> copy.c: delete unused code in copy_file()
> copy.c: convert bb_(p)error_msg to error(_errno)
> copy.c: style fix
> copy.c: convert copy_file() to copy_dir_recursively()
> 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 | 181 ++++++++++++++++
> cache.h | 1 +
> contrib/completion/git-completion.bash | 5 +-
> copy.c | 369 +++++++++++++++++++++++++++++++++
> t/t2028-worktree-move.sh | 56 +++++
> worktree.c | 84 ++++++++
> worktree.h | 11 +
> 8 files changed, 724 insertions(+), 11 deletions(-)
Does this round address the issue raised in
http://public-inbox.org/git/alpine.DEB.2.20.1611161041040.3746@virtualbox
by Dscho?
Even if you are not tracking a fifo, for example, your working tree
may have one created in t/trash* directory during testing, and
letting platform "cp -r" taking care of it (if that is possible---I
didn't look at the code that calls busybox copy to see if you are
doing something exotic or just blindly copying everything in the
directory) may turn out to be a more portable way to do this, and I
suspect that the cost of copying one whole-tree would dominate the
run_command() overhead.
^ permalink raw reply
* Re: [PATCH 0/3] Minor fixes on 'git worktree'
From: Junio C Hamano @ 2016-11-28 19:36 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List, Mike Rappazzo
In-Reply-To: <CACsJy8CpySQs07cfioOnoxoH1TALRePGbeVCc45ZqDr43519qQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> An interesting thing maybe worth
> sharing is, if HEAD is broken (the only reason we would fail to create
> "struct worktree" for main worktree), then "git worktree list" from
> main worktree would fail too, because repo setup code fails to parse
> HEAD as well and die().
Yeah, that makes sense.
> That makes the "always show main worktree"
> patch useless, since we won't get far enough to execute
> get_worktrees() in the first place. However, you can still do "git
> worktree list" from a linked worktree, gitdir setup code will not stop
> you (how can it). And the patch is still needed.
Yes.
^ permalink raw reply
* Re: [PATCH] l10n: de.po: translate 210 new message
From: Junio C Hamano @ 2016-11-28 19:29 UTC (permalink / raw)
To: Ralf Thielow
Cc: Jiang Xin, Git List, Thomas Rast, Jan Krüger,
Christian Stimming, Phillip Sz, Matthias Rüster,
Magnus Görlitz
In-Reply-To: <CAN0XMOJ0mQ7KF_f2dh9YFA62a4RxoYJMkT7HXUpArK5CdHimew@mail.gmail.com>
Ralf Thielow <ralf.thielow@gmail.com> writes:
> 2016-11-28 15:21 GMT+01:00 Jiang Xin <worldhello.net@gmail.com>:
>> 2016-11-25 2:25 GMT+08:00 Ralf Thielow <ralf.thielow@gmail.com>:
>>> #: sequencer.c:251
>>> -#, fuzzy, c-format
>>> +#, c-format
>>> msgid "could not write eol to '%s"
>>
>> Unmatched single quote has been fixed in l10n round 3.
>> You can rebase and update de.po file.
>>
>> BTW, Git 2.11.0 will be released tomorrow, please send PR in time.
>
> I've rebased, fixed the subject line and send a pull request to you.
Thanks, both.
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Junio C Hamano @ 2016-11-28 19:27 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <alpine.DEB.2.20.1611281830040.117539@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> However, I have been bitten time and again by problems that occurred only
> in production, our test suite (despite taking already waaaaaay too long to
> be truly useful in my daily development) was simply not good enough.
>
> So my plan was different: to let end users opt-in to test this new beast
> thoroughly, more thoroughly than any review would.
I agree with that 100%.
We need to ensure "fallback to known working code" escape hatch is
robust for that plan to work well, and that is why (1) I have been
more focused on getting 1/2 right, and (2) I do not think it should
be Windows-only like in your early plan, and (3) I do not think it
would be "this will merely be there for only a month or so", like
you said earlier.
> And for that, environment variables are just not an option. I need
> something that can be configured in a portable application, so that the
> main Git for Windows installation is unaffected.
I am not sure I follow here.
Are you saying that the users who are opting into the experiment
will keep two installations, one for daily use that avoids getting
hit by the experimental code and the other that is used for testing?
How are they switching between the two? By using different %PATH%?
I am not sure how it is different from setting an environment
$GIT_TEST_BUILTIN_DIFFTOOL.
In any case, I do not have strong preference between environment and
configuration. If you can make 1/2 robust with configuration, that
is just as well. My message you are responding to was merely to
suggest another possibility.
The latter two points in my four-bullet list are hopefully still
viable if you go with the configuration; it may go something like:
- The bulk of the tests is moved into a common dot-sourced file,
with (1) PERL prerequite stripped and (2) "git difftool" replaced
with $git_difftool
- Two test files do one of
git_difftool="git difftool"
git_difftool="git -c difftool.useBuiltin=true difftool"
and include the dot-sourced file. The one that does the former
needs to give up early depending on PERL prerequisite.
perhaps.
> My original "create a file in libexec/git-core/" was simple, did the job
> reliably, and worked also for testing.
It may have been OK for quick-and-dirty hack during development, but
I do not think it was good in anything released.
^ permalink raw reply
* Re: [PATCH] allow git-p4 to create shelved changelists
From: Junio C Hamano @ 2016-11-28 19:06 UTC (permalink / raw)
To: Vinicius Kursancew, Luke Diamand, Lars Schneider; +Cc: git
In-Reply-To: <1480325598-12344-1-git-send-email-viniciusalexandre@gmail.com>
Vinicius Kursancew <viniciusalexandre@gmail.com> writes:
> This patch adds a "--shelve" option to the submit subcommand, it will
> save the changes to a perforce shelve instead of commiting them.
>
> Vinicius Kursancew (1):
> git-p4: allow submit to create shelved changelists.
>
> Documentation/git-p4.txt | 5 +++++
> git-p4.py | 36 ++++++++++++++++++++++--------------
> t/t9807-git-p4-submit.sh | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 58 insertions(+), 14 deletions(-)
Thanks, but I am a wrong person to review this change, so I'll
summon two people who appear in "git shortlog --since=18.months"
output to help review it.
^ permalink raw reply
* [PATCH] diff: handle --no-abbrev outside of repository
From: Jack Bates @ 2016-11-28 18:25 UTC (permalink / raw)
To: git; +Cc: Jack Bates
The "git diff --no-index" codepath
doesn't handle the --no-abbrev option.
Signed-off-by: Jack Bates <jack@nottheoilrig.com>
---
diff.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index ec87283..0447eff 100644
--- a/diff.c
+++ b/diff.c
@@ -3106,7 +3106,8 @@ static const char *diff_abbrev_oid(const struct object_id *oid, int abbrev)
abbrev = FALLBACK_DEFAULT_ABBREV;
if (abbrev > GIT_SHA1_HEXSZ)
die("BUG: oid abbreviation out of range: %d", abbrev);
- hex[abbrev] = '\0';
+ if (abbrev)
+ hex[abbrev] = '\0';
return hex;
}
}
@@ -4024,6 +4025,8 @@ int diff_opt_parse(struct diff_options *options,
offending, optarg);
return argcount;
}
+ else if (!strcmp(arg, "--no-abbrev"))
+ options->abbrev = 0;
else if (!strcmp(arg, "--abbrev"))
options->abbrev = DEFAULT_ABBREV;
else if (skip_prefix(arg, "--abbrev=", &arg)) {
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges
From: Junio C Hamano @ 2016-11-28 18:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Markus Klein
In-Reply-To: <xmqqmvgjjvk5.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Looking at the commit you blamed, what happened in this case before
> that change was that
>
> (1) make_cache_entry() would have called refresh_cache_entry() with
> CE_MATCH_REFRESH and returned a NULL;
>
> (2) merge-recursive.c::add_cacheinfo() noticed NULL and did
>
> return error(_("addinfo_cache failed for path '%s'"), path)
>
> But the updated code forgot that refresh_cache_entry() could return
> NULL. So 1335d76e45 ("merge: avoid "safer crlf" during recording of
> merge results", 2016-07-08) was not a faithful rewrite.
I'd tentatively queue the two patches fro you on top of the
jc/renormalize-merge-kill-safer-crlf topic that ends at 1335d76e45
("merge: avoid "safer crlf" during recording of merge results",
2016-07-08). The real "fix" became like this with the above
analysis. Semantic adjustment "error(" -> "err(o," between the old
codebase and the current one will be handled when merging.
Thanks for catching my incorrect refactoring.
-- >8 --
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sat, 26 Nov 2016 13:48:06 +0100
Subject: [PATCH] merge-recursive: handle NULL in add_cacheinfo() correctly
1335d76e45 ("merge: avoid "safer crlf" during recording of merge
results", 2016-07-08) tried to split make_cache_entry() call made
with CE_MATCH_REFRESH into a call to make_cache_entry() without one,
followed by a call to add_cache_entry(), refresh_cache() and another
add_cache_entry() as needed. However the conversion was botched in
that it forgot that refresh_cache() can return NULL, which was handled
correctly in make_cache_entry() but not in the updated code.
This fixes https://github.com/git-for-windows/git/issues/952
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
merge-recursive.c | 2 ++
t/t3501-revert-cherry-pick.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/merge-recursive.c b/merge-recursive.c
index de37e5153c..56385d4c01 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -213,6 +213,8 @@ static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,
struct cache_entry *nce;
nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
+ if (!nce)
+ return error(_("addinfo_cache failed for path '%s'"), path);
if (nce != ce)
ret = add_cache_entry(nce, options);
}
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index 5bef564ff1..22970d2223 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -141,7 +141,7 @@ test_expect_success 'cherry-pick "-" works with arguments' '
test_cmp expect actual
'
-test_expect_failure 'cherry-pick works with dirty renamed file' '
+test_expect_success 'cherry-pick works with dirty renamed file' '
test_commit to-rename &&
git checkout -b unrelated &&
test_commit unrelated &&
--
2.11.0-rc3-172-gc8d0e450d3
^ permalink raw reply related
* Re: [PATCH v2 2/2] Avoid a segmentation fault with renaming merges
From: Junio C Hamano @ 2016-11-28 18:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Markus Klein
In-Reply-To: <alpine.DEB.2.20.1611261348420.117539@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sat, 26 Nov 2016, Johannes Schindelin wrote:
>
>> diff --git a/merge-recursive.c b/merge-recursive.c
>> index 9041c2f149..609061f58a 100644
>> --- a/merge-recursive.c
>> +++ b/merge-recursive.c
>> @@ -235,6 +235,8 @@ static int add_cacheinfo(struct merge_options *o,
>> struct cache_entry *nce;
>>
>> nce = refresh_cache_entry(ce, CE_MATCH_REFRESH | CE_MATCH_IGNORE_MISSING);
>> + if (!nce)
>> + return err(o, _("addinfo: '%s' is not up-to-date"), path);
>> if (nce != ce)
>> ret = add_cache_entry(nce, options);
>> }
>
> BTW I was not quite sure why we need to refresh the cache entry here, and
> 1335d76e45 (merge: avoid "safer crlf" during recording of merge results,
> 2016-07-08) has a commit message for which I need some time to wrap my
> head around.
This callsite used to call make_cache_entry() with CE_MATCH_REFRESH,
which creates a new cache entry, calls refresh_cache_ent, and
returns the cache entry"; the log message attempts to explain why we
avoid passing CE_MATCH_REFRESH and instead first add it as a merged
entry and then refresh it (and re-add it if ce got changed). We
used to leave the old (possibly conflicted) entries for the same
path in the index while refreshing the new cache entry, which has
correctly converted result, and the old entries got in the way,
attempting a wrong eol conversion and declaring that the new entry
out-of-date. By adding the correctly converted result as a merged
entry, which gets rid of the old entries, the refresh operation will
not be corrupted by them.
> Also, an error here may be overkill. Maybe we should simply change the "if
> (nce != ce)" to an "if (nce && nce != ce)" here, as a locally-modified
> file will give a nicer message later, anyway.
Looking at the commit you blamed, what happened in this case before
that change was that
(1) make_cache_entry() would have called refresh_cache_entry() with
CE_MATCH_REFRESH and returned a NULL;
(2) merge-recursive.c::add_cacheinfo() noticed NULL and did
return error(_("addinfo_cache failed for path '%s'"), path)
But the updated code forgot that refresh_cache_entry() could return
NULL. So 1335d76e45 ("merge: avoid "safer crlf" during recording of
merge results", 2016-07-08) was not a faithful rewrite.
So I agree that your patch is the right fix; using the old message
lost by mistake in 1335d76e45 may have made it more clear that this
is a fix for a misconversion in that commit, though.
In any case, this does not seem like a new regression (1/2 applied
on v2.10.0 dies the same way), so I am inclined to queue these two
but not ship in the upcoming release for now.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2016, #05; Wed, 23)
From: Junio C Hamano @ 2016-11-28 18:08 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8DaOB=ybWFbcqFgZ_U_JYWFUXNDJRmG6_-S3raWh6AWaQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Thu, Nov 24, 2016 at 6:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> * nd/rebase-forget (2016-10-28) 1 commit
>> - rebase: add --forget to cleanup rebase, leave HEAD untouched
>>
>> "git rebase" learned "--forget" option, which allows a user to
>> remove the metadata left by an earlier "git rebase" that was
>> manually aborted without using "git rebase --abort".
>>
>> Waiting for a reroll.
>
> The reroll was http://public-inbox.org/git/%3C20161112020041.2335-1-pclouds@gmail.com%3E/
Thanks.
^ permalink raw reply
* Re: [PATCH v2] date-formats.txt: Typo fix
From: Junio C Hamano @ 2016-11-28 18:06 UTC (permalink / raw)
To: Luis Ressel; +Cc: git
In-Reply-To: <20161125173657.9656-1-aranea@aixah.de>
Luis Ressel <aranea@aixah.de> writes:
> Last time I checked, I was living in the UTC+01:00 time zone. UTC+02:00
> would be Central European _Summer_ Time.
> ---
Thanks; please sign-off your patches, even a trivial ones like this.
> Documentation/date-formats.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt
> index 35e8da2..6926e0a 100644
> --- a/Documentation/date-formats.txt
> +++ b/Documentation/date-formats.txt
> @@ -11,7 +11,7 @@ Git internal format::
> It is `<unix timestamp> <time zone offset>`, where `<unix
> timestamp>` is the number of seconds since the UNIX epoch.
> `<time zone offset>` is a positive or negative offset from UTC.
> - For example CET (which is 2 hours ahead UTC) is `+0200`.
> + For example CET (which is 1 hour ahead of UTC) is `+0100`.
>
> RFC 2822::
> The standard email format as described by RFC 2822, for example
^ permalink raw reply
* Re: [PATCH 31/35] pathspec: allow querying for attributes
From: Brandon Williams @ 2016-11-28 18:03 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, pclouds, git
In-Reply-To: <20161110203428.30512-32-sbeller@google.com>
On 11/10, Stefan Beller wrote:
> +static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)
> +{
>
> [...]
>
> + if (!item->attr_check)
> + git_attr_check_initv(&item->attr_check, attrs.argv);
> + else
> + die(_("Only one 'attr:' specification is allowed."));
> +
> + argv_array_clear(&attrs);
> + string_list_clear(&list, 0);
> + return;
> +}
Unnecessary return statement, maybe you want to remove it?
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH 31/35] pathspec: allow querying for attributes
From: Brandon Williams @ 2016-11-28 18:02 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, pclouds, git
In-Reply-To: <20161110203428.30512-32-sbeller@google.com>
On 11/10, Stefan Beller wrote:
> @@ -500,6 +586,18 @@ void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
>
> void clear_pathspec(struct pathspec *pathspec)
> {
> + int i, j;
> + for (i = 0; i < pathspec->nr; i++) {
> + if (!pathspec->items[i].attr_match_nr)
> + continue;
> + for (j = 0; j < pathspec->items[j].attr_match_nr; j++)
This looks like a bug. Should it be:
for (j = 0; j < pathspec->items[i].attr_match_nr; j++)
where items[j] -> items[i]
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] l10n: de.po: translate 210 new message
From: Ralf Thielow @ 2016-11-28 17:55 UTC (permalink / raw)
To: Jiang Xin
Cc: Git List, Thomas Rast, Jan Krüger, Christian Stimming,
Phillip Sz, Matthias Rüster, Magnus Görlitz
In-Reply-To: <CANYiYbEzoN8S0o7_1N4hpO6OHZqq5Y4cMPxPPLEMA4TJ2n-d1g@mail.gmail.com>
2016-11-28 15:21 GMT+01:00 Jiang Xin <worldhello.net@gmail.com>:
> 2016-11-25 2:25 GMT+08:00 Ralf Thielow <ralf.thielow@gmail.com>:
>> #: sequencer.c:251
>> -#, fuzzy, c-format
>> +#, c-format
>> msgid "could not write eol to '%s"
>
> Unmatched single quote has been fixed in l10n round 3.
> You can rebase and update de.po file.
>
> BTW, Git 2.11.0 will be released tomorrow, please send PR in time.
I've rebased, fixed the subject line and send a pull request to you.
Thanks
>
>> -msgstr "Konnte nicht nach '%s' schreiben."
>> +msgstr "Konnte EOL nicht nach '%s' schreiben."
>
> --
> Jiang Xin
^ permalink raw reply
* Re: [feature request] Make "commit --only" work with new files
From: Junio C Hamano @ 2016-11-28 17:52 UTC (permalink / raw)
To: Luis Ressel; +Cc: git
In-Reply-To: <20161125175619.19e13e59@gentp.lnet>
Luis Ressel <aranea@aixah.de> writes:
> currently "git commit --only <file>" only works if <file> is already
> checked into the repo, but not with newly created and still untracked
> files (builtin/commit.c:list_path() throws the error "error: pathspec
> '<file>' did not match any file(s) known to git.")
The fact that pathspec on the command line of "commit" does not let
you add new files is true with or without "--only". Yes, "--only"
is the default so with or without it it means the same thing, but
even with "--include" that says "I am happy with what is in the
index, but please take further changes to these paths, too" does not
affect files that are not so far tracked.
> I don't think this limitation is intented.
This actually was intended. Back when "commit [--opts] <pathspec>"
was invented, out tools were designed to avoid adding unwanted files
by mistake (e.g. "update-index" without an explicit "--add" work
only on paths already known to Git), and the behaviour is in line
with that design. It partly was because back then we didn't even
have ".gitignore" mechanism, I would say. So it was not only
intended, but was a sensible design decision back then.
I suspect that an argument could be made that it is about time we
shift the design philosophy and allow adding new paths with pathspec
given to "git commit". If I were designing Git without any existing
users, with all the other goodies we already have, and "git commit"
in my version of Git lacked pathspec support now, I might allow it
to add untracked files with the pathspec [*1*].
There however are backward compatibility worries. People who are
used to the designed behaviour for the past 10 years still expect
and rely on that
$ git commit <path-to-dir>
to take _only_ changes to the files that are already tracked in the
<path-to-dir> since the last "git add" they did to them, and other
files in the same <path-to-dir> that are not yet ready (and they
deliberately left un-added) will not be in the commit.
[Footnote]
*1* I might decide not to, after thinking long enough, though. The
point is that times changed and the trade off between safetly of
not adding at the point of commit and convenience of adding has
shifted. I haven't thought enough to decide that the shift is
big enough to warrant the change in behaviour, but at least it
is now worth considering.
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Johannes Schindelin @ 2016-11-28 17:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <xmqqa8cjlekl.fsf@gitster.mtv.corp.google.com>
Hi,
On Mon, 28 Nov 2016, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Sat, Nov 26, 2016 at 02:01:36PM +0100, Johannes Schindelin wrote:
> >
> >> > If you want to control it from outside the test script, you'd need
> >> > something like:
> >> >
> >> > if test "$GIT_TEST_DIFFTOOL" = "builtin"
> >>
> >> That is a bit magic. I first used "GIT_USE_BUILTIN_DIFFTOOL" and it did
> >> not work. My name is arguably more correct (see also Jakub's note about
> >> "naming is hard"), but yours works because there is a "TEST" substring in
> >> it.
> >
> > Yes. You are free to add an exception to the env list in test-lib.sh,
> > but we usually use GIT_TEST_* to avoid having to do so.
>
> Perhaps
>
> - The switch between "do I use builtin, or scripted?" mechanism in
> 1/2 can look at an environment (just like the old "am" rewrite
> series did), instead of configuration. This would make the code
> a lot more simppler (you do not have to worry about the
> interaction between "setup" and .git/config).
>
> - That environment variable can be named GIT_TEST_BUILTIN_DIFFTOOL;
> after all, people are opting into helping to test the new shiny
> to make/prove it ready sooner.
>
> - The bulk of the existing test for difftool can be moved to a
> dot-included file (in a way similar to t/annotate-tests are
> usable to test both annotate and blame-imitating-annotate).
> Existing PERL prerequisites can all be lost.
>
> - Two tests can include that dot-included file; one would
> explicitly unset that environment (and gives up without PERL
> prerequisite), while the other explicitly sets it.
If my main worry was the test suite, I would agree with this plan.
However, I have been bitten time and again by problems that occurred only
in production, our test suite (despite taking already waaaaaay too long to
be truly useful in my daily development) was simply not good enough.
So my plan was different: to let end users opt-in to test this new beast
thoroughly, more thoroughly than any review would.
And for that, environment variables are just not an option. I need
something that can be configured in a portable application, so that the
main Git for Windows installation is unaffected.
My original "create a file in libexec/git-core/" was simple, did the job
reliably, and worked also for testing.
It is a pity that you two gentlemen shot it down for being inelegant. And
ever since, we try to find a solution that is as simple, works as
reliably, also for testing, *and* appeases your tastes.
Ciao,
Dscho
^ permalink raw reply
* Re: trustExitCode doesn't apply to vimdiff mergetool
From: Junio C Hamano @ 2016-11-28 17:17 UTC (permalink / raw)
To: David Aguilar; +Cc: Jeff King, Dun Peal, Git ML
In-Reply-To: <20161128021554.GA30863@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> deltawalker, diffmerge, emerge, kdiff3, kompare, and tkdiff originally
> provided behavior that matched trustExitCode=true.
>
> The default for all tools is trustExitCode=false, which conflicts with
> these tools' defaults. Allow tools to advertise their own default value
> for trustExitCode so that users do not need to opt-in to the original
> behavior.
>
> While this makes the default inconsistent between tools, it can still be
> overridden, and it makes it consistent with the current Git behavior.
I think this is sensible, because the way I look at this issue is
that in an ideal world, we would want all tool backends consistently
give us usable exit codes, but some tools are known to give unusable
exit codes, so we ignore their exit codes by default.
As to the implementation, I think you can reduce the duplication by
having each tool backend
- export a new function that echos "true" or "false"; or
- export a new function that returns true or false; or
- set a variable whose value is either "true" or "false"
and use that from the trust_exit_code() in git-mergetool--lib.sh.
Something like this (for the second alternative).
trust_exit_code () {
if git config --bool "mergtool.$.trustExitCode"
then
:; # OK
elif mergetool_exitcode_trustable
then
echo true
else
echo false
fi
}
^ permalink raw reply
* Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin
From: Junio C Hamano @ 2016-11-28 17:06 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git, David Aguilar, Dennis Kaarsemaker
In-Reply-To: <20161127165058.uxujjehyjq7httro@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, Nov 26, 2016 at 02:01:36PM +0100, Johannes Schindelin wrote:
>
>> > If you want to control it from outside the test script, you'd need
>> > something like:
>> >
>> > if test "$GIT_TEST_DIFFTOOL" = "builtin"
>>
>> That is a bit magic. I first used "GIT_USE_BUILTIN_DIFFTOOL" and it did
>> not work. My name is arguably more correct (see also Jakub's note about
>> "naming is hard"), but yours works because there is a "TEST" substring in
>> it.
>
> Yes. You are free to add an exception to the env list in test-lib.sh,
> but we usually use GIT_TEST_* to avoid having to do so.
Perhaps
- The switch between "do I use builtin, or scripted?" mechanism in
1/2 can look at an environment (just like the old "am" rewrite
series did), instead of configuration. This would make the code
a lot more simppler (you do not have to worry about the
interaction between "setup" and .git/config).
- That environment variable can be named GIT_TEST_BUILTIN_DIFFTOOL;
after all, people are opting into helping to test the new shiny
to make/prove it ready sooner.
- The bulk of the existing test for difftool can be moved to a
dot-included file (in a way similar to t/annotate-tests are
usable to test both annotate and blame-imitating-annotate).
Existing PERL prerequisites can all be lost.
- Two tests can include that dot-included file; one would
explicitly unset that environment (and gives up without PERL
prerequisite), while the other explicitly sets it.
^ permalink raw reply
* Re: [PATCH v1 15/19] config: add git_config_get_date_string() from gc.c
From: Junio C Hamano @ 2016-11-28 16:56 UTC (permalink / raw)
To: Christian Couder
Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <CAP8UFD0Q4yTfnhLW6XbfqbxBvMc_QmZTEn4XJb-9fj6Uvq6hkw@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> What about something like this then:
>
> /* This dies if the configured or default date is in the future */
> extern int git_config_get_expiry_or_die(const char *key, const char **output);
>
> Also git_config_get_int(), git_config_get_bool() and probably other
> such functions are indirectly calling git_config_int() which die()s is
> the value is not a number, so it feels a bit strange to have only one
> function with a name that ends with "_or_die" when many other
> functions could die(). In fact it could give a false sense of security
> to those who just read cache.h.
It is half-a-good-point that existing functions would die and I
agree with you that get_expiry_or_die can lose _or_die part.
But get_int() dying when it is fed something that is not "int" (or
something that cannot be returned in "int") is one thing; get_date()
being happy when it is fed a string that is a valid date 2015-01-01
but dying when fed another valid date 2018-01-01 is quite puzzling.
I think get_expiry() is OK.
Q: What's expiry? How is it different from date
A: A past timestamp that is speifies the cutoff time for the purpose
of expiring old stuff. You can say "now" to expire everything
you have.
is more pleasant and understandable conversation than
Q: Why can I feed only past date to get_date()? Otherwise it dies.
A: ...? The people who named the function didn't know what "date" was?
^ permalink raw reply
* Re: [PATCH v1 15/19] config: add git_config_get_date_string() from gc.c
From: Christian Couder @ 2016-11-28 16:19 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Nguyen Thai Ngoc Duy, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <xmqqlgwam76c.fsf@gitster.mtv.corp.google.com>
On Wed, Nov 23, 2016 at 6:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> Ok it will appear like this in cache.h:
>>
>> /* This dies if the configured or default date is in the future */
>> extern int git_config_get_expire_date_string(const char *key, const
>> char **output);
>
> Those who imitate existing callsites never read comments, and you
> need to spend effort to get the name right to protect the codebase
> from them.
>
> "get-expiry" may be shorter. Neither still does not say it will
> die, though.
What about something like this then:
/* This dies if the configured or default date is in the future */
extern int git_config_get_expiry_or_die(const char *key, const char **output);
Also git_config_get_int(), git_config_get_bool() and probably other
such functions are indirectly calling git_config_int() which die()s is
the value is not a number, so it feels a bit strange to have only one
function with a name that ends with "_or_die" when many other
functions could die(). In fact it could give a false sense of security
to those who just read cache.h.
^ permalink raw reply
* Re: cherry-pick -Xrenormalize fails with formerly CRLF files
From: Torsten Bögershausen @ 2016-11-28 15:54 UTC (permalink / raw)
To: eevee.reply; +Cc: git
In-Reply-To: <777ee899-4cfb-e3b4-da0d-793fde35e412@veekun.com>
On Sun, Nov 27, 2016 at 10:19:35PM -0800, Eevee (Lexy Munroe) wrote:
> I'm working with a repo that used to be all CRLF. At some point it
> was changed to all LF, with `text=auto` in .gitattributes for the
> sake of Windows devs. I'm on Linux and have never touched any
> twiddles relating to line endings. I'm trying to cherry-pick some
> commits from before the switchover.
>
> Straightforward cherry-picking causes entire files at a time to
> conflict, which I've seen before when switching from tabs to spaces.
> So I tried -Xrenormalize and got:
>
> fatal: CRLF would be replaced by LF in [path]
>
Which version of Git are you using, what does
git --version
say?
> The error comes from check_safe_crlf, which warns if checksafe is
> CRLF_SAFE_WARN and dies if it's (presumably) CRLF_SAFE_FAIL. The
> funny thing is that it's CRLF_SAFE_RENORMALIZE.
>
> I don't know what the semantics of this value are, but the caller
> (crlf_to_git) explicitly checks for CRLF_SAFE_RENORMALIZE and
> changes it to CRLF_SAFE_FALSE instead. But that check only happens
> if crlf_action is CRLF_AUTO*, and for me it's CRLF_TEXT_INPUT.
>
> I moved the check to happen regardless of the value of crlf_action,
> and at least in this case, git appears to happily do the right
> thing. So I think this is a bug, but line endings are such a tangle
> that I'm really not sure. :)
>
I am not sure either.
Could you send me the diff you made ?
git diff
I am happy to look into it, (in the amount of time I have).
> The repository in question is ZDoom: https://github.com/rheit/zdoom
> I'm trying to cherry-pick commits from the 3dfloors3 branch (e.g.,
> 9fb2daf58e9d512170859302a1ac0ea9c2ec5993) onto a slightly outdated
> master, 6384e81d0f135a2c292ac3e874f6fe26093f45b1.
This is what I tried:
user@pc:~/NoBackup> cd zdoom/ 9fb2daf58e9d512170859302a1ac0ea9c2ec5993 t9fb2daf5Note: checking out '9fb2daf58e9d512170859302a1ac0ea9c2ec5993'.02a1ac0ea9c2ec5993
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:
git checkout -b <new-branch-name>
HEAD is now at 9fb2daf... - Force use of GL nodes when 3D floors are present. - Move MAPINFO line modification flags into P_AdjustLine().
user@pc:~/NoBackup/zdoom> git format-patch HEAD^..HEAD --stdout >9fb2daf.patch
user@pc:~/NoBackup/zdoom> git format-patch HEAD^..HEAD --stdout | tr -d "\r" >9fb2daf-noCRLF.patch
user@pc:~/NoBackup/zdoom> git checkout 6384e81d0f135a2c292ac3e874f6fe26093f45b1
Previous HEAD position was 9fb2daf... - Force use of GL nodes when 3D floors are present. - Move MAPINFO line modification flags into P_AdjustLine().
HEAD is now at 6384e81... - Add support for Skulltag ACS IsNetworkGame.
user@pc:~/NoBackup/zdoom> git cherry-pick 9fb2daf58e9d512170859302a1ac0ea9c2ec5993'.02a1ac0ea9c2ec5993
>
user@pc:~/NoBackup/zdoom> git cherry-pick 9fb2daf58e9d512170859302a1ac0ea9c2ec5993
error: could not apply 9fb2daf... - Force use of GL nodes when 3D floors are present.
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'
user@pc:~/NoBackup/zdoom> git status
HEAD detached at 6384e81
You are currently cherry-picking commit 9fb2daf.
(fix conflicts and run "git cherry-pick --continue")
(use "git cherry-pick --abort" to cancel the cherry-pick operation)
Unmerged paths:
(use "git add <file>..." to mark resolution)
both modified: src/p_setup.cpp
Untracked files:
(use "git add <file>..." to include in what will be committed)
9fb2daf-noCRLF.patch
9fb2daf.patch
no changes added to commit (use "git add" and/or "git commit -a")
user@pc:~/NoBackup/zdoom> git cherry-pick --abort
user@pc:~/NoBackup/zdoom> git am < 9fb2daf-noCRLF.patch
Applying: - Force use of GL nodes when 3D floors are present. - Move MAPINFO line modification flags into P_AdjustLine().
error: patch failed: src/p_setup.cpp:1798
error: src/p_setup.cpp: patch does not apply
Patch failed at 0001 - Force use of GL nodes when 3D floors are present. - Move MAPINFO line modification flags into P_AdjustLine().
The copy of the patch that failed is found in: .git/rebase-apply/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".
The patch did not apply, but for different reasons.
Could you send us, what exactly you did, to help me out ?
^ permalink raw reply
* Re: [PATCH] l10n: de.po: translate 210 new message
From: Jiang Xin @ 2016-11-28 14:21 UTC (permalink / raw)
To: Ralf Thielow
Cc: Git List, Thomas Rast, Jan Krüger, Christian Stimming,
Phillip Sz, matthias.ruester, magnus.goerlitz
In-Reply-To: <20161124182500.6875-1-ralf.thielow@gmail.com>
2016-11-25 2:25 GMT+08:00 Ralf Thielow <ralf.thielow@gmail.com>:
> #: sequencer.c:251
> -#, fuzzy, c-format
> +#, c-format
> msgid "could not write eol to '%s"
Unmatched single quote has been fixed in l10n round 3.
You can rebase and update de.po file.
BTW, Git 2.11.0 will be released tomorrow, please send PR in time.
> -msgstr "Konnte nicht nach '%s' schreiben."
> +msgstr "Konnte EOL nicht nach '%s' schreiben."
--
Jiang Xin
^ 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