* [PATCH v4 17/22] read-cache: unlink old sharedindex files
From: Christian Couder @ 2017-02-27 18:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170227180019.18666-1-chriscool@tuxfamily.org>
Everytime split index is turned on, it creates a "sharedindex.XXXX"
file in the git directory. This change makes sure that shared index
files that haven't been used for a long time are removed when a new
shared index file is created.
The new "splitIndex.sharedIndexExpire" config variable is created
to tell the delay after which an unused shared index file can be
deleted. It defaults to "2.weeks.ago".
A previous commit made sure that each time a split index file is
created the mtime of the shared index file it references is updated.
This makes sure that recently used shared index file will not be
deleted.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/read-cache.c b/read-cache.c
index 5f295af4c6..45fc831010 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2199,6 +2199,65 @@ static int write_split_index(struct index_state *istate,
return ret;
}
+static const char *shared_index_expire = "2.weeks.ago";
+
+static unsigned long get_shared_index_expire_date(void)
+{
+ static unsigned long shared_index_expire_date;
+ static int shared_index_expire_date_prepared;
+
+ if (!shared_index_expire_date_prepared) {
+ git_config_get_expiry("splitindex.sharedindexexpire",
+ &shared_index_expire);
+ shared_index_expire_date = approxidate(shared_index_expire);
+ shared_index_expire_date_prepared = 1;
+ }
+
+ return shared_index_expire_date;
+}
+
+static int can_delete_shared_index(const char *shared_index_path)
+{
+ struct stat st;
+ unsigned long expiration;
+
+ /* Check timestamp */
+ expiration = get_shared_index_expire_date();
+ if (!expiration)
+ return 0;
+ if (stat(shared_index_path, &st))
+ return error_errno(_("could not stat '%s"), shared_index_path);
+ if (st.st_mtime > expiration)
+ return 0;
+
+ return 1;
+}
+
+static int clean_shared_index_files(const char *current_hex)
+{
+ struct dirent *de;
+ DIR *dir = opendir(get_git_dir());
+
+ if (!dir)
+ return error_errno(_("unable to open git dir: %s"), get_git_dir());
+
+ while ((de = readdir(dir)) != NULL) {
+ const char *sha1_hex;
+ const char *shared_index_path;
+ if (!skip_prefix(de->d_name, "sharedindex.", &sha1_hex))
+ continue;
+ if (!strcmp(sha1_hex, current_hex))
+ continue;
+ shared_index_path = git_path("%s", de->d_name);
+ if (can_delete_shared_index(shared_index_path) > 0 &&
+ unlink(shared_index_path))
+ error_errno(_("unable to unlink: %s"), shared_index_path);
+ }
+ closedir(dir);
+
+ return 0;
+}
+
static struct tempfile temporary_sharedindex;
static int write_shared_index(struct index_state *istate,
@@ -2220,8 +2279,11 @@ static int write_shared_index(struct index_state *istate,
}
ret = rename_tempfile(&temporary_sharedindex,
git_path("sharedindex.%s", sha1_to_hex(si->base->sha1)));
- if (!ret)
+ if (!ret) {
hashcpy(si->base_sha1, si->base->sha1);
+ clean_shared_index_files(sha1_to_hex(si->base->sha1));
+ }
+
return ret;
}
--
2.12.0.22.g0672473d40
^ permalink raw reply related
* [PATCH v4 21/22] Documentation/config: add splitIndex.sharedIndexExpire
From: Christian Couder @ 2017-02-27 18:00 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Nguyen Thai Ngoc Duy,
Ævar Arnfjörð Bjarmason, Ramsay Jones, Jeff King,
Christian Couder
In-Reply-To: <20170227180019.18666-1-chriscool@tuxfamily.org>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
Documentation/config.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8e745bda52..0e9982c5e3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2844,6 +2844,18 @@ splitIndex.maxPercentChange::
than 20 percent of the total number of entries.
See linkgit:git-update-index[1].
+splitIndex.sharedIndexExpire::
+ When the split index feature is used, shared index files that
+ were not modified since the time this variable specifies will
+ be removed when a new shared index file is created. The value
+ "now" expires all entries immediately, and "never" suppresses
+ expiration altogether.
+ The default value is "2.weeks.ago".
+ Note that a shared index file is considered modified (for the
+ purpose of expiration) each time a new split-index file is
+ created based on it.
+ See linkgit:git-update-index[1].
+
status.relativePaths::
By default, linkgit:git-status[1] shows paths relative to the
current directory. Setting this variable to `false` shows paths
--
2.12.0.22.g0672473d40
^ permalink raw reply related
* Re: [PATCH] http: add an "auto" mode for http.emptyauth
From: Junio C Hamano @ 2017-02-27 18:35 UTC (permalink / raw)
To: Jeff King
Cc: Johannes Schindelin, David Turner, git@vger.kernel.org,
sandals@crustytoothpaste.net, Eric Sunshine
In-Reply-To: <20170225191831.dkjasyv3tmkwutre@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The auto mode may incur an extra round-trip over setting
> http.emptyauth=true, because part of the emptyauth hack is
> to feed this blank password to curl even before we've made a
> single request.
IOW, people who care about an extra round-trip have this workaround,
which is good.
This, along with the possible security implications, may want to be
added to the documentation but that is outside the topic of this
change, and I think we would want to see such an update come from
those who actually use NTLM (or Kerberos, but they know they have
minimum security implications).
> +#ifndef LIBCURL_CAN_HANDLE_AUTH_ANY
> + /*
> + * Our libcurl is too old to do AUTH_ANY in the first place;
> + * just default to turning the feature off.
> + */
> +#else
> + /*
> + * In the automatic case, kick in the empty-auth
> + * hack as long as we would potentially try some
> + * method more exotic than "Basic" or "Digest".
> + *
> + * But only do this when this is our second or
> + * subsequent * request, as by then we know what
I'll drop the '*' that you left while line-wrapping ;-)
> + * methods are available.
> + */
Thanks. This looks good.
^ permalink raw reply
* Re: git-clone --config order & fetching extra refs during initial clone
From: Junio C Hamano @ 2017-02-27 19:16 UTC (permalink / raw)
To: Jeff King; +Cc: Robin H. Johnson, SZEDER Gábor, Git Mailing List
In-Reply-To: <20170225205052.j3p7obbf4onf6cbf@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> [Re-sending, as I used an old address for Gábor on the original]
>
> On Sat, Feb 25, 2017 at 07:12:38PM +0000, Robin H. Johnson wrote:
>
>> TL;DR: git-clone ignores any fetch specs passed via --config.
>
> I agree that this is a bug. There's some previous discussion and an RFC
> patch from lat March (author cc'd):
>
> http://public-inbox.org/git/1457313062-10073-1-git-send-email-szeder@ira.uka.de/
>
> That discussion veered off into alternatives, but I think the v2 posted
> in that thread is taking a sane approach.
Let's see how well it fares by cooking it in 'next' ;-)
I think it was <1459349623-16443-1-git-send-email-szeder@ira.uka.de>,
which needs a bit of massaging to apply to the current codebase.
-- >8 --
From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Wed, 30 Mar 2016 16:53:43 +0200
Subject: [PATCH] clone: respect configured fetch respecs during initial fetch
Conceptually 'git clone' should behave as if the following commands
were run:
git init
git config ... # set default configuration and origin remote
git fetch
git checkout # unless '--bare' is given
However, that initial 'git fetch' behaves differently from any
subsequent fetches, because it takes only the default fetch refspec
into account and ignores all other fetch refspecs that might have
been explicitly specified on the command line (e.g. 'git -c
remote.origin.fetch=<refspec> clone' or 'git clone -c ...').
Check whether there are any fetch refspecs configured for the origin
remote and take all of them into account during the initial fetch as
well.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
builtin/clone.c | 36 ++++++++++++++++++++++++++++--------
t/t5611-clone-config.sh | 24 ++++++++++++++++++++++++
2 files changed, 52 insertions(+), 8 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 3f63edbbf9..97229268b6 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -516,7 +516,7 @@ static struct ref *find_remote_branch(const struct ref *refs, const char *branch
}
static struct ref *wanted_peer_refs(const struct ref *refs,
- struct refspec *refspec)
+ struct refspec *refspec, unsigned int refspec_count)
{
struct ref *head = copy_ref(find_ref_by_name(refs, "HEAD"));
struct ref *local_refs = head;
@@ -537,13 +537,18 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
warning(_("Could not find remote branch %s to clone."),
option_branch);
else {
- get_fetch_map(remote_head, refspec, &tail, 0);
+ unsigned int i;
+ for (i = 0; i < refspec_count; i++)
+ get_fetch_map(remote_head, &refspec[i], &tail, 0);
/* if --branch=tag, pull the requested tag explicitly */
get_fetch_map(remote_head, tag_refspec, &tail, 0);
}
- } else
- get_fetch_map(refs, refspec, &tail, 0);
+ } else {
+ unsigned int i;
+ for (i = 0; i < refspec_count; i++)
+ get_fetch_map(refs, &refspec[i], &tail, 0);
+ }
if (!option_mirror && !option_single_branch)
get_fetch_map(refs, tag_refspec, &tail, 0);
@@ -856,7 +861,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
int submodule_progress;
struct refspec *refspec;
- const char *fetch_pattern;
+ unsigned int refspec_count = 1;
+ const char **fetch_patterns;
+ const struct string_list *config_fetch_patterns;
packet_trace_identity("clone");
argc = parse_options(argc, argv, prefix, builtin_clone_options,
@@ -1002,9 +1009,21 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (option_required_reference.nr || option_optional_reference.nr)
setup_reference();
- fetch_pattern = value.buf;
- refspec = parse_fetch_refspec(1, &fetch_pattern);
+ strbuf_addf(&key, "remote.%s.fetch", option_origin);
+ config_fetch_patterns = git_config_get_value_multi(key.buf);
+ if (config_fetch_patterns)
+ refspec_count = 1 + config_fetch_patterns->nr;
+ fetch_patterns = xcalloc(refspec_count, sizeof(*fetch_patterns));
+ fetch_patterns[0] = value.buf;
+ if (config_fetch_patterns) {
+ struct string_list_item *fp;
+ unsigned int i = 1;
+ for_each_string_list_item(fp, config_fetch_patterns)
+ fetch_patterns[i++] = fp->string;
+ }
+ refspec = parse_fetch_refspec(refspec_count, fetch_patterns);
+ strbuf_reset(&key);
strbuf_reset(&value);
remote = remote_get(option_origin);
@@ -1058,7 +1077,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
refs = transport_get_remote_refs(transport);
if (refs) {
- mapped_refs = wanted_peer_refs(refs, refspec);
+ mapped_refs = wanted_peer_refs(refs, refspec, refspec_count);
/*
* transport_get_remote_refs() may return refs with null sha-1
* in mapped_refs (see struct transport->get_refs_list
@@ -1147,6 +1166,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
strbuf_release(&value);
junk_mode = JUNK_LEAVE_ALL;
+ free(fetch_patterns);
free(refspec);
return err;
}
diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh
index e4850b778c..3bed17783b 100755
--- a/t/t5611-clone-config.sh
+++ b/t/t5611-clone-config.sh
@@ -37,6 +37,30 @@ test_expect_success 'clone -c config is available during clone' '
test_cmp expect child/file
'
+test_expect_success 'clone -c remote.origin.fetch=<refspec> works' '
+ rm -rf child &&
+ git update-ref refs/grab/it refs/heads/master &&
+ git update-ref refs/keep/out refs/heads/master &&
+ git clone -c "remote.origin.fetch=+refs/grab/*:refs/grab/*" . child &&
+ (
+ cd child &&
+ git for-each-ref --format="%(refname)" refs/grab/ >../actual
+ ) &&
+ echo refs/grab/it >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git -c remote.origin.fetch=<refspec> clone works' '
+ rm -rf child &&
+ git -c "remote.origin.fetch=+refs/grab/*:refs/grab/*" clone . child &&
+ (
+ cd child &&
+ git for-each-ref --format="%(refname)" refs/grab/ >../actual
+ ) &&
+ echo refs/grab/it >expect &&
+ test_cmp expect actual
+'
+
# Tests for the hidden file attribute on windows
is_hidden () {
# Use the output of `attrib`, ignore the absolute path
^ permalink raw reply related
* Re: [PATCH] t6300: avoid creating refs/heads/HEAD
From: Junio C Hamano @ 2017-02-27 19:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170227092931.7iquwaxomeuuusi2@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> This comes originally from Junio's 84679d470. I cannot see how naming
> the new branch HEAD would make any difference to the test, but perhaps I
> am missing something.
Nah, I think it was just a random string that came to mind and the
topic being "ah we blindly dereference something when showing %(HEAD)"
it was plausible I thought of "H E A D" as that random string before
I used my usual other random strings like frotz ;-)
> I noticed this while digging on a nearby issue around "git branch -m @".
> This does happen to be the only test that checks that we can make a
> branch called refs/heads/HEAD, and I found it because it triggers if you
> try to disallow "git branch -m HEAD". :)
About that "nearby" one, does it even make sense to do the interpret
thing on the <new> name? I can understand "please rename the branch
I was previously on to this new name" wanting to say @{-1} when the
user does not recall the exact spelling of a long name, but I do not
quite see how "to this new name" part benefits by the "interpret
branch name" magic in the first place.
> If we care about that, though, I think we should make an explicit test
> for "git branch HEAD". But I'm not sure we _do_ care about that. Making
> a branch called HEAD is moderately insane, and I don't think it would be
> unreasonable for us to outlaw it at some point.
Yeah, at that point we would have "test_must_fail git branch HEAD".
> t/t6300-for-each-ref.sh | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index aea1dfc71..a468041c5 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -558,7 +558,7 @@ test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '
> test_when_finished "git checkout master" &&
> git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&
> sed -e "s/^\* / /" actual >expect &&
> - git checkout --orphan HEAD &&
> + git checkout --orphan orphaned-branch &&
> git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&
> test_cmp expect actual
> '
^ permalink raw reply
* Re: 'git submodules update' ignores credential.helper config of the parent repository
From: Stefan Beller @ 2017-02-27 19:09 UTC (permalink / raw)
To: Dmitry Neverov, Duy Nguyen, Jeff King, Junio C Hamano; +Cc: Git List
In-Reply-To: <CAC+L6n0YeX_n_AysCLtBWkA+jPHwg7HmOWq2PLj75byxOZE=qQ@mail.gmail.com>
On Mon, Feb 27, 2017 at 5:33 AM, Dmitry Neverov
<dmitry.neverov@gmail.com> wrote:>
> git -c credential.helper= submodule update
>
> Is it by design?
A similar question came up w.r.t. submodule configuration
recently. It is about url.<URLISH>.insteadOf[1] that is set
in the super project and is expected to work in the submodules.
More reading on some background there, as it is the very same
problem: Which configuration should propagate to the submodules,
how do we tell the users and can the user influence if some
articular settings are propagated?
For both these settings (url...insteadOf and the credentialHelper)
one might think that they absolutely should be propagated
to the submodules, but that may not be true; e.g. a submodule
might be hosted at a different hosting provider, needing a different
credentials setup. (The submodule might be an open source library
that you use, which may even require no credentials at all)
So I think we have to come up with a generic solution to respect
certain settings of the superproject instead of e.g. hard coding
credential.helper to be looked up in the superproject.
So the current proposal (in that mentioned thread) is
to borrow the idea from worktrees that have a similar problem:
split up the config into multiple files and each file applies to
a different worktree or in our case we would have
(A) a config file that applies to the superproject;
(B) a config file that applies to both superproject
and all submodules
(C) and each submodule has its own config file as well.
---
For worktrees these multiple config files sounded like
the obvious solution, but I wonder if there was also
some bike shedding about other solutions?
I could imagine that we would want to have attributes
for specific configuration, e.g.:
--8<--
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
[remote "origin"]
url = git://github.com/gitster/git
fetch = +refs/heads/*:refs/remotes/origin/*
[attribute "submodules"]
read = true
# this will be read and respected by submodules as well:
[url."internal-git-miror"]
insteadOf = github.com
[attribute "submodules"]
read = false
# This (and the beginning of this file) will not be respected
# by submodules
[credential]
helper =
-->8--
This would change the semantics of a config file as the attribute for
each setting depends on the location (was attribute.FOO.read =
{true, false} read before).
This would be read-compatible with older versions of Git, and it seems
as if it were write compatible as well. Just writing a new value with a specifc
attribute would be interesting to implement.
Thanks,
Stefan
[1] https://public-inbox.org/git/84fcb0bd-85dc-0142-dd58-47a04eaa7c2b@durchholz.org/
^ permalink raw reply
* Re: [PATCH] cvs tests: When root, skip tests that call "cvs commit"
From: Junio C Hamano @ 2017-02-27 19:35 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, Robin Rosenberg
In-Reply-To: <20170227112628.10410-1-avarab@gmail.com>
Thanks, makes sense.
^ permalink raw reply
* Re: [PATCH 1/2] commit: be more precise when searching for headers
From: Junio C Hamano @ 2017-02-27 19:18 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List
In-Reply-To: <23989e76-24ba-90a4-91a9-9f66bfccb7c9@web.de>
René Scharfe <l.s.r@web.de> writes:
> Search for a space character only within the current line in
> read_commit_extra_header_lines() instead of searching in the whole
> buffer (and possibly beyond, if it's not NUL-terminated) and then
> discarding any results after the end of the current line.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> commit.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Makes sense.
> diff --git a/commit.c b/commit.c
> index 2cf85158b4..173c6d3818 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -1354,8 +1354,8 @@ static struct commit_extra_header *read_commit_extra_header_lines(
> strbuf_reset(&buf);
> it = NULL;
>
> - eof = strchr(line, ' ');
> - if (next <= eof)
> + eof = memchr(line, ' ', next - line);
> + if (!eof)
> eof = next;
>
> if (standard_header_field(line, eof - line) ||
^ permalink raw reply
* Re: [PATCH] interpret_branch_name(): handle auto-namelen for @{-1}
From: Junio C Hamano @ 2017-02-27 19:27 UTC (permalink / raw)
To: Jeff King; +Cc: git, Karthik Nayak
In-Reply-To: <20170227093122.3jdb2b62hlbbio5r@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Feb 27, 2017 at 04:25:40AM -0500, Jeff King wrote:
>
>> However, before we do that auto-namelen magic, we call
>> interpret_nth_prior_checkout(), which gets fed the bogus
>> "0". This was broken by 8cd4249c4 (interpret_branch_name:
>> always respect "namelen" parameter, 2014-01-15). Though to
>> be fair to that commit, it was broken in the _opposite_
>> direction before, where we would always treat "name" as a
>> string even if a length was passed.
>
> That commit is mine, by the way. More embarrassing than introducing the
> bug is that I _noticed_ the problem at the time and wrote a paragraph in
> the commit message rationalizing why it was OK, rather than just doing
> this trivial fix.
Thanks, I should also be embarrased since I didn't even notice the
issue when we queued it ;-)
^ permalink raw reply
* Re: show all merge conflicts
From: Junio C Hamano @ 2017-02-27 19:45 UTC (permalink / raw)
To: Michael J Gruber; +Cc: G. Sylvie Davies, Jeff King, Michael Spiegel, git
In-Reply-To: <6ff25254-720e-5b85-ba6d-22b16e91b354@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> If you're curious, I kept rebasing Thomas' remerge-diff (on top of our
> next) so far. You can find it at
>
> https://github.com/mjg/git/tree/remerge-diff
;-).
Yes, this was a good one.
> if you're interested. I don't know what problems were found back then,
> or what it would take to get this in-tree now.
If I recall correctly, everybody was in favor of what it does (or at
least attempted to do), but was leaky and not ready for "log -p" to
be used on a long stretch of history or something?
^ permalink raw reply
* RE: Unconventional roles of git
From: Randall S. Becker @ 2017-02-27 19:16 UTC (permalink / raw)
To: 'ankostis', 'Git Mailing List'; +Cc: 'Jason Cooper'
In-Reply-To: <CA+dhYEWV4TWp_-sVoGCK-r14JSKsS3_Q7tfwjmowRr5V_F7BZA@mail.gmail.com>
> -----Original Message-----
> From: git-owner@vger.kernel.org [mailto:git-owner@vger.kernel.org] On
> Behalf Of ankostis
> Sent: February 26, 2017 6:52 AM
> To: Git Mailing List <git@vger.kernel.org>
> Cc: Jason Cooper <git@lakedaemon.net>
> Subject: Unconventional roles of git
>
> On 26 February 2017 at 02:13, Jason Cooper <git@lakedaemon.net> wrote:
> > As someone looking to deploy (and having previously deployed) git in
> > unconventional roles, I'd like to add ...
>
> We are developing a distributed storage for type approval files regarding all
> vehicles registered in Europe.[1] To ensure integrity even after 10 or 30
> years, the hash of a commit of these files (as contained in a tag) are to be
> printed on the the paper certificates.
>
> - Can you provide some hints for other similar unconventional roles of git?
> - Any other comment on the above usage of git are welcomed.
I am involved in managing manufacturing designs and parts configurations and approvals with git being intimately involved in the process of developing and deploying tested designs to computerized manufacturing environments. It's pretty cool actually to see things become real.
Cheers,
Randall
-- Brief whoami: NonStop&UNIX developer since approximately UNIX(421664400)/NonStop(211288444200000000)
-- In my real life, I talk too much.
^ permalink raw reply
* Re: [PATCH 2/2] apply: handle assertion failure gracefully
From: Junio C Hamano @ 2017-02-27 20:04 UTC (permalink / raw)
To: René Scharfe; +Cc: Vegard Nossum, git, Christian Couder, Michal Zalewski
In-Reply-To: <a5626d97-e644-65b5-2fd3-41ce870f85a6@web.de>
René Scharfe <l.s.r@web.de> writes:
>> diff --git a/apply.c b/apply.c
>> index cbf7cc7f2..9219d2737 100644
>> --- a/apply.c
>> +++ b/apply.c
>> @@ -3652,7 +3652,6 @@ static int check_preimage(struct apply_state *state,
>> if (!old_name)
>> return 0;
>>
>> - assert(patch->is_new <= 0);
>
> 5c47f4c6 (builtin-apply: accept patch to an empty file) added that
> line. Its intent was to handle diffs that contain an old name even for
> a file that's created. Citing from its commit message: "When we
> cannot be sure by parsing the patch that it is not a creation patch,
> we shouldn't complain when if there is no such a file." Why not stop
> complaining also in case we happen to know for sure that it's a
> creation patch? I.e., why not replace the assert() with:
>
> if (patch->is_new == 1)
> goto is_new;
>
>> previous = previous_patch(state, patch, &status);
When the caller does know is_new is true, old_name must be made/left
NULL. That is the invariant this assert is checking to catch an
error in the calling code.
Errors in the patches fed as its input are caught by "if we do not
know if the patch is to add a new path yet, then declare it is, but
if we do know the patch is _NOT_ adding a new path, barf if that
path is not there" and other checks in this function, and changing
the assert to "if already new, then make it a no-op" defeats the
whole point of having an assert (and just removing it is even worse).
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] apply: guard against renames of non-existant empty files
From: Junio C Hamano @ 2017-02-27 20:10 UTC (permalink / raw)
To: René Scharfe; +Cc: Vegard Nossum, git, Christian Couder, Michal Zalewski
In-Reply-To: <baf195cc-ef81-bbad-4e01-4149498efedb@web.de>
René Scharfe <l.s.r@web.de> writes:
> Would it make sense to mirror the previously existing condition and
> check for is_new instead? I.e.:
>
> if ((!patch->is_delete && !patch->new_name) ||
> (!patch->is_new && !patch->old_name)) {
>
Yes, probably.
> or
>
> if (!(patch->is_delete || patch->new_name) ||
> !(patch->is_new || patch->old_name)) {
This happens after calling parse_git_header() so we should know the
actual value of is_delete and is_new by now (instead of mistaking
-1 aka "unknown" as true), so this rewrite would also be OK.
^ permalink raw reply
* Re: Transition plan for git to move to a new hash function
From: Tony Finch @ 2017-02-27 19:26 UTC (permalink / raw)
To: Ian Jackson
Cc: Jeff King, Ævar Arnfjörð Bjarmason, Linus Torvalds,
brian m. carlson, Jason Cooper, ankostis, Junio C Hamano,
Git Mailing List, Stefan Beller, David Lang, Joey Hess
In-Reply-To: <22708.8913.864049.452252@chiark.greenend.org.uk>
Ian Jackson <ijackson@chiark.greenend.org.uk> wrote:
A few questions and one or two suggestions...
> TEXTUAL SYNTAX
> ==============
>
> We also reserve the following syntax for private experiments:
> E[A-Z]+[0-9a-z]+
> We declare that public releases of git will never accept such
> object names.
Instead of this I would suggest that experimental hash names should have
multi-character prefixes and an easy registration process - rationale:
https://tools.ietf.org/html/rfc6648
> A single object may refer to other objects the hash function which
> names the object itself, or by other hash functions, in any
> combination.
If I understand it correctly, this freedom is greatly restricted later on
in this document, depending on the object type in question. If so, it's
probably worth saying so at this point.
> Commits
> -------
>
> The hash function naming an origin commit is controlled by the hint
> left in .git for the ref named by HEAD (or for HEAD itself, if HEAD is
> detached) by git checkout --orphan or git init.
This confused me for a while - I think you mean "root commit"?
> TRANSITION PLAN
> ===============
>
> Y4: BLAKE by default for new projects.
>
> When creating a new working tree, it starts using BLAKE.
>
> Servers which have been updated will accept BLAKE.
Why not allow newhash pushes before making it the default for new
projects? Wouldn't it make sense to get the server side ready some time
before projects start actively using new hashes?
Or is the idea that newhash upgrade is driven from the server?
What's the upgrade process for send-email patch exchange?
Tony.
--
f.anthony.n.finch <dot@dotat.at> http://dotat.at/ - I xn--zr8h punycode
Fair Isle: Southwest 6 to gale 8, backing east 5 or 6, backing north 6 to gale
8 later. Rough or very rough. Rain or showers. Moderate or good.
^ permalink raw reply
* Re: [PATCH] t6300: avoid creating refs/heads/HEAD
From: Jeff King @ 2017-02-27 20:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqzih7whrw.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 27, 2017 at 11:33:23AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > This comes originally from Junio's 84679d470. I cannot see how naming
> > the new branch HEAD would make any difference to the test, but perhaps I
> > am missing something.
>
> Nah, I think it was just a random string that came to mind and the
> topic being "ah we blindly dereference something when showing %(HEAD)"
> it was plausible I thought of "H E A D" as that random string before
> I used my usual other random strings like frotz ;-)
OK, thanks for confirming.
> > I noticed this while digging on a nearby issue around "git branch -m @".
> > This does happen to be the only test that checks that we can make a
> > branch called refs/heads/HEAD, and I found it because it triggers if you
> > try to disallow "git branch -m HEAD". :)
>
> About that "nearby" one, does it even make sense to do the interpret
> thing on the <new> name? I can understand "please rename the branch
> I was previously on to this new name" wanting to say @{-1} when the
> user does not recall the exact spelling of a long name, but I do not
> quite see how "to this new name" part benefits by the "interpret
> branch name" magic in the first place.
Yeah, it's arguable whether the "new" side of a rename should do any
interpretation at all. At the same time, the bug is in the underlying
function that assumes you can slap "refs/heads/" in front of the results
of interpret_branch_name(). And that function gets used in a lot of
places, including the "old" side of a rename. So:
git branch @{-1} foo
should clearly work. Doing:
git branch @{upstream} foo
is more debatable. It _does_ work, but only if your upstream is actually
a local branch (otherwise it tries to rename refs/heads/origin/master or
some such nonsense. It happens to fail most of the time because you
probably don't have such a branch, but it's still wrong to even look at
that).
I suspect there are a lot of other places that are less clear cut. E.g.,
I think just:
git branch foo bar
will put "foo" through the same interpretation. So you could do:
git branch -f @{-1} bar
Is that insane? Maybe. But it does work now.
-Peff
^ permalink raw reply
* Re: show all merge conflicts
From: Jeff King @ 2017-02-27 20:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, G. Sylvie Davies, Michael Spiegel, git
In-Reply-To: <xmqqr32jwh7o.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 27, 2017 at 11:45:31AM -0800, Junio C Hamano wrote:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
> > If you're curious, I kept rebasing Thomas' remerge-diff (on top of our
> > next) so far. You can find it at
> >
> > https://github.com/mjg/git/tree/remerge-diff
>
> ;-).
> Yes, this was a good one.
FWIW, I have also been carrying it forward. It's not a tool I reach for
often, but a couple of times it has come in very handy (mostly helping
somebody to track down a mistake that somebody made in a merge, like
accidentally using "checkout --ours" on top of a conflict).
> > if you're interested. I don't know what problems were found back then,
> > or what it would take to get this in-tree now.
>
> If I recall correctly, everybody was in favor of what it does (or at
> least attempted to do), but was leaky and not ready for "log -p" to
> be used on a long stretch of history or something?
The last round was at:
http://public-inbox.org/git/cover.1409860234.git.tr@thomasrast.ch/
I think. I think the leakiness was dealt with by rebasing onto the
name_hash refactoring. But it looks like there are a lot of little
issues, and maybe one bigger one: it turns "log" from a read-only
operation into that writes into the object database.
-Peff
^ permalink raw reply
* Re: git-clone --config order & fetching extra refs during initial clone
From: Jeff King @ 2017-02-27 21:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Robin H. Johnson, SZEDER Gábor, Git Mailing List
In-Reply-To: <xmqqd1e3xx4c.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 27, 2017 at 11:16:35AM -0800, Junio C Hamano wrote:
> >> TL;DR: git-clone ignores any fetch specs passed via --config.
> >
> > I agree that this is a bug. There's some previous discussion and an RFC
> > patch from lat March (author cc'd):
> >
> > http://public-inbox.org/git/1457313062-10073-1-git-send-email-szeder@ira.uka.de/
> >
> > That discussion veered off into alternatives, but I think the v2 posted
> > in that thread is taking a sane approach.
>
> Let's see how well it fares by cooking it in 'next' ;-)
>
> I think it was <1459349623-16443-1-git-send-email-szeder@ira.uka.de>,
> which needs a bit of massaging to apply to the current codebase.
Yeah, that is the most recent one I found.
I didn't actually review it very carefully before, but I'll do so now
(spoiler: a few nits, but it looks fine).
> static struct ref *wanted_peer_refs(const struct ref *refs,
> - struct refspec *refspec)
> + struct refspec *refspec, unsigned int refspec_count)
Most of the changes here and elsewhere are just about passing along
multiple refspecs instead of a single, which makes sense.
> @@ -856,7 +861,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> int submodule_progress;
>
> struct refspec *refspec;
> - const char *fetch_pattern;
> + unsigned int refspec_count = 1;
> + const char **fetch_patterns;
> + const struct string_list *config_fetch_patterns;
This "1" seems funny to up here by itself, as it must be kept in sync
with the later logic that feeds exactly one non-configured refspec into
our list. The current code isn't wrong, but it would be nice to have it
all together. I.e., replacing:
> + if (config_fetch_patterns)
> + refspec_count = 1 + config_fetch_patterns->nr;
> + fetch_patterns = xcalloc(refspec_count, sizeof(*fetch_patterns));
> + fetch_patterns[0] = value.buf;
with:
refspec_count = 1;
if (config_fetch_patterns)
refspec_count += config_fetch_patterns->nr;
...
Though if I'm bikeshedding, I'd probably have written the whole thing
with an argv_array and avoided counting at all.
> + refspec = parse_fetch_refspec(refspec_count, fetch_patterns);
>
> + strbuf_reset(&key);
> strbuf_reset(&value);
>
> remote = remote_get(option_origin);
I do also notice that right _after_ this parsing, we use remote_get(),
which is supposed to give us this config anyway. Which makes me wonder
if we could just reorder this to put remote_get() first, and then read
the resulting refspecs from remote->fetch.
> diff --git a/t/t5611-clone-config.sh b/t/t5611-clone-config.sh
> index e4850b778c..3bed17783b 100755
> --- a/t/t5611-clone-config.sh
> +++ b/t/t5611-clone-config.sh
> @@ -37,6 +37,30 @@ test_expect_success 'clone -c config is available during clone' '
> test_cmp expect child/file
> '
>
> +test_expect_success 'clone -c remote.origin.fetch=<refspec> works' '
> + rm -rf child &&
> + git update-ref refs/grab/it refs/heads/master &&
> + git update-ref refs/keep/out refs/heads/master &&
> + git clone -c "remote.origin.fetch=+refs/grab/*:refs/grab/*" . child &&
> + (
> + cd child &&
> + git for-each-ref --format="%(refname)" refs/grab/ >../actual
> + ) &&
> + echo refs/grab/it >expect &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'git -c remote.origin.fetch=<refspec> clone works' '
> + rm -rf child &&
> + git -c "remote.origin.fetch=+refs/grab/*:refs/grab/*" clone . child &&
> + (
> + cd child &&
> + git for-each-ref --format="%(refname)" refs/grab/ >../actual
> + ) &&
> + echo refs/grab/it >expect &&
> + test_cmp expect actual
> +'
These look reasonable. Using "git -C for-each-ref" would save a
subshell, but that's minor.
If we wanted to be thorough, we could also check that the feature works
correctly with "--origin" (I think it does).
-Peff
^ permalink raw reply
* Re: [PATCH 4/4] ident: do not ignore empty config name/email
From: Junio C Hamano @ 2017-02-27 20:42 UTC (permalink / raw)
To: Dennis Kaarsemaker, Christian Couder; +Cc: Jeff King, bs.x.ttp, git
In-Reply-To: <1488208102.10235.3.camel@kaarsemaker.net>
Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
> On Thu, 2017-02-23 at 23:18 -0500, Jeff King wrote:
>> On Thu, Feb 23, 2017 at 08:11:11PM -0800, Junio C Hamano wrote:
>>
>> > > So I dunno. I could really go either way on it. Feel free to drop it, or
>> > > even move it into a separate topic to be cooked longer.
>> >
>> > If it were 5 years ago, it would have been different, but I do not
>> > think cooking it longer in 'next' would smoke out breakages in
>> > obscure scripts any longer. Git is used by too many people who have
>> > never seen its source these days.
>>
>> Yeah, I have noticed that, too. I wonder if it would be interesting to
>> cut "weeklies" or something of "master" or even "next" that people could
>> install with a single click.
>>
>> Of course it's not like we have a binary installer in the first place,
>> so I guess that's a prerequisite.
>
> I provide daily[*] snapshots of git's master and next tree as packages
> for Ubuntu, Debian, Fedora and CentOS on launchpad and SuSE's
> openbuildservice. If there's sufficient interest in this (I know of
> only a few users), I can try to put more effort into this.
That sounds handy for people who do not build from the source
themselves.
Christian, perhaps rev-news can help advertising Dennis's effort to
recruit like-minded souls?
^ permalink raw reply
* Re: [PATCH] t6300: avoid creating refs/heads/HEAD
From: Junio C Hamano @ 2017-02-27 21:19 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170227205151.rjhod347ddhmdmxp@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I suspect there are a lot of other places that are less clear cut. E.g.,
> I think just:
>
> git branch foo bar
>
> will put "foo" through the same interpretation. So you could do:
>
> git branch -f @{-1} bar
>
> Is that insane? Maybe. But it does work now.
No, it _is_ very sensible, so is "git checkout -B @{-1} <someplace>"
Perhaps interpret-branch-name that does not error out when given "@"
is what is broken? I suspect that calling interpret_empty_at() from
that function is fundamentally flawed. The "@" end user types never
means refs/heads/HEAD, and HEAD@{either reflog or -1} would not mean
anything that should be taken as a branch_name, either.
So perhaps what interpret_empty_at() does is necessary for the "four
capital letters is too many to type, so just type one key while
holding a shift", but it should be called from somewhere else, and
not from interpret_branch_name()?
^ permalink raw reply
* [PATCH 5/6] ref-filter: avoid using `unsigned long` for catch-all data type
From: Johannes Schindelin @ 2017-02-27 21:31 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1488231002.git.johannes.schindelin@gmx.de>
In its `atom_value` struct, the ref-filter source code wants to store
different values in a field called `ul` (for `unsigned long`), e.g.
timestamps.
However, as we are about to switch the data type of timestamps away from
`unsigned long` (because it may be 32-bit even when `time_t` is 64-bit),
that data type is not large enough.
Simply use `uintmax_t` instead.
Unfortunately, this patch is larger than that because the field's name
was tied to its data type.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
ref-filter.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 07c1f372351..b8b34d4dd9e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -236,7 +236,7 @@ struct atom_value {
struct align align;
} u;
void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state);
- unsigned long ul; /* used for sorting when not FIELD_STR */
+ uintmax_t value; /* used for sorting when not FIELD_STR */
};
/*
@@ -492,7 +492,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct object
if (!strcmp(name, "objecttype"))
v->s = typename(obj->type);
else if (!strcmp(name, "objectsize")) {
- v->ul = sz;
+ v->value = sz;
v->s = xstrfmt("%lu", sz);
}
else if (deref)
@@ -539,8 +539,8 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
v->s = xstrdup(oid_to_hex(&commit->tree->object.oid));
}
else if (!strcmp(name, "numparent")) {
- v->ul = commit_list_count(commit->parents);
- v->s = xstrfmt("%lu", v->ul);
+ v->value = commit_list_count(commit->parents);
+ v->s = xstrfmt("%lu", (unsigned long)v->value);
}
else if (!strcmp(name, "parent")) {
struct commit_list *parents;
@@ -644,11 +644,11 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
if ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)
goto bad;
v->s = xstrdup(show_date(timestamp, tz, &date_mode));
- v->ul = timestamp;
+ v->value = timestamp;
return;
bad:
v->s = "";
- v->ul = 0;
+ v->value = 0;
}
/* See grab_values */
@@ -1583,9 +1583,9 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
else if (cmp_type == FIELD_STR)
cmp = cmp_fn(va->s, vb->s);
else {
- if (va->ul < vb->ul)
+ if (va->value < vb->value)
cmp = -1;
- else if (va->ul == vb->ul)
+ else if (va->value == vb->value)
cmp = cmp_fn(a->refname, b->refname);
else
cmp = 1;
--
2.11.1.windows.1.379.g44ae0bc
^ permalink raw reply related
* [PATCH 4/6] Prepare for timestamps to use 64-bit signed types
From: Johannes Schindelin @ 2017-02-27 21:31 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1488231002.git.johannes.schindelin@gmx.de>
Currently, Git's source code uses the unsigned long type to represent
timestamps. However, this type is limited to 32-bit e.g. on 64-bit
Windows. Hence it is a suboptimal type for this use case.
In any case, we need to use the time_t type to represent timestamps
since we often send those values to system functions which are declared
to accept time_t parameters.
So let's prepare for the case where timestamps are represented as 64-bit
signed integers by introducing the Makefile option TIME_T_IS_INT64.
As we have to resort to using `strtoull()` (and casting the parsed,
unsigned value to an `int64_t`), the check in the `date_overflows()`
helper has to be relaxed: a value of ULLONG_MAX (cast to `int64_t`)
now *also* indicates an overflow.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Makefile | 4 ++++
archive-tar.c | 5 ++++-
builtin/name-rev.c | 2 +-
builtin/prune.c | 2 +-
builtin/worktree.c | 2 +-
credential-cache--daemon.c | 2 +-
date.c | 8 ++++----
git-compat-util.h | 7 +++++++
ref-filter.c | 2 +-
9 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/Makefile b/Makefile
index 8e4081e0619..0232cf62d33 100644
--- a/Makefile
+++ b/Makefile
@@ -1518,6 +1518,10 @@ ifdef HAVE_GETDELIM
BASIC_CFLAGS += -DHAVE_GETDELIM
endif
+ifdef TIME_T_IS_INT64
+ BASIC_CFLAGS += -DTIME_T_IS_INT64
+endif
+
ifeq ($(TCLTK_PATH),)
NO_TCLTK = NoThanks
endif
diff --git a/archive-tar.c b/archive-tar.c
index 380e3aedd23..695339a2369 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -27,9 +27,12 @@ static int write_tar_filter_archive(const struct archiver *ar,
*/
#if ULONG_MAX == 0xFFFFFFFF
#define USTAR_MAX_SIZE ULONG_MAX
-#define USTAR_MAX_MTIME ULONG_MAX
#else
#define USTAR_MAX_SIZE 077777777777UL
+#endif
+#if TIME_MAX == 0xFFFFFFFF
+#define USTAR_MAX_MTIME TIME_MAX
+#else
#define USTAR_MAX_MTIME 077777777777UL
#endif
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index cd89d48b65e..a0f16407b93 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -145,7 +145,7 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
struct name_ref_data *data = cb_data;
int can_abbreviate_output = data->tags_only && data->name_only;
int deref = 0;
- unsigned long taggerdate = ULONG_MAX;
+ unsigned long taggerdate = TIME_MAX;
if (data->tags_only && !starts_with(path, "refs/tags/"))
return 0;
diff --git a/builtin/prune.c b/builtin/prune.c
index 8f4f0522856..1e5eb0292b1 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -111,7 +111,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)
};
char *s;
- expire = ULONG_MAX;
+ expire = TIME_MAX;
save_commit_buffer = 0;
check_replace_refs = 0;
ref_paranoia = 1;
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 831fe058a53..3df95e112e5 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -131,7 +131,7 @@ static int prune(int ac, const char **av, const char *prefix)
OPT_END()
};
- expire = ULONG_MAX;
+ expire = TIME_MAX;
ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
if (ac)
usage_with_options(worktree_usage, options);
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index 46c5937526a..b298ac01e4f 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -52,7 +52,7 @@ static int check_expirations(void)
static unsigned long wait_for_entry_until;
int i = 0;
unsigned long now = time(NULL);
- unsigned long next = (unsigned long)-1;
+ unsigned long next = TIME_MAX;
/*
* Initially give the client 30 seconds to actually contact us
diff --git a/date.c b/date.c
index 97ab5fcc349..23dee2964c1 100644
--- a/date.c
+++ b/date.c
@@ -659,7 +659,7 @@ static int match_object_header_date(const char *date, unsigned long *timestamp,
if (*date < '0' || '9' < *date)
return -1;
stamp = parse_timestamp(date, &end, 10);
- if (*end != ' ' || stamp == ULONG_MAX || (end[1] != '+' && end[1] != '-'))
+ if (*end != ' ' || stamp == TIME_MAX || (end[1] != '+' && end[1] != '-'))
return -1;
date = end + 2;
ofs = strtol(date, &end, 10);
@@ -762,7 +762,7 @@ int parse_expiry_date(const char *date, unsigned long *timestamp)
* of the past, and there is nothing from the future
* to be kept.
*/
- *timestamp = ULONG_MAX;
+ *timestamp = TIME_MAX;
else
*timestamp = approxidate_careful(date, &errors);
@@ -1184,8 +1184,8 @@ int date_overflows(unsigned long t)
{
time_t sys;
- /* If we overflowed our unsigned long, that's bad... */
- if (t == ULONG_MAX)
+ /* If we overflowed our timestamp data type, that's bad... */
+ if ((uintmax_t)t >= TIME_MAX)
return 1;
/*
diff --git a/git-compat-util.h b/git-compat-util.h
index 4365012c536..5cf1133532d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -319,8 +319,15 @@ extern char *gitdirname(char *);
#define PRIo32 "o"
#endif
+#ifdef TIME_T_IS_INT64
+#define PRItime PRId64
+#define parse_timestamp strtoull
+#define TIME_MAX INT64_MAX
+#else
#define PRItime "lu"
#define parse_timestamp strtoul
+#define TIME_MAX ULONG_MAX
+#endif
#ifndef PATH_SEP
#define PATH_SEP ':'
diff --git a/ref-filter.c b/ref-filter.c
index 6fab0db5e0d..07c1f372351 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -638,7 +638,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
if (!eoemail)
goto bad;
timestamp = parse_timestamp(eoemail + 2, &zone, 10);
- if (timestamp == ULONG_MAX)
+ if (timestamp == TIME_MAX)
goto bad;
tz = strtol(zone, NULL, 10);
if ((tz == LONG_MIN || tz == LONG_MAX) && errno == ERANGE)
--
2.11.1.windows.1.379.g44ae0bc
^ permalink raw reply related
* [PATCH 3/6] Introduce a new "printf format" for timestamps
From: Johannes Schindelin @ 2017-02-27 21:31 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <cover.1488231002.git.johannes.schindelin@gmx.de>
Currently, Git's source code treats all timestamps as if they were
unsigned longs. Therefore, it is okay to write "%lu" when printing them.
There is a substantial problem with that, though: at least on Windows,
time_t is *larger* than unsigned long, and hence we will want to switch
to using time_t instead.
So let's introduce the pseudo format "PRItime" (currently simply being
"lu") so that it is easy later on to change the data type to time_t.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/blame.c | 6 +++---
builtin/fsck.c | 2 +-
builtin/log.c | 2 +-
builtin/receive-pack.c | 4 ++--
date.c | 26 +++++++++++++-------------
fetch-pack.c | 2 +-
git-compat-util.h | 1 +
t/helper/test-date.c | 2 +-
t/helper/test-parse-options.c | 2 +-
upload-pack.c | 2 +-
vcs-svn/fast_export.c | 4 ++--
11 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index cffc6265408..c9486dd580b 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1736,11 +1736,11 @@ static int emit_one_suspect_detail(struct origin *suspect, int repeat)
get_commit_info(suspect->commit, &ci, 1);
printf("author %s\n", ci.author.buf);
printf("author-mail %s\n", ci.author_mail.buf);
- printf("author-time %lu\n", ci.author_time);
+ printf("author-time %"PRItime"\n", ci.author_time);
printf("author-tz %s\n", ci.author_tz.buf);
printf("committer %s\n", ci.committer.buf);
printf("committer-mail %s\n", ci.committer_mail.buf);
- printf("committer-time %lu\n", ci.committer_time);
+ printf("committer-time %"PRItime"\n", ci.committer_time);
printf("committer-tz %s\n", ci.committer_tz.buf);
printf("summary %s\n", ci.summary.buf);
if (suspect->commit->object.flags & UNINTERESTING)
@@ -1853,7 +1853,7 @@ static const char *format_time(unsigned long time, const char *tz_str,
strbuf_reset(&time_buf);
if (show_raw_time) {
- strbuf_addf(&time_buf, "%lu %s", time, tz_str);
+ strbuf_addf(&time_buf, "%"PRItime" %s", time, tz_str);
}
else {
const char *time_str;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1a5caccd0f5..5413c76e7a6 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -407,7 +407,7 @@ static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
if (timestamp && name_objects)
add_decoration(fsck_walk_options.object_names,
obj,
- xstrfmt("%s@{%ld}", refname, timestamp));
+ xstrfmt("%s@{%"PRItime"}", refname, timestamp));
obj->used = 1;
mark_object_reachable(obj);
} else {
diff --git a/builtin/log.c b/builtin/log.c
index 55d20cc2d88..24612c2299a 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -903,7 +903,7 @@ static void get_patch_ids(struct rev_info *rev, struct patch_ids *ids)
static void gen_message_id(struct rev_info *info, char *base)
{
struct strbuf buf = STRBUF_INIT;
- strbuf_addf(&buf, "%s.%lu.git.%s", base,
+ strbuf_addf(&buf, "%s.%"PRItime".git.%s", base,
(unsigned long) time(NULL),
git_committer_info(IDENT_NO_NAME|IDENT_NO_DATE|IDENT_STRICT));
info->message_id = strbuf_detach(&buf, NULL);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1dbb8a06922..4a878645847 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -456,12 +456,12 @@ static char *prepare_push_cert_nonce(const char *path, unsigned long stamp)
struct strbuf buf = STRBUF_INIT;
unsigned char sha1[20];
- strbuf_addf(&buf, "%s:%lu", path, stamp);
+ strbuf_addf(&buf, "%s:%"PRItime, path, stamp);
hmac_sha1(sha1, buf.buf, buf.len, cert_nonce_seed, strlen(cert_nonce_seed));;
strbuf_release(&buf);
/* RFC 2104 5. HMAC-SHA1-80 */
- strbuf_addf(&buf, "%lu-%.*s", stamp, 20, sha1_to_hex(sha1));
+ strbuf_addf(&buf, "%"PRItime"-%.*s", stamp, 20, sha1_to_hex(sha1));
return strbuf_detach(&buf, NULL);
}
diff --git a/date.c b/date.c
index a8848f6e141..97ab5fcc349 100644
--- a/date.c
+++ b/date.c
@@ -100,41 +100,41 @@ void show_date_relative(unsigned long time, int tz,
diff = now->tv_sec - time;
if (diff < 90) {
strbuf_addf(timebuf,
- Q_("%lu second ago", "%lu seconds ago", diff), diff);
+ Q_("%"PRItime" second ago", "%"PRItime" seconds ago", diff), diff);
return;
}
/* Turn it into minutes */
diff = (diff + 30) / 60;
if (diff < 90) {
strbuf_addf(timebuf,
- Q_("%lu minute ago", "%lu minutes ago", diff), diff);
+ Q_("%"PRItime" minute ago", "%"PRItime" minutes ago", diff), diff);
return;
}
/* Turn it into hours */
diff = (diff + 30) / 60;
if (diff < 36) {
strbuf_addf(timebuf,
- Q_("%lu hour ago", "%lu hours ago", diff), diff);
+ Q_("%"PRItime" hour ago", "%"PRItime" hours ago", diff), diff);
return;
}
/* We deal with number of days from here on */
diff = (diff + 12) / 24;
if (diff < 14) {
strbuf_addf(timebuf,
- Q_("%lu day ago", "%lu days ago", diff), diff);
+ Q_("%"PRItime" day ago", "%"PRItime" days ago", diff), diff);
return;
}
/* Say weeks for the past 10 weeks or so */
if (diff < 70) {
strbuf_addf(timebuf,
- Q_("%lu week ago", "%lu weeks ago", (diff + 3) / 7),
+ Q_("%"PRItime" week ago", "%"PRItime" weeks ago", (diff + 3) / 7),
(diff + 3) / 7);
return;
}
/* Say months for the past 12 months or so */
if (diff < 365) {
strbuf_addf(timebuf,
- Q_("%lu month ago", "%lu months ago", (diff + 15) / 30),
+ Q_("%"PRItime" month ago", "%"PRItime" months ago", (diff + 15) / 30),
(diff + 15) / 30);
return;
}
@@ -145,20 +145,20 @@ void show_date_relative(unsigned long time, int tz,
unsigned long months = totalmonths % 12;
if (months) {
struct strbuf sb = STRBUF_INIT;
- strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
+ strbuf_addf(&sb, Q_("%"PRItime" year", "%"PRItime" years", years), years);
strbuf_addf(timebuf,
/* TRANSLATORS: "%s" is "<n> years" */
- Q_("%s, %lu month ago", "%s, %lu months ago", months),
+ Q_("%s, %"PRItime" month ago", "%s, %"PRItime" months ago", months),
sb.buf, months);
strbuf_release(&sb);
} else
strbuf_addf(timebuf,
- Q_("%lu year ago", "%lu years ago", years), years);
+ Q_("%"PRItime" year ago", "%"PRItime" years ago", years), years);
return;
}
/* Otherwise, just years. Centuries is probably overkill. */
strbuf_addf(timebuf,
- Q_("%lu year ago", "%lu years ago", (diff + 183) / 365),
+ Q_("%"PRItime" year ago", "%"PRItime" years ago", (diff + 183) / 365),
(diff + 183) / 365);
}
@@ -179,7 +179,7 @@ const char *show_date(unsigned long time, int tz, const struct date_mode *mode)
if (mode->type == DATE_UNIX) {
strbuf_reset(&timebuf);
- strbuf_addf(&timebuf, "%lu", time);
+ strbuf_addf(&timebuf, "%"PRItime, time);
return timebuf.buf;
}
@@ -188,7 +188,7 @@ const char *show_date(unsigned long time, int tz, const struct date_mode *mode)
if (mode->type == DATE_RAW) {
strbuf_reset(&timebuf);
- strbuf_addf(&timebuf, "%lu %+05d", time, tz);
+ strbuf_addf(&timebuf, "%"PRItime" %+05d", time, tz);
return timebuf.buf;
}
@@ -643,7 +643,7 @@ static void date_string(unsigned long date, int offset, struct strbuf *buf)
offset = -offset;
sign = '-';
}
- strbuf_addf(buf, "%lu %c%02d%02d", date, sign, offset / 60, offset % 60);
+ strbuf_addf(buf, "%"PRItime" %c%02d%02d", date, sign, offset / 60, offset % 60);
}
/*
diff --git a/fetch-pack.c b/fetch-pack.c
index 601f0779a19..54fb35e39c5 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -357,7 +357,7 @@ static int find_common(struct fetch_pack_args *args,
packet_buf_write(&req_buf, "deepen %d", args->depth);
if (args->deepen_since) {
unsigned long max_age = approxidate(args->deepen_since);
- packet_buf_write(&req_buf, "deepen-since %lu", max_age);
+ packet_buf_write(&req_buf, "deepen-since %"PRItime, max_age);
}
if (args->deepen_not) {
int i;
diff --git a/git-compat-util.h b/git-compat-util.h
index 5eff97bea2e..4365012c536 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -319,6 +319,7 @@ extern char *gitdirname(char *);
#define PRIo32 "o"
#endif
+#define PRItime "lu"
#define parse_timestamp strtoul
#ifndef PATH_SEP
diff --git a/t/helper/test-date.c b/t/helper/test-date.c
index 98637053760..ba309ec1760 100644
--- a/t/helper/test-date.c
+++ b/t/helper/test-date.c
@@ -52,7 +52,7 @@ static void parse_dates(const char **argv, struct timeval *now)
strbuf_reset(&result);
parse_date(*argv, &result);
- if (sscanf(result.buf, "%lu %d", &t, &tz) == 2)
+ if (sscanf(result.buf, "%"PRItime" %d", &t, &tz) == 2)
printf("%s -> %s\n",
*argv, show_date(t, tz, DATE_MODE(ISO8601)));
else
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index a01430c24bd..7d93627e454 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -161,7 +161,7 @@ int cmd_main(int argc, const char **argv)
show(&expect, &ret, "boolean: %d", boolean);
show(&expect, &ret, "integer: %d", integer);
show(&expect, &ret, "magnitude: %lu", magnitude);
- show(&expect, &ret, "timestamp: %lu", timestamp);
+ show(&expect, &ret, "timestamp: %"PRItime, timestamp);
show(&expect, &ret, "string: %s", string ? string : "(not set)");
show(&expect, &ret, "abbrev: %d", abbrev);
show(&expect, &ret, "verbose: %d", verbose);
diff --git a/upload-pack.c b/upload-pack.c
index 8c47dc1707a..c2be661f6d4 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -859,7 +859,7 @@ static void receive_needs(void)
argv_array_push(&av, "rev-list");
if (deepen_since)
- argv_array_pushf(&av, "--max-age=%lu", deepen_since);
+ argv_array_pushf(&av, "--max-age=%"PRItime, deepen_since);
if (deepen_not.nr) {
argv_array_push(&av, "--not");
for (i = 0; i < deepen_not.nr; i++) {
diff --git a/vcs-svn/fast_export.c b/vcs-svn/fast_export.c
index 97cba39cdf5..6c9f2866d8b 100644
--- a/vcs-svn/fast_export.c
+++ b/vcs-svn/fast_export.c
@@ -73,7 +73,7 @@ void fast_export_begin_note(uint32_t revision, const char *author,
static int firstnote = 1;
size_t loglen = strlen(log);
printf("commit %s\n", note_ref);
- printf("committer %s <%s@%s> %lu +0000\n", author, author, "local", timestamp);
+ printf("committer %s <%s@%s> %"PRItime" +0000\n", author, author, "local", timestamp);
printf("data %"PRIuMAX"\n", (uintmax_t)loglen);
fwrite(log, loglen, 1, stdout);
if (firstnote) {
@@ -107,7 +107,7 @@ void fast_export_begin_commit(uint32_t revision, const char *author,
}
printf("commit %s\n", local_ref);
printf("mark :%"PRIu32"\n", revision);
- printf("committer %s <%s@%s> %lu +0000\n",
+ printf("committer %s <%s@%s> %"PRItime" +0000\n",
*author ? author : "nobody",
*author ? author : "nobody",
*uuid ? uuid : "local", timestamp);
--
2.11.1.windows.1.379.g44ae0bc
^ permalink raw reply related
* Re: [PATCH v7 4/6] stash: teach 'push' (and 'create_stash') to honor pathspec
From: Junio C Hamano @ 2017-02-27 20:35 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Jeff King, Johannes Schindelin, sunny, Jakub Narębski,
Matthieu Moy
In-Reply-To: <20170225213306.2410-5-t.gummerer@gmail.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> + test -n "$untracked" || git ls-files --error-unmatch -- "$@" >/dev/null || exit 1
This silent "exit 1" made me scratch my head, but --error-unmatch
would have already given an error message, like
error: pathspec 'no such' did not match any file(s) known to git.
Did you forget to 'git add'?
so that would be OK.
^ permalink raw reply
* Re: [PATCH] t6300: avoid creating refs/heads/HEAD
From: Jeff King @ 2017-02-27 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqshmzuyam.fsf@gitster.mtv.corp.google.com>
On Mon, Feb 27, 2017 at 01:19:29PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I suspect there are a lot of other places that are less clear cut. E.g.,
> > I think just:
> >
> > git branch foo bar
> >
> > will put "foo" through the same interpretation. So you could do:
> >
> > git branch -f @{-1} bar
> >
> > Is that insane? Maybe. But it does work now.
>
> No, it _is_ very sensible, so is "git checkout -B @{-1} <someplace>"
>
> Perhaps interpret-branch-name that does not error out when given "@"
> is what is broken? I suspect that calling interpret_empty_at() from
> that function is fundamentally flawed. The "@" end user types never
> means refs/heads/HEAD, and HEAD@{either reflog or -1} would not mean
> anything that should be taken as a branch_name, either.
>
> So perhaps what interpret_empty_at() does is necessary for the "four
> capital letters is too many to type, so just type one key while
> holding a shift", but it should be called from somewhere else, and
> not from interpret_branch_name()?
I think _most_ of interpret_branch_name() is in the same boat. The
"@{upstream}" mark is not likely to give you a branch in refs/heads
either.
So in practice, I think strbuf_check_branch_ref() could probably get by
with just calling interpret_nth_prior_checkout(). Or if you prefer, to
rip everything out of interpret_branch_name() except that. :) But that
other stuff has to go somewhere, and there are some challenges with the
recursion from reinterpret().
The "other" stuff could sometimes be useful, I guess. It's not _always_
wrong to do:
git branch -f @{upstream} foo
It depends on what your @{upstream} resolves to. Switching to just using
interpret_nth_prior_checkout() would break the case when it resolves to
a local branch. I'm not sure if we're OK with that or not. If we want to
keep all the existing cases working, I think we need something like the
"not_in_refs_heads" patch I posted elsewhere.
-Peff
^ permalink raw reply
* [PATCH 0/6] Use time_t
From: Johannes Schindelin @ 2017-02-27 21:30 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Git v2.9.2 was released in a hurry to accomodate for platforms like
Windows, where the `unsigned long` data type is 32-bit even for 64-bit
setups.
The quick fix was to simply disable all the testing with "absurd" future
dates.
However, we can do much better than that, as `time_t` exists, and at
least on 64-bit Windows it is 64-bit. Meaning: we *can* support these
absurd future dates on those platforms.
So let's do this.
One notable fallout of this patch series is that on 64-bit Linux (and
other platforms where `unsigned long` is 64-bit), we now limit the range
of dates to LONG_MAX (i.e. the *signed* maximum value). This needs to be
done as `time_t` can be signed (and indeed is at least on my Ubuntu
setup).
Obviously, I think that we can live with that, and I hope that all
interested parties agree.
Johannes Schindelin (6):
t0006 & t5000: prepare for 64-bit time_t
Specify explicitly where we parse timestamps
Introduce a new "printf format" for timestamps
Prepare for timestamps to use 64-bit signed types
ref-filter: avoid using `unsigned long` for catch-all data type
Use time_t where appropriate
Documentation/technical/api-parse-options.txt | 8 +--
Makefile | 4 ++
archive-tar.c | 5 +-
builtin/am.c | 4 +-
builtin/blame.c | 14 ++---
builtin/fsck.c | 6 +-
builtin/gc.c | 2 +-
builtin/log.c | 4 +-
builtin/merge-base.c | 2 +-
builtin/name-rev.c | 6 +-
builtin/pack-objects.c | 4 +-
builtin/prune.c | 4 +-
builtin/receive-pack.c | 10 +--
builtin/reflog.c | 24 +++----
builtin/show-branch.c | 4 +-
builtin/worktree.c | 4 +-
bundle.c | 4 +-
cache.h | 14 ++---
commit.c | 16 ++---
commit.h | 2 +-
config.mak.uname | 2 +
credential-cache--daemon.c | 12 ++--
date.c | 90 +++++++++++++--------------
fetch-pack.c | 8 +--
fsck.c | 2 +-
git-compat-util.h | 10 +++
http-backend.c | 4 +-
parse-options-cb.c | 4 +-
pretty.c | 4 +-
reachable.c | 10 ++-
reachable.h | 4 +-
ref-filter.c | 22 +++----
reflog-walk.c | 8 +--
refs.c | 14 ++---
refs.h | 8 +--
refs/files-backend.c | 4 +-
revision.c | 6 +-
revision.h | 4 +-
sha1_name.c | 6 +-
t/helper/test-date.c | 11 ++--
t/helper/test-parse-options.c | 4 +-
t/t0006-date.sh | 4 +-
t/t5000-tar-tree.sh | 6 +-
t/test-lib.sh | 2 +
tag.c | 4 +-
tag.h | 2 +-
upload-pack.c | 8 +--
vcs-svn/fast_export.c | 8 +--
vcs-svn/fast_export.h | 4 +-
vcs-svn/svndump.c | 2 +-
wt-status.c | 2 +-
51 files changed, 221 insertions(+), 199 deletions(-)
base-commit: e7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7
Published-As: https://github.com/dscho/git/releases/tag/time_t-may-be-int64-v1
Fetch-It-Via: git fetch https://github.com/dscho/git time_t-may-be-int64-v1
--
2.11.1.windows.1.379.g44ae0bc
^ 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