* [PATCH v4 18/22] t1700: test shared index file expiration
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>
---
t/t1700-split-index.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index 21f43903f8..480d3a8dc3 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -310,4 +310,48 @@ test_expect_success 'check splitIndex.maxPercentChange set to 0' '
test_cmp expect actual
'
+test_expect_success 'shared index files expire after 2 weeks by default' '
+ : >ten &&
+ git update-index --add ten &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_under_2_weeks_ago=$((5-14*86400)) &&
+ test-chmtime =$just_under_2_weeks_ago .git/sharedindex.* &&
+ : >eleven &&
+ git update-index --add eleven &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_over_2_weeks_ago=$((-1-14*86400)) &&
+ test-chmtime =$just_over_2_weeks_ago .git/sharedindex.* &&
+ : >twelve &&
+ git update-index --add twelve &&
+ test $(ls .git/sharedindex.* | wc -l) = 1
+'
+
+test_expect_success 'check splitIndex.sharedIndexExpire set to 16 days' '
+ git config splitIndex.sharedIndexExpire "16.days.ago" &&
+ test-chmtime =$just_over_2_weeks_ago .git/sharedindex.* &&
+ : >thirteen &&
+ git update-index --add thirteen &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ just_over_16_days_ago=$((-1-16*86400)) &&
+ test-chmtime =$just_over_16_days_ago .git/sharedindex.* &&
+ : >fourteen &&
+ git update-index --add fourteen &&
+ test $(ls .git/sharedindex.* | wc -l) = 1
+'
+
+test_expect_success 'check splitIndex.sharedIndexExpire set to "never" and "now"' '
+ git config splitIndex.sharedIndexExpire never &&
+ just_10_years_ago=$((-365*10*86400)) &&
+ test-chmtime =$just_10_years_ago .git/sharedindex.* &&
+ : >fifteen &&
+ git update-index --add fifteen &&
+ test $(ls .git/sharedindex.* | wc -l) -gt 1 &&
+ git config splitIndex.sharedIndexExpire now &&
+ just_1_second_ago=-1 &&
+ test-chmtime =$just_1_second_ago .git/sharedindex.* &&
+ : >sixteen &&
+ git update-index --add sixteen &&
+ test $(ls .git/sharedindex.* | wc -l) = 1
+'
+
test_done
--
2.12.0.22.g0672473d40
^ permalink raw reply related
* Re: [PATCH 10/10] submodule--helper clone: check for configured submodules using helper
From: Brandon Williams @ 2017-02-27 18:38 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kYmmeVOYSPTvAbGpRQn1YL9yjPOkT5xCoSDmtNzQ1t2fw@mail.gmail.com>
On 02/23, Stefan Beller wrote:
> On Thu, Feb 23, 2017 at 3:47 PM, Brandon Williams <bmwill@google.com> wrote:
>
> > @@ -795,14 +794,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
> > }
> >
> > /*
> > - * Looking up the url in .git/config.
> > + * Check if the submodule has been initialized.
> > * We must not fall back to .gitmodules as we only want
> > * to process configured submodules.
>
> This sentence "we must not ..." is also no longer accurate,
> as we do exactly that when using sub->url instead of just url
> below.
You're right, I'll fix that.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v5 1/1] config: add conditional include
From: Junio C Hamano @ 2017-02-27 18:42 UTC (permalink / raw)
To: Jeff King; +Cc: Duy Nguyen, Git Mailing List, Sebastian Schuberth, Matthieu Moy
In-Reply-To: <20170226060726.mvdip4vnbdngypzx@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I don't think driving that with a two-entry table is the right thing
> here. We are as likely to add another "foobar:" entry as we are to add
> another modifier "/i" modifier to "gitdir:", and it is unclear whether
> that modifier would be mutually exclusive with "/i".
OK, I didn't take /i as something that was meant as a modifier; I
took the "gitdir:" and "gitdir/i:" are totally different tests that
are spelled similarly, but for the implementation expediency, called
into a single helper function without having a layer that presents
the same function signature in the middle to make it drivable by a
table.
Let's leave it to the review of a future patch that wants to add a
third condition then. At that time, we will have more things to
look at to make a better decision.
^ permalink raw reply
* [PATCH v4 13/22] Documentation/config: add splitIndex.maxPercentChange
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 | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 61a863adeb..8e745bda52 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2831,6 +2831,19 @@ showbranch.default::
The default set of branches for linkgit:git-show-branch[1].
See linkgit:git-show-branch[1].
+splitIndex.maxPercentChange::
+ When the split index feature is used, this specifies the
+ percent of entries the split index can contain compared to the
+ total number of entries in both the split index and the shared
+ index before a new shared index is written.
+ The value should be between 0 and 100. If the value is 0 then
+ a new shared index is always written, if it is 100 a new
+ shared index is never written.
+ By default the value is 20, so a new shared index is written
+ if the number of entries in the split index would be greater
+ than 20 percent of the total number of entries.
+ 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] gitweb tests: Skip tests when we don't have Time::HiRes
From: Ævar Arnfjörð Bjarmason @ 2017-02-27 18:25 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <4b34e3a0-3da7-d821-2a7f-9a420ac1d3f6@gmail.com>
On Mon, Feb 27, 2017 at 6:48 PM, Jakub Narębski <jnareb@gmail.com> wrote:
> W dniu 27.02.2017 o 13:37, Ævar Arnfjörð Bjarmason pisze:
>> Change the gitweb tests to skip when we can't load the Time::HiRes
>> module.
>
> Could you tell us in the commit message why this module is needed?
> Is it because gitweb loads it unconditionally, or does that at least
> in the default configuration, or is it used in tests, or...?
>
> [I see it is somewhat addressed below]
>
>>
>> This module has bee in perl core since v5.8, which is the oldest
>
> s/bee/been/
I'll clarify that in a re-roll & fix the typo, pending any other
comments. Thanks!
>> version we support, however CentOS (and perhaps some other
>> distributions) carve it into its own non-core-perl package that's not
>> installed along with /usr/bin/perl by default. Without this we'll hard
>> fail the gitweb tests when trying to load the module.
>
> I see that it because gitweb.perl as the following at line 20:
>
> use Time::HiRes qw(gettimeofday tv_interval);
>
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> Good catch (if a strange one...).
This and the associated cvs tests failing as root patch I submitted
were discovered when trying to build git in the standard mock build
environment on CentOS. It creates a chroot and rpm installs just the
packages you declare, so issues like these crop up.
>> ---
>> t/gitweb-lib.sh | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
>> index d5dab5a94f..116c3890e6 100644
>> --- a/t/gitweb-lib.sh
>> +++ b/t/gitweb-lib.sh
>> @@ -114,4 +114,9 @@ perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {
>> test_done
>> }
>>
>> +perl -mTime::HiRes -e 0 >/dev/null 2>&1 || {
>> + skip_all='skipping gitweb tests, Time::HiRes module unusable'
>
> Is "unusable" a good description, instead of "not found"?
Yeah it's odd, but I just copied the several lines above that use that phrasing.
>> + test_done
>> +}
>> +
>> gitweb_init
>>
>
^ permalink raw reply
* Re: [PATCH 0/5] recursing submodules with relative pathspec (grep and ls-files)
From: Brandon Williams @ 2017-02-27 17:52 UTC (permalink / raw)
To: git; +Cc: sbeller, pclouds
In-Reply-To: <20170224235100.52627-1-bmwill@google.com>
On 02/24, Brandon Williams wrote:
> It was discovered that when using the --recurse-submodules flag with `git grep`
> and `git ls-files` and specifying a relative path when not at the root causes
> the child processes spawned to error out with an error like:
>
> fatal: ..: '..' is outside repository
>
> While true that ".." is outside the scope of the submodule repository, it
> probably doesn't make much sense to the user who gave that pathspec with
> respect to the superproject. Since the child processes that are spawned to
> handle the submodules have some context that they are executing underneath a
> superproject (via the 'super_prefix'), they should be able to prevent dying
> under this circumstance.
>
> This series fixes this bug in both git grep and git ls-files as well as
> correctly formatting the output from submodules to handle the relative paths
> with "..".
>
> One of the changes made to fix this was to add an additional flag for the
> parse_pathspec() function in order to treat all paths provided as being from
> the root of the repository. I hesitantly selected the name 'PATHSPEC_FROMROOT'
> but I'm not fond of it since its too similar to the pathspec magic define
> 'PATHSPEC_FROMTOP'. So I'm open for naming suggestions.
>
> Brandon Williams (5):
> grep: illustrate bug when recursing with relative pathspec
> pathspec: add PATHSPEC_FROMROOT flag
> grep: fix bug when recuring with relative pathspec
> ls-files: illustrate bug when recursing with relative pathspec
> ls-files: fix bug when recuring with relative pathspec
>
> builtin/grep.c | 8 ++++--
> builtin/ls-files.c | 8 ++++--
> pathspec.c | 2 +-
> pathspec.h | 2 ++
> t/t3007-ls-files-recurse-submodules.sh | 50 ++++++++++++++++++++++++++++++++++
> t/t7814-grep-recurse-submodules.sh | 42 ++++++++++++++++++++++++++++
> 6 files changed, 107 insertions(+), 5 deletions(-)
>
Turns out that this series doesn't address all of the issues. This
series also seems to introduce broken behavior when recursing from a
subdirectory. So I need to think about this problem a little bit more
and reroll.
--
Brandon Williams
^ permalink raw reply
* [PATCH v4 15/22] read-cache: touch shared index files when used
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>
When a split-index file is created, let's update the mtime of the
shared index file that the split-index file is referencing.
In a following commit we will make shared index file expire
depending on their mtime, so updating the mtime makes sure that
the shared index file will not be deleted soon.
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
read-cache.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index aeb413a508..5f295af4c6 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1674,6 +1674,19 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist)
die("index file corrupt");
}
+/*
+ * Signal that the shared index is used by updating its mtime.
+ *
+ * This way, shared index can be removed if they have not been used
+ * for some time.
+ */
+static void freshen_shared_index(char *base_sha1_hex, int warn)
+{
+ const char *shared_index = git_path("sharedindex.%s", base_sha1_hex);
+ if (!check_and_freshen_file(shared_index, 1) && warn)
+ warning("Could not freshen shared index '%s'", shared_index);
+}
+
int read_index_from(struct index_state *istate, const char *path)
{
struct split_index *split_index;
@@ -2245,6 +2258,7 @@ static int too_many_not_shared_entries(struct index_state *istate)
int write_locked_index(struct index_state *istate, struct lock_file *lock,
unsigned flags)
{
+ int new_shared_index, ret;
struct split_index *si = istate->split_index;
if (!si || alternate_index_output ||
@@ -2261,13 +2275,22 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
}
if (too_many_not_shared_entries(istate))
istate->cache_changed |= SPLIT_INDEX_ORDERED;
- if (istate->cache_changed & SPLIT_INDEX_ORDERED) {
- int ret = write_shared_index(istate, lock, flags);
+
+ new_shared_index = istate->cache_changed & SPLIT_INDEX_ORDERED;
+
+ if (new_shared_index) {
+ ret = write_shared_index(istate, lock, flags);
if (ret)
return ret;
}
- return write_split_index(istate, lock, flags);
+ ret = write_split_index(istate, lock, flags);
+
+ /* Freshen the shared index only if the split-index was written */
+ if (!ret && !new_shared_index)
+ freshen_shared_index(sha1_to_hex(si->base_sha1), 1);
+
+ return ret;
}
/*
--
2.12.0.22.g0672473d40
^ permalink raw reply related
* [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
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