* Re: [PATCH/RFC 0/7] Pie-in-the-sky attempt to fix the early config
From: Jeff King @ 2016-12-08 17:26 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
On Thu, Dec 08, 2016 at 04:35:56PM +0100, Johannes Schindelin wrote:
> The idea here is to discover the .git/ directory gently (i.e. without
> changing the current working directory), and to use it to read the
> .git/config file early, before we actually called setup_git_directory()
> (if we ever do that).
Great. Thanks for taking a stab at this.
> Notes:
>
> - I find the diff pretty ugly: I wish there was a more elegant way to
> *disable* discovery of .git/ *just* for `init` and `clone`. I
> considered a function `about_to_create_git_dir()` that is called in a
> hard-coded manner *only* for `init` and `clone`, but that would
> introduce another magic side effect, when all I want is to reduce those.
It looks like a lot of that ugliness comes from passing around the "are
we OK to peek at git-dir config" flag through the various pager-related
calls. I don't think it would be bad to use a global for "we do not
want a repo". After all, it's just modifying the _existing_ global for
"are we in a repo". And I do not see that global going away anytime
soon. Sometimes it's good to make incremental steps towards an end goal,
but in this case it seems like we just pay the cost of the step without
any real plan for reaping the benefit in the long term.
As an alternative, I also think it would be OK to just always have the
pager config read from local repo, even for init/clone. In other words,
to loosen the idea that git-init can _never_ look in the current
git-dir, and declare that there is a stage before the command is
initiated, and during which git may read local-repo config. Aliases
would fall into this, too, so:
git config --local alias.foo init
git foo /some/other/dir
would work (as it must, because we cannot know that "foo" is "init"
until we read the config!).
I have a feeling you may declare that too magical, but I think it's
consistent and practical.
> - For the moment, I do not handle dashed invocations of `init` and
> `clone` correctly. The real problem is the continued existence of
> the dashed git-init and git-clone, of course.
I assume you mean setting the CREATES_GIT_DIR flag here? I think it
would apply to the dashed invocations, too. They strip off the "git-"
and end up in handle_builtin() just like "git clone" does. I didn't test
it, though.
> - There is still duplicated code. For the sake of this RFC, I did not
> address that yet.
Yeah, I did not read your discover function very carefully. Because I
think the one thing we really don't want to do here is introduce a
separate lookup process that is not shared by setup_git_directory(). The
only sane path, I think, is to refactor setup_git_directory() to build
on top of discover_git_directory(), which implies that the latter
handles all of the cases.
> - The read_early_config() function is called multiple times, re-reading
> all the config files and re-discovering the .git/ directory multiple
> times, which is quite wasteful. For the sake of this RFC, I did not
> address that yet.
We already have a config-caching system. If we went with a global
"config_discover_refs", then I think the sequence for something like
git-init would become:
1. When git.c starts, config_discover_refs is set to "true". Pager and
alias lookup may look in .git/config if it's available, even if
they go through the configset cache.
2. As soon as git-init starts, it disables config_discover_refs, and
it flushes the config cache. Any configset lookups will now examine
the reduced config.
3. When git-init has set up the real repo it is operating on, it can
reenable config_discover_refs (though it may not even need to; that
flag probably wouldn't have any effect once we've entered the
repository and have_git_dir() returns true).
> - t7006 fails and the error message is a bit cryptic (not to mention the
> involved function trace, which is mind-boggling for what is supposed
> to be a simply, shell script-based test suite). I did not have time to
> look into that yet.
Running t7006 I see a lot of old failures turned into successes, which
is good (because running from a subdirectory now actually respects local
pager config). The one failure looks like it is testing the wrong thing.
It is checking that we _don't_ respect a local core.pager in some cases,
as a proxy for whether or not we are breaking things by doing setup too
early. But the whole point of your series is to fix that, and respect
the config without causing the setup breakage. After your patches, the
proxy behavior and the failure case are disconnected. The test should be
flipped, and ideally another one added that confirms we didn't actually
run setup_git_directory(), but I'm not sure how to test that directly.
> - after discover_git_directory_gently() did its work, the code happily
> uses its result *only* for the current read_early_config() run, and
> lets setup_git_dir_gently() do the whole work *again*. For the sake of
> this RFC, I did not address that yet.
If caching happens at the config layer, then we'd probably only call
this once anyway (or if we did call it again after a config flush, it
would be a good sign that we should compute its value again).
-Peff
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Junio C Hamano @ 2016-12-08 16:54 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Brandon Williams, git, sbeller, peff, jacob.keller
In-Reply-To: <1767f01a-4125-d99b-37db-3f4a56aaa28a@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> Am 07.12.2016 um 23:29 schrieb Brandon Williams:
>> Instead of assuming root is "/"
>> I'll need to extract what root is from an absolute path. Aside from
>> what root looks like, do most other path constructs behave similarly in
>> unix and windows? (like ".." and "." as examples)
>
> Yes, .. and . work the same way, except that they cannot appear in the
> \\server\share part. I also think that .. does not cancel these parts.
>
> As long as you use is_absolute_path() and do not simplify path
> components before offset_1st_component(), you should be on the safe
> side.
>
>> Since I don't really have a windows machine to test things it might be
>> slightly difficult to get everything correct quickly but hopefully we can
>> get this working :)
>
> I'll lend a hand, of course, as time permits.
Thanks, as always, for working well together ;-).
^ permalink raw reply
* Re: [PATCH v2] commit: make --only --allow-empty work without paths
From: Junio C Hamano @ 2016-12-08 16:47 UTC (permalink / raw)
To: Andreas Krey; +Cc: git, Jeff King
In-Reply-To: <20161208135029.GA16292@inner.h.apk.li>
Andreas Krey <a.krey@gmx.de> writes:
> Ok, I've removed the clever message, as Junio suggested.
> I don't know what else to do to make it acceptable. :-)
> We're going to deploy it internally anyway, but I think
> it belongs in git.git as well (aka 'Can I has "will queue"?').
Oh, sorry for being unclear. Before I started saying "Slightly
related topic.", after quoting "The patch itself looks good to me."
by Peff, I meant to say "Yeah, this looks good; thanks.", but
apparently I forgot.
Removal of "Clever" is a separate issue and it may make sense to do
so, but it deserves its own commit with its own justification.
Sorry for making you send an extra round; let's queue the original,
and if you still are interested, have the "Clever" removal as its
own patch.
Thanks.
^ permalink raw reply
* [PATCH/RFC 6/7] WIP read_config_early(): respect ceiling directories
From: Johannes Schindelin @ 2016-12-08 15:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
This ports the part from setup_git_directory_gently_1() where the
GIT_CEILING_DIRECTORIES environment variable is handled.
TODO: DRY up code again (exporting canonicalize_ceiling_directories()?)
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 53 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index 286d3cad66..eda3546491 100644
--- a/config.c
+++ b/config.c
@@ -1386,6 +1386,37 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
}
/*
+ * A "string_list_each_func_t" function that canonicalizes an entry
+ * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
+ * discards it if unusable. The presence of an empty entry in
+ * GIT_CEILING_DIRECTORIES turns off canonicalization for all
+ * subsequent entries.
+ */
+static int canonicalize_ceiling_entry(struct string_list_item *item,
+ void *cb_data)
+{
+ int *empty_entry_found = cb_data;
+ char *ceil = item->string;
+
+ if (!*ceil) {
+ *empty_entry_found = 1;
+ return 0;
+ } else if (!is_absolute_path(ceil)) {
+ return 0;
+ } else if (*empty_entry_found) {
+ /* Keep entry but do not canonicalize it */
+ return 1;
+ } else {
+ const char *real_path = real_path_if_valid(ceil);
+ if (!real_path)
+ return 0;
+ free(item->string);
+ item->string = xstrdup(real_path);
+ return 1;
+ }
+}
+
+/*
* Note that this is a really dirty hack that replicates what the
* setup_git_directory() function does, without changing the current
* working directory. The crux of the problem is that we cannot run
@@ -1394,6 +1425,8 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
*/
static int discover_git_directory_gently(struct strbuf *result)
{
+ const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
+ int ceiling_offset = -1;
const char *p;
if (strbuf_getcwd(result) < 0)
@@ -1403,6 +1436,23 @@ static int discover_git_directory_gently(struct strbuf *result)
return -1;
strbuf_reset(result);
strbuf_addstr(result, p);
+
+ if (env_ceiling_dirs) {
+ struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
+ int empty_entry_found = 0;
+
+ string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP,
+ -1);
+ filter_string_list(&ceiling_dirs, 0, canonicalize_ceiling_entry,
+ &empty_entry_found);
+ ceiling_offset = longest_ancestor_length(result->buf,
+ &ceiling_dirs);
+ string_list_clear(&ceiling_dirs, 0);
+ }
+
+ if (ceiling_offset < 0 && has_dos_drive_prefix(result->buf))
+ ceiling_offset = 1;
+
for (;;) {
int len = result->len, i;
@@ -1418,10 +1468,10 @@ static int discover_git_directory_gently(struct strbuf *result)
strbuf_setlen(result, len);
if (is_git_directory(result->buf))
return 0;
- for (i = len; i > 0; )
- if (is_dir_sep(result->buf[--i]))
+ for (i = len; --i > ceiling_offset; )
+ if (is_dir_sep(result->buf[i]))
break;
- if (!i)
+ if (i <= ceiling_offset)
return -1;
strbuf_setlen(result, i);
}
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH/RFC 7/7] WIP: read_early_config(): add tests
From: Johannes Schindelin @ 2016-12-08 15:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/helper/test-config.c | 15 +++++++++++++++
t/t1309-early-config.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
create mode 100755 t/t1309-early-config.sh
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 83a4f2ab86..5105069587 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -66,6 +66,16 @@ static int iterate_cb(const char *var, const char *value, void *data)
return 0;
}
+static int early_config_cb(const char *var, const char *value, void *vdata)
+{
+ const char *key = vdata;
+
+ if (!strcmp(key, var))
+ printf("%s\n", value);
+
+ return 0;
+}
+
int cmd_main(int argc, const char **argv)
{
int i, val;
@@ -73,6 +83,11 @@ int cmd_main(int argc, const char **argv)
const struct string_list *strptr;
struct config_set cs;
+ if (argc == 3 && !strcmp(argv[1], "read_early_config")) {
+ read_early_config(early_config_cb, (void *)argv[2], 1);
+ return 0;
+ }
+
setup_git_directory();
git_configset_init(&cs);
diff --git a/t/t1309-early-config.sh b/t/t1309-early-config.sh
new file mode 100755
index 0000000000..0c55dee514
--- /dev/null
+++ b/t/t1309-early-config.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='Test read_early_config()'
+
+. ./test-lib.sh
+
+test_expect_success 'read early config' '
+ test_config early.config correct &&
+ test-config read_early_config early.config >output &&
+ test correct = "$(cat output)"
+'
+
+test_expect_success 'in a sub-directory' '
+ test_config early.config sub &&
+ mkdir -p sub &&
+ (
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test sub = "$(cat output)"
+'
+
+test_expect_success 'ceiling' '
+ test_config early.config ceiling &&
+ mkdir -p sub &&
+ (
+ GIT_CEILING_DIRECTORIES="$PWD" &&
+ export GIT_CEILING_DIRECTORIES &&
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test -z "$(cat output)"
+'
+
+test_expect_success 'ceiling #2' '
+ mkdir -p xdg/git &&
+ git config -f xdg/git/config early.config xdg &&
+ test_config early.config ceiling &&
+ mkdir -p sub &&
+ (
+ XDG_CONFIG_HOME="$PWD"/xdg &&
+ GIT_CEILING_DIRECTORIES="$PWD" &&
+ export GIT_CEILING_DIRECTORIES XDG_CONFIG_HOME &&
+ cd sub &&
+ test-config read_early_config early.config
+ ) >output &&
+ test xdg = "$(cat output)"
+'
+
+test_done
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* Re: Serious bug with Git-2.11.0-64-bit and Git-LFS
From: Nick Warr @ 2016-12-08 15:36 UTC (permalink / raw)
To: Lars Schneider; +Cc: git
In-Reply-To: <D1437EA2-F4D3-4190-8D79-020C06CFA3DB@gmail.com>
On 8 December 2016 at 14:18, Lars Schneider <larsxschneider@gmail.com> wrote:
>
>> On 08 Dec 2016, at 15:00, Nick Warr <nick.warr@bossastudios.com> wrote:
>>
>> That looks pretty much like the error we're dealing with, any reason
>> why going back a point version on Git (not git-lfs) would resolve the
>> issue however?
>
> Going back to GitLFS 1.4.* would make the error disappear. However, I think
> you should fix your repository. Try this:
>
> git lfs clone <YOUR REPO URL>
> cd <YOUR REPO>
> git rm --cached "workers/unity/Assets/3rdPartyAssets/FORGE3D/Sci-Fi Effects/Effects/Debris/Meshes/debris_junk.FBX"
> git add --force "workers/unity/Assets/3rdPartyAssets/FORGE3D/Sci-Fi Effects/Effects/Debris/Meshes/debris_junk.FBX"
> git commit -m "Move files properly to GitLFS"
> git push
>
> Afterwards you should be able to use the latest version of Git and GitLFS
> without trouble.
>
> Cheers,
> Lars
>
> PS: Top posting is not that popular in the Git community ;-)
>
>
>>
>> On 8 December 2016 at 13:57, Lars Schneider <larsxschneider@gmail.com> wrote:
>>>
>>>> On 08 Dec 2016, at 12:46, Nick Warr <nick.warr@bossastudios.com> wrote:
>>>>
>>>> Using Git-2.11.0 with the latest git-lfs 1.5.2 (also tested with
>>>> 1.5.3) cloning from our locally hosted gitlab CE server via HTTPS.
>>>>
>>>> When cloning a repo (large, 3.3 gig in git, 10.3 in LFS) for the
>>>> first time the clone will finish the checkout of the git part, then
>>>> when it starts downloading the LFS parts it will reliably finish with
>>>> a smudge filter error.
>>>>
>>>> This leaves the repo in an unstable condition, you can then fetch the
>>>> lfs part without issue, but checking out the lfs files or trying a git
>>>> reset --hard will continue to spit out the same error. As you can see,
>>>> the actual error is not particularly useful.
>>>>
>>>> fatal: workers/unity/Assets/3rdPartyAssets/FORGE3D/Sci-Fi
>>>> Effects/Effects/Debris/Meshes/debris_junk.FBX: smudge filter lfs
>>>> failed
>>>> Unknown command ""
>>>>
>>>> Possibly it's due to the file extension being all capital letters, we
>>>> did manage to change the error by recommitting the file with a
>>>> lowercase extension, but it failed on the next file (which also had a
>>>> capital letter extension).
>>>>
>>>> This has happened on multiple fresh windows 10 64 bit installs,
>>>> different machines and target directories (to hopefully remove the
>>>> possibility of file permissions) where cloning is taking place.
>>>>
>>>> The solution is to back level to Git 2.10.2 and the error disappears.
>>>>
>>>> More than willing to provide any further information,
>>>
>>> Hi Nick,
>>>
>>> debris_junk.FBX is not stored properly in Git LFS.
>>> I explained the problem in detail here:
>>> https://github.com/git-lfs/git-lfs/issues/1729
>>>
>>> You should add the file properly to GitLFS to fix the problem.
>>> However, I think this is a regression in GitLFS and I hope it will
>>> be fixed in the next version.
>>>
>>> No change/fix in Git is required.
>>>
>>> Cheers,
>>> Lars
>
Thank gmail for the top posting, hard enough getting it to send non html mail :(
Just to add a bit more to the discussion, we went and had a look at
that directory which was freshly cloned with 2.10.2 and 1.5.3 and the
files were there (400 KB + FBX files, not 2KB pointers) after the git
reset --hard, we're actually looking at renaming all those files, as
that isn't the only one unfortunately..
^ permalink raw reply
* [PATCH/RFC 5/7] read_early_config(): really discover .git/
From: Johannes Schindelin @ 2016-12-08 15:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
Earlier, we punted and simply assumed that we are in the top-level
directory of the project, and that there is no .git file but a .git/
directory so that we can read directly from .git/config.
Let's discover the .git/ directory correctly.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.c | 69 +++++++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 49 insertions(+), 20 deletions(-)
diff --git a/config.c b/config.c
index 4c756edca1..286d3cad66 100644
--- a/config.c
+++ b/config.c
@@ -1385,35 +1385,64 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
}
}
+/*
+ * Note that this is a really dirty hack that replicates what the
+ * setup_git_directory() function does, without changing the current
+ * working directory. The crux of the problem is that we cannot run
+ * setup_git_directory() early on in git's setup, so we have to
+ * duplicate the work that setup_git_directory() would otherwise do.
+ */
+static int discover_git_directory_gently(struct strbuf *result)
+{
+ const char *p;
+
+ if (strbuf_getcwd(result) < 0)
+ return -1;
+ p = real_path_if_valid(result->buf);
+ if (!p)
+ return -1;
+ strbuf_reset(result);
+ strbuf_addstr(result, p);
+ for (;;) {
+ int len = result->len, i;
+
+ strbuf_addstr(result, "/" DEFAULT_GIT_DIR_ENVIRONMENT);
+ p = read_gitfile_gently(result->buf, &i);
+ if (p) {
+ strbuf_reset(result);
+ strbuf_addstr(result, p);
+ return 0;
+ }
+ if (is_git_directory(result->buf))
+ return 0;
+ strbuf_setlen(result, len);
+ if (is_git_directory(result->buf))
+ return 0;
+ for (i = len; i > 0; )
+ if (is_dir_sep(result->buf[--i]))
+ break;
+ if (!i)
+ return -1;
+ strbuf_setlen(result, i);
+ }
+}
+
void read_early_config(config_fn_t cb, void *data, int discover_git_dir)
{
+ struct strbuf buf = STRBUF_INIT;
+
git_config_with_options(cb, data, NULL, 1);
- /*
- * Note that this is a really dirty hack that does the wrong thing in
- * many cases. The crux of the problem is that we cannot run
- * setup_git_directory() early on in git's setup, so we have no idea if
- * we are in a repository or not, and therefore are not sure whether
- * and how to read repository-local config.
- *
- * So if we _aren't_ in a repository (or we are but we would reject its
- * core.repositoryformatversion), we'll read whatever is in .git/config
- * blindly. Similarly, if we _are_ in a repository, but not at the
- * root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc). See t7006 for a complete set of failures.
- *
- * However, we have historically provided this hack because it does
- * work some of the time (namely when you are at the top-level of a
- * valid repository), and would rarely make things worse (i.e., you do
- * not generally have a .git/config file sitting around).
- */
- if (discover_git_dir && !have_git_dir()) {
+ if (discover_git_dir && !have_git_dir() &&
+ !discover_git_directory_gently(&buf)) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
- repo_config.file = ".git/config";
+ strbuf_addstr(&buf, "/config");
+ repo_config.file = buf.buf;
git_config_with_options(cb, data, &repo_config, 1);
}
+ strbuf_release(&buf);
}
static void git_config_check_init(void);
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH/RFC 4/7] read_early_config(): special-case `init` and `clone`
From: Johannes Schindelin @ 2016-12-08 15:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
The `init` and `clone` commands create their own .git/ directory,
therefore we must be careful not to read any repository config when
determining the pager settings.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin/am.c | 2 +-
builtin/blame.c | 2 +-
builtin/grep.c | 4 ++--
builtin/log.c | 4 ++--
builtin/var.c | 2 +-
cache.h | 9 +++++----
config.c | 4 ++--
diff.c | 4 ++--
git.c | 16 ++++++++--------
| 13 +++++++------
10 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 6981f42ce9..e6c2ee01bc 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1791,7 +1791,7 @@ static int do_interactive(struct am_state *state)
}
strbuf_release(&msg);
} else if (*reply == 'v' || *reply == 'V') {
- const char *pager = git_pager(1);
+ const char *pager = git_pager(1, 1);
struct child_process cp = CHILD_PROCESS_INIT;
if (!pager)
diff --git a/builtin/blame.c b/builtin/blame.c
index 4ddfadb71f..5b7daa3686 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2913,7 +2913,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
assign_blame(&sb, opt);
if (!incremental)
- setup_pager();
+ setup_pager(1);
free(final_commit_name);
diff --git a/builtin/grep.c b/builtin/grep.c
index 8887b6addb..363a753369 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -800,7 +800,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
}
if (show_in_pager == default_pager)
- show_in_pager = git_pager(1);
+ show_in_pager = git_pager(1, 1);
if (show_in_pager) {
opt.color = 0;
opt.name_only = 1;
@@ -896,7 +896,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
}
if (!show_in_pager && !opt.status_only)
- setup_pager();
+ setup_pager(1);
if (!use_index && (untracked || cached))
die(_("--cached or --untracked cannot be used with --no-index."));
diff --git a/builtin/log.c b/builtin/log.c
index 55d20cc2d8..96618d38cb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -203,7 +203,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
if (rev->line_level_traverse)
line_log_init(rev, line_cb.prefix, &line_cb.args);
- setup_pager();
+ setup_pager(1);
}
static void cmd_log_init(int argc, const char **argv, const char *prefix,
@@ -1600,7 +1600,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
if (!use_stdout)
output_directory = set_outdir(prefix, output_directory);
else
- setup_pager();
+ setup_pager(1);
if (output_directory) {
if (rev.diffopt.use_color != GIT_COLOR_ALWAYS)
diff --git a/builtin/var.c b/builtin/var.c
index aedbb53a2d..879867b842 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -19,7 +19,7 @@ static const char *editor(int flag)
static const char *pager(int flag)
{
- const char *pgm = git_pager(1);
+ const char *pgm = git_pager(1, 1);
if (!pgm)
pgm = "cat";
diff --git a/cache.h b/cache.h
index 96e0cb2523..6627239de8 100644
--- a/cache.h
+++ b/cache.h
@@ -1325,7 +1325,7 @@ extern const char *fmt_name(const char *name, const char *email);
extern const char *ident_default_name(void);
extern const char *ident_default_email(void);
extern const char *git_editor(void);
-extern const char *git_pager(int stdout_is_tty);
+extern const char *git_pager(int stdout_is_tty, int discover_git_dir);
extern int git_ident_config(const char *, const char *, void *);
extern void reset_ident_date(void);
@@ -1692,7 +1692,8 @@ extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
const char *name, const char *buf, size_t len, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
-extern void read_early_config(config_fn_t cb, void *data);
+extern void read_early_config(config_fn_t cb, void *data,
+ int discover_git_dir);
extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
@@ -1888,12 +1889,12 @@ __attribute__((format (printf, 2, 3)))
extern void write_file(const char *path, const char *fmt, ...);
/* pager.c */
-extern void setup_pager(void);
+extern void setup_pager(int discover_git_dir);
extern int pager_in_use(void);
extern int pager_use_color;
extern int term_columns(void);
extern int decimal_width(uintmax_t);
-extern int check_pager_config(const char *cmd);
+extern int check_pager_config(const char *cmd, int discover_git_dir);
extern void prepare_pager_args(struct child_process *, const char *pager);
extern const char *editor_program;
diff --git a/config.c b/config.c
index 104c3c3dcd..4c756edca1 100644
--- a/config.c
+++ b/config.c
@@ -1385,7 +1385,7 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
}
}
-void read_early_config(config_fn_t cb, void *data)
+void read_early_config(config_fn_t cb, void *data, int discover_git_dir)
{
git_config_with_options(cb, data, NULL, 1);
@@ -1407,7 +1407,7 @@ void read_early_config(config_fn_t cb, void *data)
* valid repository), and would rarely make things worse (i.e., you do
* not generally have a .git/config file sitting around).
*/
- if (!have_git_dir()) {
+ if (discover_git_dir && !have_git_dir()) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
diff --git a/diff.c b/diff.c
index ec8728362d..0769c0590a 100644
--- a/diff.c
+++ b/diff.c
@@ -5267,6 +5267,6 @@ void setup_diff_pager(struct diff_options *opt)
* --exit-code" in hooks and other scripts, we do not do so.
*/
if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&
- check_pager_config("diff") != 0)
- setup_pager();
+ check_pager_config("diff", 1) != 0)
+ setup_pager(1);
}
diff --git a/git.c b/git.c
index 61df78afc8..2b007b83ec 100644
--- a/git.c
+++ b/git.c
@@ -61,13 +61,13 @@ static void restore_env(int external_alias)
}
}
-static void commit_pager_choice(void) {
+static void commit_pager_choice(int discover_git_dir) {
switch (use_pager) {
case 0:
setenv("GIT_PAGER", "cat", 1);
break;
case 1:
- setup_pager();
+ setup_pager(discover_git_dir);
break;
default:
break;
@@ -261,7 +261,7 @@ static int handle_alias(int *argcp, const char ***argv)
if (alias_string[0] == '!') {
struct child_process child = CHILD_PROCESS_INIT;
- commit_pager_choice();
+ commit_pager_choice(1);
restore_env(1);
child.use_shell = 1;
@@ -349,7 +349,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
}
if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY))
- use_pager = check_pager_config(p->cmd);
+ use_pager = check_pager_config(p->cmd, !(p->option & CREATES_GIT_DIR));
if (use_pager == -1 && p->option & USE_PAGER)
use_pager = 1;
@@ -357,7 +357,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
startup_info->have_repository) /* get_git_dir() may set up repo, avoid that */
trace_repo_setup(prefix);
}
- commit_pager_choice();
+ commit_pager_choice(!(p->option & CREATES_GIT_DIR));
if (!help && get_super_prefix()) {
if (!(p->option & SUPPORT_SUPER_PREFIX))
@@ -584,8 +584,8 @@ static void execv_dashed_external(const char **argv)
die("%s doesn't support --super-prefix", argv[0]);
if (use_pager == -1)
- use_pager = check_pager_config(argv[0]);
- commit_pager_choice();
+ use_pager = check_pager_config(argv[0], 1);
+ commit_pager_choice(1);
strbuf_addf(&cmd, "git-%s", argv[0]);
@@ -688,7 +688,7 @@ int cmd_main(int argc, const char **argv)
skip_prefix(argv[0], "--", &argv[0]);
} else {
/* The user didn't specify a command; give them help */
- commit_pager_choice();
+ commit_pager_choice(1);
printf("usage: %s\n\n", git_usage_string);
list_common_cmds_help();
printf("\n%s\n", _(git_more_info_string));
--git a/pager.c b/pager.c
index 73ca8bc3b1..16b3cbe232 100644
--- a/pager.c
+++ b/pager.c
@@ -43,7 +43,7 @@ static int core_pager_config(const char *var, const char *value, void *data)
return 0;
}
-const char *git_pager(int stdout_is_tty)
+const char *git_pager(int stdout_is_tty, int discover_git_dir)
{
const char *pager;
@@ -53,7 +53,8 @@ const char *git_pager(int stdout_is_tty)
pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
- read_early_config(core_pager_config, NULL);
+ read_early_config(core_pager_config, NULL,
+ discover_git_dir);
pager = pager_program;
}
if (!pager)
@@ -100,9 +101,9 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
setup_pager_env(&pager_process->env_array);
}
-void setup_pager(void)
+void setup_pager(int discover_git_dir)
{
- const char *pager = git_pager(isatty(1));
+ const char *pager = git_pager(isatty(1), discover_git_dir);
if (!pager)
return;
@@ -208,7 +209,7 @@ static int pager_command_config(const char *var, const char *value, void *vdata)
}
/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
-int check_pager_config(const char *cmd)
+int check_pager_config(const char *cmd, int discover_git_dir)
{
struct pager_command_config_data data;
@@ -216,7 +217,7 @@ int check_pager_config(const char *cmd)
data.want = -1;
data.value = NULL;
- read_early_config(pager_command_config, &data);
+ read_early_config(pager_command_config, &data, discover_git_dir);
if (data.value)
pager_program = data.value;
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH/RFC 2/7] read_early_config(): avoid .git/config hack when unneeded
From: Johannes Schindelin @ 2016-12-08 15:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
So far, we only look whether the startup_info claims to have seen a
git_dir.
However, do_git_config_sequence() (and consequently the
git_config_with_options() call used by read_early_config() asks the
have_git_dir() function whether we have a .git/ directory, which in turn
also looks at git_dir and at the environment variable GIT_DIR.
Let's just use the same function, have_git_dir(), to determine whether we
have to look for .git/config specifically.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/config.c b/config.c
index 7dcd8d8622..104c3c3dcd 100644
--- a/config.c
+++ b/config.c
@@ -1407,7 +1407,7 @@ void read_early_config(config_fn_t cb, void *data)
* valid repository), and would rarely make things worse (i.e., you do
* not generally have a .git/config file sitting around).
*/
- if (!startup_info->have_repository) {
+ if (!have_git_dir()) {
struct git_config_source repo_config;
memset(&repo_config, 0, sizeof(repo_config));
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH/RFC 1/7] Make read_early_config() reusable
From: Johannes Schindelin @ 2016-12-08 15:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
The pager configuration needs to be read early, possibly before
discovering any .git/ directory.
Let's not hide this function in pager.c, but make it available to other
callers.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
cache.h | 1 +
config.c | 31 +++++++++++++++++++++++++++++++
| 31 -------------------------------
3 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/cache.h b/cache.h
index a50a61a197..96e0cb2523 100644
--- a/cache.h
+++ b/cache.h
@@ -1692,6 +1692,7 @@ extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type,
const char *name, const char *buf, size_t len, void *data);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
+extern void read_early_config(config_fn_t cb, void *data);
extern void git_config(config_fn_t fn, void *);
extern int git_config_with_options(config_fn_t fn, void *,
struct git_config_source *config_source,
diff --git a/config.c b/config.c
index 83fdecb1bc..7dcd8d8622 100644
--- a/config.c
+++ b/config.c
@@ -1385,6 +1385,37 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
}
}
+void read_early_config(config_fn_t cb, void *data)
+{
+ git_config_with_options(cb, data, NULL, 1);
+
+ /*
+ * Note that this is a really dirty hack that does the wrong thing in
+ * many cases. The crux of the problem is that we cannot run
+ * setup_git_directory() early on in git's setup, so we have no idea if
+ * we are in a repository or not, and therefore are not sure whether
+ * and how to read repository-local config.
+ *
+ * So if we _aren't_ in a repository (or we are but we would reject its
+ * core.repositoryformatversion), we'll read whatever is in .git/config
+ * blindly. Similarly, if we _are_ in a repository, but not at the
+ * root, we'll fail to find .git/config (because it's really
+ * ../.git/config, etc). See t7006 for a complete set of failures.
+ *
+ * However, we have historically provided this hack because it does
+ * work some of the time (namely when you are at the top-level of a
+ * valid repository), and would rarely make things worse (i.e., you do
+ * not generally have a .git/config file sitting around).
+ */
+ if (!startup_info->have_repository) {
+ struct git_config_source repo_config;
+
+ memset(&repo_config, 0, sizeof(repo_config));
+ repo_config.file = ".git/config";
+ git_config_with_options(cb, data, &repo_config, 1);
+ }
+}
+
static void git_config_check_init(void);
void git_config(config_fn_t fn, void *data)
--git a/pager.c b/pager.c
index ae79643363..73ca8bc3b1 100644
--- a/pager.c
+++ b/pager.c
@@ -43,37 +43,6 @@ static int core_pager_config(const char *var, const char *value, void *data)
return 0;
}
-static void read_early_config(config_fn_t cb, void *data)
-{
- git_config_with_options(cb, data, NULL, 1);
-
- /*
- * Note that this is a really dirty hack that does the wrong thing in
- * many cases. The crux of the problem is that we cannot run
- * setup_git_directory() early on in git's setup, so we have no idea if
- * we are in a repository or not, and therefore are not sure whether
- * and how to read repository-local config.
- *
- * So if we _aren't_ in a repository (or we are but we would reject its
- * core.repositoryformatversion), we'll read whatever is in .git/config
- * blindly. Similarly, if we _are_ in a repository, but not at the
- * root, we'll fail to find .git/config (because it's really
- * ../.git/config, etc). See t7006 for a complete set of failures.
- *
- * However, we have historically provided this hack because it does
- * work some of the time (namely when you are at the top-level of a
- * valid repository), and would rarely make things worse (i.e., you do
- * not generally have a .git/config file sitting around).
- */
- if (!startup_info->have_repository) {
- struct git_config_source repo_config;
-
- memset(&repo_config, 0, sizeof(repo_config));
- repo_config.file = ".git/config";
- git_config_with_options(cb, data, &repo_config, 1);
- }
-}
-
const char *git_pager(int stdout_is_tty)
{
const char *pager;
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH/RFC 3/7] Mark builtins that create .git/ directories
From: Johannes Schindelin @ 2016-12-08 15:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <cover.1481211338.git.johannes.schindelin@gmx.de>
To refactor read_early_config() so that it discovers .git/config
properly, we have to make certain that commands such as `git init` (i.e.
commands that create their own .git/ and therefore do *not* want to
be affected by any other .git/ directory) skip this discovery.
Let's introduce a flag that states for every builtin whether it creates
its own .git/ directory or not.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
git.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/git.c b/git.c
index dce529fcbf..61df78afc8 100644
--- a/git.c
+++ b/git.c
@@ -318,12 +318,13 @@ static int handle_alias(int *argcp, const char ***argv)
#define RUN_SETUP (1<<0)
#define RUN_SETUP_GENTLY (1<<1)
#define USE_PAGER (1<<2)
+#define CREATES_GIT_DIR (1<<3)
/*
* require working tree to be present -- anything uses this needs
* RUN_SETUP for reading from the configuration file.
*/
-#define NEED_WORK_TREE (1<<3)
-#define SUPPORT_SUPER_PREFIX (1<<4)
+#define NEED_WORK_TREE (1<<4)
+#define SUPPORT_SUPER_PREFIX (1<<5)
struct cmd_struct {
const char *cmd;
@@ -412,7 +413,7 @@ static struct cmd_struct commands[] = {
{ "cherry", cmd_cherry, RUN_SETUP },
{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
- { "clone", cmd_clone },
+ { "clone", cmd_clone, CREATES_GIT_DIR },
{ "column", cmd_column, RUN_SETUP_GENTLY },
{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
{ "commit-tree", cmd_commit_tree, RUN_SETUP },
@@ -438,7 +439,7 @@ static struct cmd_struct commands[] = {
{ "hash-object", cmd_hash_object },
{ "help", cmd_help },
{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
- { "init", cmd_init_db },
+ { "init", cmd_init_db, CREATES_GIT_DIR },
{ "init-db", cmd_init_db },
{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
{ "log", cmd_log, RUN_SETUP },
--
2.11.0.rc3.windows.1
^ permalink raw reply related
* [PATCH/RFC 0/7] Pie-in-the-sky attempt to fix the early config
From: Johannes Schindelin @ 2016-12-08 15:35 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
Hopefully these patches will lead to something that we can integrate,
and that eventually will make Git's startup sequence much less
surprising.
The idea here is to discover the .git/ directory gently (i.e. without
changing the current working directory), and to use it to read the
.git/config file early, before we actually called setup_git_directory()
(if we ever do that).
Notes:
- I find the diff pretty ugly: I wish there was a more elegant way to
*disable* discovery of .git/ *just* for `init` and `clone`. I
considered a function `about_to_create_git_dir()` that is called in a
hard-coded manner *only* for `init` and `clone`, but that would
introduce another magic side effect, when all I want is to reduce those.
- For the moment, I do not handle dashed invocations of `init` and
`clone` correctly. The real problem is the continued existence of
the dashed git-init and git-clone, of course.
- There is still duplicated code. For the sake of this RFC, I did not
address that yet.
- The read_early_config() function is called multiple times, re-reading
all the config files and re-discovering the .git/ directory multiple
times, which is quite wasteful. For the sake of this RFC, I did not
address that yet.
- t7006 fails and the error message is a bit cryptic (not to mention the
involved function trace, which is mind-boggling for what is supposed
to be a simply, shell script-based test suite). I did not have time to
look into that yet.
- after discover_git_directory_gently() did its work, the code happily
uses its result *only* for the current read_early_config() run, and
lets setup_git_dir_gently() do the whole work *again*. For the sake of
this RFC, I did not address that yet.
Johannes Schindelin (7):
Make read_early_config() reusable
read_early_config(): avoid .git/config hack when unneeded
Mark builtins that create .git/ directories
read_early_config(): special-case `init` and `clone`
read_early_config(): really discover .git/
WIP read_config_early(): respect ceiling directories
WIP: read_early_config(): add tests
builtin/am.c | 2 +-
builtin/blame.c | 2 +-
builtin/grep.c | 4 +-
builtin/log.c | 4 +-
builtin/var.c | 2 +-
cache.h | 8 ++--
config.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++++
diff.c | 4 +-
git.c | 25 +++++------
pager.c | 44 +++----------------
t/helper/test-config.c | 15 +++++++
t/t1309-early-config.sh | 50 ++++++++++++++++++++++
12 files changed, 209 insertions(+), 61 deletions(-)
create mode 100755 t/t1309-early-config.sh
base-commit: 8d7a455ed52e2a96debc080dfc011b6bb00db5d2
Published-As: https://github.com/dscho/git/releases/tag/early-config-v1
Fetch-It-Via: git fetch https://github.com/dscho/git early-config-v1
--
2.11.0.rc3.windows.1
^ permalink raw reply
* Re: [PATCH 4/5] Make sequencer abort safer
From: Johannes Schindelin @ 2016-12-08 15:28 UTC (permalink / raw)
To: Stephan Beyer; +Cc: git, Christian Couder, Junio C Hamano, SZEDER Gábor
In-Reply-To: <20161207215133.13433-4-s-beyer@gmx.net>
Hi,
On Wed, 7 Dec 2016, Stephan Beyer wrote:
> diff --git a/sequencer.c b/sequencer.c
> index 30b10ba14..c9b560ac1 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -27,6 +27,7 @@ GIT_PATH_FUNC(git_path_seq_dir, "sequencer")
> static GIT_PATH_FUNC(git_path_todo_file, "sequencer/todo")
> static GIT_PATH_FUNC(git_path_opts_file, "sequencer/opts")
> static GIT_PATH_FUNC(git_path_head_file, "sequencer/head")
> +static GIT_PATH_FUNC(git_path_curr_file, "sequencer/current")
Is it required by law to have a four-letter infix, or can we have a nicer
variable name (e.g. git_path_current_file)?
Ciao,
Dscho
^ permalink raw reply
* [PATCHv2 6.5/7] squash! versioncmp: use earliest-longest contained suffix to determine sorting order
From: SZEDER Gábor @ 2016-12-08 14:48 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-7-szeder.dev@gmail.com>
As the number of identical steps to be done for both tagnames grows,
extract them into a helper function, with the additional benefit that
the conditionals near the end of swap_prereleases() will use more
meaningful variable names.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
I was not particularly happy with the amount of code duplication this
series added to swap_prereleases() in patches 5 and 6, hence this bit
of refactoring. Which I'm not particularly happy with either,
because, well, look at that diffstat.
I don't have strong feelings either way, but here it is, take it or
leave it.
versioncmp.c | 62 ++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 37 insertions(+), 25 deletions(-)
diff --git a/versioncmp.c b/versioncmp.c
index 62c8d69dc..601ceddef 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -24,6 +24,29 @@
static const struct string_list *prereleases;
static int initialized;
+struct suffix_match {
+ int conf_pos;
+ int start;
+ int len;
+};
+
+static void find_better_matching_suffix(const char *tagname, const char *suffix,
+ int suffix_len, int start, int conf_pos,
+ struct suffix_match *match)
+{
+ /* A better match either starts earlier or starts at the same offset
+ * but is longer. */
+ int end = match->len < suffix_len ? match->start : match->start-1;
+ int i;
+ for (i = start; i <= end; i++)
+ if (starts_with(tagname + i, suffix)) {
+ match->conf_pos = conf_pos;
+ match->start = i;
+ match->len = suffix_len;
+ break;
+ }
+}
+
/*
* off is the offset of the first different character in the two strings
* s1 and s2. If either s1 or s2 contains a prerelease suffix containing
@@ -47,46 +70,35 @@ static int swap_prereleases(const char *s1,
int off,
int *diff)
{
- int i, i1 = -1, i2 = -1;
- int start_at1 = off, start_at2 = off, match_len1 = -1, match_len2 = -1;
+ int i;
+ struct suffix_match match1 = { -1, off, -1 };
+ struct suffix_match match2 = { -1, off, -1 };
for (i = 0; i < prereleases->nr; i++) {
const char *suffix = prereleases->items[i].string;
- int j, start, end, suffix_len = strlen(suffix);
+ int start, suffix_len = strlen(suffix);
if (suffix_len < off)
start = off - suffix_len;
else
start = 0;
- end = match_len1 < suffix_len ? start_at1 : start_at1-1;
- for (j = start; j <= end; j++)
- if (starts_with(s1 + j, suffix)) {
- i1 = i;
- start_at1 = j;
- match_len1 = suffix_len;
- break;
- }
- end = match_len2 < suffix_len ? start_at2 : start_at2-1;
- for (j = start; j <= end; j++)
- if (starts_with(s2 + j, suffix)) {
- i2 = i;
- start_at2 = j;
- match_len2 = suffix_len;
- break;
- }
+ find_better_matching_suffix(s1, suffix, suffix_len, start,
+ i, &match1);
+ find_better_matching_suffix(s2, suffix, suffix_len, start,
+ i, &match2);
}
- if (i1 == -1 && i2 == -1)
+ if (match1.conf_pos == -1 && match2.conf_pos == -1)
return 0;
- if (i1 == i2)
+ if (match1.conf_pos == match2.conf_pos)
/* Found the same suffix in both, e.g. "-rc" in "v1.0-rcX"
* and "v1.0-rcY": the caller should decide based on "X"
* and "Y". */
return 0;
- if (i1 >= 0 && i2 >= 0)
- *diff = i1 - i2;
- else if (i1 >= 0)
+ if (match1.conf_pos >= 0 && match2.conf_pos >= 0)
+ *diff = match1.conf_pos - match2.conf_pos;
+ else if (match1.conf_pos >= 0)
*diff = -1;
- else /* if (i2 >= 0) */
+ else /* if (match2.conf_pos >= 0) */
*diff = 1;
return 1;
}
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* [REGRESSION 2.10.2] problematic "empty auth" changes
From: Johannes Schindelin @ 2016-12-08 14:47 UTC (permalink / raw)
To: David Turner; +Cc: git
Hi Dave,
I got a couple of bug reports that claim that 2.10.2 regressed on using
network credentials. That is, users regularly hit Enter twice when being
asked for user name and password while fetching via https://, and cURL
automatically used to fall back to using the login credentials (i.e.
authenticating via the Domain controller).
Turns out those claims are correct: hitting Enter twice (or using URLs
with empty user name/password such as https://:tfs:8080/) work in 2.10.1
and yield "Authentication failed" in 2.10.2.
I tracked this down to 5275c3081c (http: http.emptyauth should allow empty
(not just NULL) usernames, 2016-10-04) which all of a sudden disallowed
empty user names (and now only handles things correctly when
http.emptyAuth is set to true specifically).
This smells like a real bad regression to me, certainly given the time I
had to spend to figure this out (starting from not exactly helpful bug
reports, due to being very specific to their setups being private).
I am *really* tempted to change the default of http.emptyAuth to true, *at
least* for Windows (where it is quite common to use your login credentials
to authenticate to corporate servers).
Before I do anything rash, though: Do you see any downside to that?
Ciao,
Dscho
^ permalink raw reply
* Re: Serious bug with Git-2.11.0-64-bit and Git-LFS
From: Lars Schneider @ 2016-12-08 14:18 UTC (permalink / raw)
To: Nick Warr; +Cc: git
In-Reply-To: <CABW+60za0shXucPgg_jGYt4f8QbkLzLmS5GRf8czE67Taqd+zw@mail.gmail.com>
> On 08 Dec 2016, at 15:00, Nick Warr <nick.warr@bossastudios.com> wrote:
>
> That looks pretty much like the error we're dealing with, any reason
> why going back a point version on Git (not git-lfs) would resolve the
> issue however?
Going back to GitLFS 1.4.* would make the error disappear. However, I think
you should fix your repository. Try this:
git lfs clone <YOUR REPO URL>
cd <YOUR REPO>
git rm --cached "workers/unity/Assets/3rdPartyAssets/FORGE3D/Sci-Fi Effects/Effects/Debris/Meshes/debris_junk.FBX"
git add --force "workers/unity/Assets/3rdPartyAssets/FORGE3D/Sci-Fi Effects/Effects/Debris/Meshes/debris_junk.FBX"
git commit -m "Move files properly to GitLFS"
git push
Afterwards you should be able to use the latest version of Git and GitLFS
without trouble.
Cheers,
Lars
PS: Top posting is not that popular in the Git community ;-)
>
> On 8 December 2016 at 13:57, Lars Schneider <larsxschneider@gmail.com> wrote:
>>
>>> On 08 Dec 2016, at 12:46, Nick Warr <nick.warr@bossastudios.com> wrote:
>>>
>>> Using Git-2.11.0 with the latest git-lfs 1.5.2 (also tested with
>>> 1.5.3) cloning from our locally hosted gitlab CE server via HTTPS.
>>>
>>> When cloning a repo (large, 3.3 gig in git, 10.3 in LFS) for the
>>> first time the clone will finish the checkout of the git part, then
>>> when it starts downloading the LFS parts it will reliably finish with
>>> a smudge filter error.
>>>
>>> This leaves the repo in an unstable condition, you can then fetch the
>>> lfs part without issue, but checking out the lfs files or trying a git
>>> reset --hard will continue to spit out the same error. As you can see,
>>> the actual error is not particularly useful.
>>>
>>> fatal: workers/unity/Assets/3rdPartyAssets/FORGE3D/Sci-Fi
>>> Effects/Effects/Debris/Meshes/debris_junk.FBX: smudge filter lfs
>>> failed
>>> Unknown command ""
>>>
>>> Possibly it's due to the file extension being all capital letters, we
>>> did manage to change the error by recommitting the file with a
>>> lowercase extension, but it failed on the next file (which also had a
>>> capital letter extension).
>>>
>>> This has happened on multiple fresh windows 10 64 bit installs,
>>> different machines and target directories (to hopefully remove the
>>> possibility of file permissions) where cloning is taking place.
>>>
>>> The solution is to back level to Git 2.10.2 and the error disappears.
>>>
>>> More than willing to provide any further information,
>>
>> Hi Nick,
>>
>> debris_junk.FBX is not stored properly in Git LFS.
>> I explained the problem in detail here:
>> https://github.com/git-lfs/git-lfs/issues/1729
>>
>> You should add the file properly to GitLFS to fix the problem.
>> However, I think this is a regression in GitLFS and I hope it will
>> be fixed in the next version.
>>
>> No change/fix in Git is required.
>>
>> Cheers,
>> Lars
^ permalink raw reply
* [PATCHv2 4/7] versioncmp: pass full tagnames to swap_prereleases()
From: SZEDER Gábor @ 2016-12-08 14:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
The swap_prereleases() helper function is responsible for finding
configured prerelease suffixes in a pair of tagnames to be compared,
but this function currently gets to see only the parts of those two
tagnames starting at the first different character. To fix some
issues related to the common part of two tagnames overlapping with
leading part of a prerelease suffix, this helper function must see
both full tagnames.
In preparation for the fix in the following patch, refactor
swap_prereleases() and its caller to pass two full tagnames and an
additional offset indicating the position of the first different
character.
While updating the comment describing that function, remove the
sentence about not dealing with both tagnames having the same suffix.
Currently it doesn't add much value (we know that there is a different
character, so it's obvious that it can't possibly be the same suffix
in both), and at the end of this patch series it won't even be true
anymore.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
versioncmp.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/versioncmp.c b/versioncmp.c
index 80bfd109f..a55c23ad5 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -25,32 +25,28 @@ static const struct string_list *prereleases;
static int initialized;
/*
- * p1 and p2 point to the first different character in two strings. If
- * either p1 or p2 starts with a prerelease suffix, it will be forced
- * to be on top.
+ * off is the offset of the first different character in the two strings
+ * s1 and s2. If either s1 or s2 contains a prerelease suffix starting
+ * at that offset, it will be forced to be on top.
*
- * If both p1 and p2 start with (different) suffix, the order is
- * determined by config file.
- *
- * Note that we don't have to deal with the situation when both p1 and
- * p2 start with the same suffix because the common part is already
- * consumed by the caller.
+ * If both s1 and s2 contain a (different) suffix at that position,
+ * their order is determined by the order of those two suffixes in the
+ * configuration.
*
* Return non-zero if *diff contains the return value for versioncmp()
*/
-static int swap_prereleases(const void *p1_,
- const void *p2_,
+static int swap_prereleases(const char *s1,
+ const char *s2,
+ int off,
int *diff)
{
- const char *p1 = p1_;
- const char *p2 = p2_;
int i, i1 = -1, i2 = -1;
for (i = 0; i < prereleases->nr; i++) {
const char *suffix = prereleases->items[i].string;
- if (i1 == -1 && starts_with(p1, suffix))
+ if (i1 == -1 && starts_with(s1 + off, suffix))
i1 = i;
- if (i2 == -1 && starts_with(p2, suffix))
+ if (i2 == -1 && starts_with(s2 + off, suffix))
i2 = i;
}
if (i1 == -1 && i2 == -1)
@@ -121,7 +117,8 @@ int versioncmp(const char *s1, const char *s2)
initialized = 1;
prereleases = git_config_get_value_multi("versionsort.prereleasesuffix");
}
- if (prereleases && swap_prereleases(p1 - 1, p2 - 1, &diff))
+ if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1,
+ &diff))
return diff;
state = result_type[state * 3 + (((c2 == '0') + (isdigit (c2) != 0)))];
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* [PATCHv2 0/7] Fix and generalize version sort reordering
From: SZEDER Gábor @ 2016-12-08 14:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161005033353.Horde.33pf2naqnF4HgwPWSy9DaHV@webmail.informatik.kit.edu>
> After having slept on this a couple of times
After having slept on it a few (erm...) more times...
> I think the longest
> matching prerelease suffix should determine the sorting order.
>
> A release tag usually consists of an optional prefix, e.g. 'v' or
> 'snapshot-', followed by the actual version number, followed by an
> optional suffix. In the contrived example quoted above this suffix
> is '-foo-bar', thus it feels wrong to match '-bar' against it, when
> the user explicitly configured '-foo-bar' as prerelease suffix as
> well.
At the risk of overthinking it: I think it would be more correct to
say that the earliest starting matching prerelease suffix should
determine the sorting order in the first place.
> Then here is a more realistic case for sorting based on the longest
> matching suffix, where
>
> - a longer suffix starts with the shorter one,
> - and the longer suffix comes after the shorter one in the
> configuration.
>
> With my patches it looks like this:
>
> $ git -c versionsort.prereleasesuffix=-pre \
> -c versionsort.prereleasesuffix=-prerelease \
> tag -l --sort=version:refname
> v1.0.0-prerelease1
> v1.0.0-pre1
> v1.0.0-pre2
> v1.0.0
And when there happen to be more than one matching suffixes starting
at the same earliest position, then we should pick the longest of
them. The new patch 6/7 implements this behavior.
Changes since v1:
- Documentation, in-code comment and commit message updates.
- Use different tagnames and suffixes in the tests, keeping the new
tests simpler and changes to existing tests smaller.
- Added a test of questionable value to try to check that we don't
read memory before the tagname strings in case of an unusually long
suffix.
- Removed unnecessary {}.
- Added two patches on top to address the corner cases and to
introduce 'versionsort.suffix' for the resulting more general
suffix reordering behavior.
The interdiff at the bottom is between v1 and only the first five
patches of v2, i.e. not including the two new patches. I think it's
easier to judge the changes this way.
SZEDER Gábor (7):
t7004-tag: delete unnecessary tags with test_when_finished
t7004-tag: use test_config helper
t7004-tag: add version sort tests to show prerelease reordering issues
versioncmp: pass full tagnames to swap_prereleases()
versioncmp: cope with common part overlapping with prerelease suffix
versioncmp: use earliest-longest contained suffix to determine sorting
order
versioncmp: generalize version sort suffix reordering
Documentation/config.txt | 44 ++++++++++++----
Documentation/git-tag.txt | 4 +-
t/t7004-tag.sh | 132 +++++++++++++++++++++++++++++++++++++++-------
versioncmp.c | 68 +++++++++++++++++-------
4 files changed, 196 insertions(+), 52 deletions(-)
--
2.11.0.78.g5a2d011
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb6790d..c1a9616e9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3008,8 +3008,12 @@ versionsort.prereleaseSuffix::
This variable can be specified multiple times, once per suffix. The
order of suffixes in the config file determines the sorting order
(e.g. if "-pre" appears before "-rc" in the config file then 1.0-preXX
-is sorted before 1.0-rcXX). The sorting order between different
-suffixes is undefined if they are in multiple config files.
+is sorted before 1.0-rcXX).
+If more than one suffixes match the same tagname, then that tagname will
+be sorted according to the matching suffix which comes first in the
+configuration.
+The sorting order between different suffixes is undefined if they are
+in multiple config files.
web.browser::
Specify a web browser that may be used by some commands.
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index f0cfe1fa3..c7aaace8c 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1511,14 +1511,14 @@ test_expect_success 'invalid sort parameter in configuratoin' '
'
test_expect_success 'version sort with prerelease reordering' '
- test_config versionsort.prereleaseSuffix -beta &&
- git tag foo1.6-beta1 &&
- git tag foo1.6-beta2 &&
+ test_config versionsort.prereleaseSuffix -rc &&
+ git tag foo1.6-rc1 &&
+ git tag foo1.6-rc2 &&
git tag -l --sort=version:refname "foo*" >actual &&
cat >expect <<-\EOF &&
foo1.3
- foo1.6-beta1
- foo1.6-beta2
+ foo1.6-rc1
+ foo1.6-rc2
foo1.6
foo1.10
EOF
@@ -1526,54 +1526,49 @@ test_expect_success 'version sort with prerelease reordering' '
'
test_expect_success 'reverse version sort with prerelease reordering' '
- test_config versionsort.prereleaseSuffix -beta &&
+ test_config versionsort.prereleaseSuffix -rc &&
git tag -l --sort=-version:refname "foo*" >actual &&
cat >expect <<-\EOF &&
foo1.10
foo1.6
- foo1.6-beta2
- foo1.6-beta1
+ foo1.6-rc2
+ foo1.6-rc1
foo1.3
EOF
test_cmp expect actual
'
test_expect_success 'version sort with prerelease reordering and common leading character' '
- test_config versionsort.prereleaseSuffix -beta &&
- git tag foo1.6-after1 &&
- git tag -l --sort=version:refname "foo*" >actual &&
+ test_config versionsort.prereleaseSuffix -before &&
+ git tag foo1.7-before1 &&
+ git tag foo1.7 &&
+ git tag foo1.7-after1 &&
+ git tag -l --sort=version:refname "foo1.7*" >actual &&
cat >expect <<-\EOF &&
- foo1.3
- foo1.6-beta1
- foo1.6-beta2
- foo1.6
- foo1.6-after1
- foo1.10
+ foo1.7-before1
+ foo1.7
+ foo1.7-after1
EOF
test_cmp expect actual
'
-# Capitalization of suffixes is important here, because "-RC" would normally
-# be sorted before "-beta" and the config settings should override that.
test_expect_success 'version sort with prerelease reordering, multiple suffixes and common leading character' '
- test_config versionsort.prereleaseSuffix -beta &&
- git config --add versionsort.prereleaseSuffix -RC &&
- git tag foo1.6-RC1 &&
- git tag foo1.6-RC2 &&
- git tag -l --sort=version:refname "foo*" >actual &&
+ test_config versionsort.prereleaseSuffix -before &&
+ git config --add versionsort.prereleaseSuffix -after &&
+ git tag -l --sort=version:refname "foo1.7*" >actual &&
cat >expect <<-\EOF &&
- foo1.3
- foo1.6-beta1
- foo1.6-beta2
- foo1.6-RC1
- foo1.6-RC2
- foo1.6
- foo1.6-after1
- foo1.10
+ foo1.7-before1
+ foo1.7-after1
+ foo1.7
EOF
test_cmp expect actual
'
+test_expect_success 'version sort with very long prerelease suffix' '
+ test_config versionsort.prereleaseSuffix -very-looooooooooooooooooooooooong-prerelease-suffix &&
+ git tag -l --sort=version:refname
+'
+
run_with_limited_stack () {
(ulimit -s 128 && "$@")
}
diff --git a/versioncmp.c b/versioncmp.c
index 87b49a622..f86ac562e 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -26,16 +26,15 @@ static int initialized;
/*
* off is the offset of the first different character in the two strings
- * s1 and s2. If either s1 or s2 contains a prerelease suffix starting
- * at that offset or the character at that offset is part of a
- * prerelease suffix, then that string will be forced to be on top.
+ * s1 and s2. If either s1 or s2 contains a prerelease suffix containing
+ * that offset, then that string will be forced to be on top.
*
- * If both s1 and s2 contain a (different) suffix at that position, the
- * order is determined by config file.
- *
- * Note that we don't have to deal with the situation when both s1 and
- * s2 contain the same suffix because the common part is already
- * consumed by the caller.
+ * If both s1 and s2 contain a (different) suffix around that position,
+ * their order is determined by the order of those two suffixes in the
+ * configuration.
+ * If any of the strings contains more than one different suffixes around
+ * that position, then that string is sorted according to the contained
+ * suffix which comes first in the configuration.
*
* Return non-zero if *diff contains the return value for versioncmp()
*/
@@ -53,18 +52,16 @@ static int swap_prereleases(const char *s1,
start = off - suffix_len + 1;
else
start = 0;
- for (j = start; j <= off; j++) {
+ for (j = start; j <= off; j++)
if (i1 == -1 && starts_with(s1 + j, suffix)) {
i1 = i;
break;
}
- }
- for (j = start; j <= off; j++) {
+ for (j = start; j <= off; j++)
if (i2 == -1 && starts_with(s2 + j, suffix)) {
i2 = i;
break;
}
- }
}
if (i1 == -1 && i2 == -1)
return 0;
^ permalink raw reply related
* [PATCHv2 2/7] t7004-tag: use test_config helper
From: SZEDER Gábor @ 2016-12-08 14:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
... instead of setting and then manually unsetting configuration
variables, on one occasion even outside the test_expect_success block.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/t7004-tag.sh | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 396cffeeb..920a1b4b2 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -297,11 +297,9 @@ EOF
'
test_expect_success 'listing tags in column with column.*' '
- git config column.tag row &&
- git config column.ui dense &&
+ test_config column.tag row &&
+ test_config column.ui dense &&
COLUMNS=40 git tag -l >actual &&
- git config --unset column.ui &&
- git config --unset column.tag &&
cat >expected <<\EOF &&
a1 aa1 cba t210 t211
v0.2.1 v1.0 v1.0.1 v1.1.3
@@ -314,9 +312,8 @@ test_expect_success 'listing tag with -n --column should fail' '
'
test_expect_success 'listing tags -n in column with column.ui ignored' '
- git config column.ui "row dense" &&
+ test_config column.ui "row dense" &&
COLUMNS=40 git tag -l -n >actual &&
- git config --unset column.ui &&
cat >expected <<\EOF &&
a1 Foo
aa1 Foo
@@ -1200,11 +1197,10 @@ test_expect_success GPG,RFC1991 \
'
# try to sign with bad user.signingkey
-git config user.signingkey BobTheMouse
test_expect_success GPG \
'git tag -s fails if gpg is misconfigured (bad key)' \
- 'test_must_fail git tag -s -m tail tag-gpg-failure'
-git config --unset user.signingkey
+ 'test_config user.signingkey BobTheMouse &&
+ test_must_fail git tag -s -m tail tag-gpg-failure'
# try to produce invalid signature
test_expect_success GPG \
@@ -1484,7 +1480,7 @@ test_expect_success 'reverse lexical sort' '
'
test_expect_success 'configured lexical sort' '
- git config tag.sort "v:refname" &&
+ test_config tag.sort "v:refname" &&
git tag -l "foo*" >actual &&
cat >expect <<-\EOF &&
foo1.3
@@ -1495,6 +1491,7 @@ test_expect_success 'configured lexical sort' '
'
test_expect_success 'option override configured sort' '
+ test_config tag.sort "v:refname" &&
git tag -l --sort=-refname "foo*" >actual &&
cat >expect <<-\EOF &&
foo1.6
@@ -1509,13 +1506,12 @@ test_expect_success 'invalid sort parameter on command line' '
'
test_expect_success 'invalid sort parameter in configuratoin' '
- git config tag.sort "v:notvalid" &&
+ test_config tag.sort "v:notvalid" &&
test_must_fail git tag -l "foo*"
'
test_expect_success 'version sort with prerelease reordering' '
- git config --unset tag.sort &&
- git config versionsort.prereleaseSuffix -rc &&
+ test_config versionsort.prereleaseSuffix -rc &&
git tag foo1.6-rc1 &&
git tag foo1.6-rc2 &&
git tag -l --sort=version:refname "foo*" >actual &&
@@ -1530,6 +1526,7 @@ test_expect_success 'version sort with prerelease reordering' '
'
test_expect_success 'reverse version sort with prerelease reordering' '
+ test_config versionsort.prereleaseSuffix -rc &&
git tag -l --sort=-version:refname "foo*" >actual &&
cat >expect <<-\EOF &&
foo1.10
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* [PATCHv2 6/7] versioncmp: use earliest-longest contained suffix to determine sorting order
From: SZEDER Gábor @ 2016-12-08 14:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
When comparing tagnames, it is possible that a tagname contains more
than one of the configured prerelease suffixes around the first
different character. After fixing a bug in the previous commit such a
tagname is sorted according to the contained suffix which comes first
in the configuration. This is, however, not quite the right thing to
do in the following corner cases:
1. $ git -c versionsort.suffix=-bar
-c versionsort.suffix=-foo-baz
-c versionsort.suffix=-foo-bar
tag -l --sort=version:refname 'v1*'
v1.0-foo-bar
v1.0-foo-baz
The suffix of the tagname 'v1.0-foo-bar' is clearly '-foo-bar',
so it should be listed last. However, as it also contains '-bar'
around the first different character, it is listed first instead,
because that '-bar' suffix comes first the configuration.
2. One of the configured suffixes starts with the other:
$ git -c versionsort.prereleasesuffix=-pre \
-c versionsort.prereleasesuffix=-prerelease \
tag -l --sort=version:refname 'v2*'
v2.0-prerelease1
v2.0-pre1
v2.0-pre2
Here the tagname 'v2.0-prerelease1' should be the last. When
comparing 'v2.0-pre1' and 'v2.0-prerelease1' the first different
characters are '1' and 'r', respectively. Since this first
different character must be part of the configured suffix, the
'-pre' suffix is not recognized in the first tagname. OTOH, the
'-prerelease' suffix is properly recognized in
'v2.0-prerelease1', thus it is listed first.
Improve version sort in these corner cases, and
- look for a configured prerelease suffix containing the first
different character or ending right before it, so the '-pre'
suffixes are recognized in case (2). This also means that
when comparing tagnames 'v2.0-pre1' and 'v2.0-pre2',
swap_prereleases() would find the '-pre' suffix in both, but then
it will return "undecided" and the caller will do the right thing
by sorting based in '1' and '2'.
- If the tagname contains more than one suffix, then give precedence
to the contained suffix that starts at the earliest offset in the
tagname to address (1).
- If there are more than one suffixes starting at that earliest
position, then give precedence to the longest of those suffixes,
thus ensuring that in (2) the tagname 'v2.0-prerelease1' won't be
sorted based on the '-pre' suffix.
Add tests for these corner cases and adjust the documentation
accordingly.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
Documentation/config.txt | 6 ++++--
t/t7004-tag.sh | 31 +++++++++++++++++++++++++++++++
versioncmp.c | 33 +++++++++++++++++++++++++--------
3 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1a9616e9..58009c70c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3010,8 +3010,10 @@ order of suffixes in the config file determines the sorting order
(e.g. if "-pre" appears before "-rc" in the config file then 1.0-preXX
is sorted before 1.0-rcXX).
If more than one suffixes match the same tagname, then that tagname will
-be sorted according to the matching suffix which comes first in the
-configuration.
+be sorted according to the suffix which starts at the earliest position in
+the tagname. If more than one different matching suffixes start at
+that earliest position, then that tagname will be sorted according to the
+longest of those suffixes.
The sorting order between different suffixes is undefined if they are
in multiple config files.
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index c7aaace8c..e2efe312d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1564,6 +1564,37 @@ test_expect_success 'version sort with prerelease reordering, multiple suffixes
test_cmp expect actual
'
+test_expect_success 'version sort with prerelease reordering, multiple suffixes match the same tag' '
+ test_config versionsort.prereleaseSuffix -bar &&
+ git config --add versionsort.prereleaseSuffix -foo-baz &&
+ git config --add versionsort.prereleaseSuffix -foo-bar &&
+ git tag foo1.8-foo-bar &&
+ git tag foo1.8-foo-baz &&
+ git tag foo1.8 &&
+ git tag -l --sort=version:refname "foo1.8*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.8-foo-baz
+ foo1.8-foo-bar
+ foo1.8
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'version sort with prerelease reordering, multiple suffixes match starting at the same position' '
+ test_config versionsort.prereleaseSuffix -pre &&
+ git config --add versionsort.prereleaseSuffix -prerelease &&
+ git tag foo1.9-pre1 &&
+ git tag foo1.9-pre2 &&
+ git tag foo1.9-prerelease1 &&
+ git tag -l --sort=version:refname "foo1.9*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.9-pre1
+ foo1.9-pre2
+ foo1.9-prerelease1
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success 'version sort with very long prerelease suffix' '
test_config versionsort.prereleaseSuffix -very-looooooooooooooooooooooooong-prerelease-suffix &&
git tag -l --sort=version:refname
diff --git a/versioncmp.c b/versioncmp.c
index f86ac562e..ae0a9199b 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -27,14 +27,18 @@ static int initialized;
/*
* off is the offset of the first different character in the two strings
* s1 and s2. If either s1 or s2 contains a prerelease suffix containing
- * that offset, then that string will be forced to be on top.
+ * that offset or a suffix ends right before that offset, then that
+ * string will be forced to be on top.
*
* If both s1 and s2 contain a (different) suffix around that position,
* their order is determined by the order of those two suffixes in the
* configuration.
* If any of the strings contains more than one different suffixes around
* that position, then that string is sorted according to the contained
- * suffix which comes first in the configuration.
+ * suffix which starts at the earliest offset in that string.
+ * If more than one different contained suffixes start at that earliest
+ * offset, then that string is sorted according to the longest of those
+ * suffixes.
*
* Return non-zero if *diff contains the return value for versioncmp()
*/
@@ -44,27 +48,40 @@ static int swap_prereleases(const char *s1,
int *diff)
{
int i, i1 = -1, i2 = -1;
+ int start_at1 = off, start_at2 = off, match_len1 = -1, match_len2 = -1;
for (i = 0; i < prereleases->nr; i++) {
const char *suffix = prereleases->items[i].string;
- int j, start, suffix_len = strlen(suffix);
+ int j, start, end, suffix_len = strlen(suffix);
if (suffix_len < off)
- start = off - suffix_len + 1;
+ start = off - suffix_len;
else
start = 0;
- for (j = start; j <= off; j++)
- if (i1 == -1 && starts_with(s1 + j, suffix)) {
+ end = match_len1 < suffix_len ? start_at1 : start_at1-1;
+ for (j = start; j <= end; j++)
+ if (starts_with(s1 + j, suffix)) {
i1 = i;
+ start_at1 = j;
+ match_len1 = suffix_len;
break;
}
- for (j = start; j <= off; j++)
- if (i2 == -1 && starts_with(s2 + j, suffix)) {
+ end = match_len2 < suffix_len ? start_at2 : start_at2-1;
+ for (j = start; j <= end; j++)
+ if (starts_with(s2 + j, suffix)) {
i2 = i;
+ start_at2 = j;
+ match_len2 = suffix_len;
break;
}
}
if (i1 == -1 && i2 == -1)
return 0;
+ if (i1 == i2)
+ /* Found the same suffix in both, e.g. "-rc" in "v1.0-rcX"
+ * and "v1.0-rcY": the caller should decide based on "X"
+ * and "Y". */
+ return 0;
+
if (i1 >= 0 && i2 >= 0)
*diff = i1 - i2;
else if (i1 >= 0)
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* [PATCHv2 7/7] versioncmp: generalize version sort suffix reordering
From: SZEDER Gábor @ 2016-12-08 14:24 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
The 'versionsort.prereleaseSuffix' configuration variable, as its name
suggests, is supposed to only deal with tagnames with prerelease
suffixes, and allows sorting those prerelease tags in a user-defined
order before the suffixless main release tag, instead of sorting them
simply lexicographically.
However, the previous changes in this series resulted in an
interesting and useful property of version sort:
- The empty string as a configured suffix matches all tagnames,
including tagnames without any suffix, but
- tagnames containing a "real" configured suffix are still ordered
according to that real suffix, because any longer suffix takes
precedence over the empty string.
Exploiting this property we can easily generalize suffix reordering
and specify the order of tags with given suffixes not only before but
even after a main release tag by using the empty suffix to denote the
position of the main release tag, without any algorithm changes:
$ git -c versionsort.prereleaseSuffix=-alpha \
-c versionsort.prereleaseSuffix=-beta \
-c versionsort.prereleaseSuffix="" \
-c versionsort.prereleaseSuffix=-gamma \
-c versionsort.prereleaseSuffix=-delta \
tag -l --sort=version:refname 'v3.0*'
v3.0-alpha1
v3.0-beta1
v3.0
v3.0-gamma1
v3.0-delta1
Since 'versionsort.prereleaseSuffix' is not a fitting name for a
configuration variable to control this more general suffix reordering,
introduce the new variable 'versionsort.suffix'. Still keep the old
configuration variable name as a deprecated alias, though, to avoid
suddenly breaking setups already using it. Ignore the old variable if
both old and new configuration variables are set, but emit a warning
so users will be aware of it and can fix their configuration. Extend
the documentation to describe and add a test to check this more
general behavior.
Note: since the empty suffix matches all tagnames, tagnames with
suffixes not included in the configuration are listed together with
the suffixless main release tag, ordered lexicographically right after
that, i.e. before tags with suffixes listed in the configuration
following the empty suffix.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
Documentation/config.txt | 36 ++++++++++++++++++++++++++----------
Documentation/git-tag.txt | 4 ++--
t/t7004-tag.sh | 35 +++++++++++++++++++++++++++++++++++
versioncmp.c | 9 ++++++++-
4 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 58009c70c..ae85d4b9a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2999,16 +2999,32 @@ user.signingKey::
This option is passed unchanged to gpg's --local-user parameter,
so you may specify a key using any method that gpg supports.
-versionsort.prereleaseSuffix::
- When version sort is used in linkgit:git-tag[1], prerelease
- tags (e.g. "1.0-rc1") may appear after the main release
- "1.0". By specifying the suffix "-rc" in this variable,
- "1.0-rc1" will appear before "1.0".
-+
-This variable can be specified multiple times, once per suffix. The
-order of suffixes in the config file determines the sorting order
-(e.g. if "-pre" appears before "-rc" in the config file then 1.0-preXX
-is sorted before 1.0-rcXX).
+versionsort.prereleaseSuffix (deprecated)::
+ Deprecated alias for `versionsort.suffix`. Ignored if
+ `versionsort.suffix` is set.
+
+versionsort.suffix::
+ Even when version sort is used in linkgit:git-tag[1], tagnames
+ with the same base version but different suffixes are still sorted
+ lexicographically, resulting e.g. in prerelease tags appearing
+ after the main release (e.g. "1.0-rc1" after "1.0"). This
+ variable can be specified to determine the sorting order of tags
+ with different suffixes.
++
+By specifying a single suffix in this variable, any tagname containing
+that suffix will appear before the corresponding main release. E.g. if
+the variable is set to "-rc", then all "1.0-rcX" tags will appear before
+"1.0". If specified multiple times, once per suffix, then the order of
+suffixes in the configuration will determine the sorting order of tagnames
+with those suffixes. E.g. if "-pre" appears before "-rc" in the
+configuration, then all "1.0-preX" tags will be listed before any
+"1.0-rcX" tags. The placement of the main release tag relative to tags
+with various suffixes can be determined by specifying the empty suffix
+among those other suffixes. E.g. if the suffixes "-rc", "", "-ck" and
+"-bfs" appear in the configuration in this order, then all "v4.8-rcX" tags
+are listed first, followed by "v4.8", then "v4.8-ckX" and finally
+"v4.8-bfsX".
++
If more than one suffixes match the same tagname, then that tagname will
be sorted according to the suffix which starts at the earliest position in
the tagname. If more than one different matching suffixes start at
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 7ecca8e24..19990850d 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -101,8 +101,8 @@ OPTIONS
multiple times, in which case the last key becomes the primary
key. Also supports "version:refname" or "v:refname" (tag
names are treated as versions). The "version:refname" sort
- order can also be affected by the
- "versionsort.prereleaseSuffix" configuration variable.
+ order can also be affected by the "versionsort.suffix"
+ configuration variable.
The keys supported are the same as those in `git for-each-ref`.
Sort order defaults to the value configured for the `tag.sort`
variable if it exists, or lexicographic order otherwise. See
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e2efe312d..bdd28dad1 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1595,6 +1595,41 @@ test_expect_success 'version sort with prerelease reordering, multiple suffixes
test_cmp expect actual
'
+test_expect_success 'version sort with general suffix reordering' '
+ test_config versionsort.suffix -alpha &&
+ git config --add versionsort.suffix -beta &&
+ git config --add versionsort.suffix "" &&
+ git config --add versionsort.suffix -gamma &&
+ git config --add versionsort.suffix -delta &&
+ git tag foo1.10-alpha &&
+ git tag foo1.10-beta &&
+ git tag foo1.10-gamma &&
+ git tag foo1.10-delta &&
+ git tag foo1.10-unlisted-suffix &&
+ git tag -l --sort=version:refname "foo1.10*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.10-alpha
+ foo1.10-beta
+ foo1.10
+ foo1.10-unlisted-suffix
+ foo1.10-gamma
+ foo1.10-delta
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'versionsort.suffix overrides versionsort.prereleaseSuffix' '
+ test_config versionsort.suffix -before &&
+ test_config versionsort.prereleaseSuffix -after &&
+ git tag -l --sort=version:refname "foo1.7*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.7-before1
+ foo1.7
+ foo1.7-after1
+ EOF
+ test_cmp expect actual
+'
+
test_expect_success 'version sort with very long prerelease suffix' '
test_config versionsort.prereleaseSuffix -very-looooooooooooooooooooooooong-prerelease-suffix &&
git tag -l --sort=version:refname
diff --git a/versioncmp.c b/versioncmp.c
index ae0a9199b..62c8d69dc 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -145,8 +145,15 @@ int versioncmp(const char *s1, const char *s2)
}
if (!initialized) {
+ const struct string_list *deprecated_prereleases;
initialized = 1;
- prereleases = git_config_get_value_multi("versionsort.prereleasesuffix");
+ prereleases = git_config_get_value_multi("versionsort.suffix");
+ deprecated_prereleases = git_config_get_value_multi("versionsort.prereleasesuffix");
+ if (prereleases) {
+ if (deprecated_prereleases)
+ warning("ignoring versionsort.prereleasesuffix because versionsort.suffix is set");
+ } else
+ prereleases = deprecated_prereleases;
}
if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1,
&diff))
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* [PATCHv2 5/7] versioncmp: cope with common part overlapping with prerelease suffix
From: SZEDER Gábor @ 2016-12-08 14:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
Version sort with prerelease reordering sometimes puts tagnames in the
wrong order, when the common part of two compared tagnames overlaps
with the leading character(s) of one or more configured prerelease
suffixes. Note the position of "v2.1.0-beta-1":
$ git -c versionsort.prereleaseSuffix=-beta \
tag -l --sort=version:refname v2.1.*
v2.1.0-beta-2
v2.1.0-beta-3
v2.1.0
v2.1.0-RC1
v2.1.0-RC2
v2.1.0-beta-1
v2.1.1
v2.1.2
The reason is that when comparing a pair of tagnames, first
versioncmp() looks for the first different character in a pair of
tagnames, and then the swap_prereleases() helper function looks for a
configured prerelease suffix _starting at_ that character. Thus, when
in the above example the sorting algorithm happens to compare the
tagnames "v2.1.0-beta-1" and "v2.1.0-RC2", swap_prereleases() tries to
match the suffix "-beta" against "beta-1" to no avail, and the two
tagnames erroneously end up being ordered lexicographically.
To fix this issue change swap_prereleases() to look for configured
prerelease suffixes _containing_ the position of that first different
character.
Care must be taken, when a configured suffix is longer than the
tagnames' common part up to the first different character, to avoid
reading memory before the beginning of the tagnames. Add a test that
uses an exceptionally long prerelease suffix to check for this, in the
hope that in case of a regression the illegal memory access causes a
segfault in 'git tag' on one of the commonly used platforms (the test
happens to pass successfully on my Linux system with the safety check
removed), or at least makes valgrind complain.
Under some circumstances it's possible that more than one prerelease
suffixes can be found in the same tagname around that first different
character. With this simple bugfix patch such a tagname is sorted
according to the contained suffix that comes first in the
configuration for now. This is less than ideal in some cases, and the
following patch will take care of those.
Reported-by: Leho Kraav <leho@conversionready.com>
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
Documentation/config.txt | 8 ++++++--
t/t7004-tag.sh | 9 +++++++--
versioncmp.c | 28 +++++++++++++++++++++-------
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb6790d..c1a9616e9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3008,8 +3008,12 @@ versionsort.prereleaseSuffix::
This variable can be specified multiple times, once per suffix. The
order of suffixes in the config file determines the sorting order
(e.g. if "-pre" appears before "-rc" in the config file then 1.0-preXX
-is sorted before 1.0-rcXX). The sorting order between different
-suffixes is undefined if they are in multiple config files.
+is sorted before 1.0-rcXX).
+If more than one suffixes match the same tagname, then that tagname will
+be sorted according to the matching suffix which comes first in the
+configuration.
+The sorting order between different suffixes is undefined if they are
+in multiple config files.
web.browser::
Specify a web browser that may be used by some commands.
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 6445aae29..c7aaace8c 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1538,7 +1538,7 @@ test_expect_success 'reverse version sort with prerelease reordering' '
test_cmp expect actual
'
-test_expect_failure 'version sort with prerelease reordering and common leading character' '
+test_expect_success 'version sort with prerelease reordering and common leading character' '
test_config versionsort.prereleaseSuffix -before &&
git tag foo1.7-before1 &&
git tag foo1.7 &&
@@ -1552,7 +1552,7 @@ test_expect_failure 'version sort with prerelease reordering and common leading
test_cmp expect actual
'
-test_expect_failure 'version sort with prerelease reordering, multiple suffixes and common leading character' '
+test_expect_success 'version sort with prerelease reordering, multiple suffixes and common leading character' '
test_config versionsort.prereleaseSuffix -before &&
git config --add versionsort.prereleaseSuffix -after &&
git tag -l --sort=version:refname "foo1.7*" >actual &&
@@ -1564,6 +1564,11 @@ test_expect_failure 'version sort with prerelease reordering, multiple suffixes
test_cmp expect actual
'
+test_expect_success 'version sort with very long prerelease suffix' '
+ test_config versionsort.prereleaseSuffix -very-looooooooooooooooooooooooong-prerelease-suffix &&
+ git tag -l --sort=version:refname
+'
+
run_with_limited_stack () {
(ulimit -s 128 && "$@")
}
diff --git a/versioncmp.c b/versioncmp.c
index a55c23ad5..f86ac562e 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -26,12 +26,15 @@ static int initialized;
/*
* off is the offset of the first different character in the two strings
- * s1 and s2. If either s1 or s2 contains a prerelease suffix starting
- * at that offset, it will be forced to be on top.
+ * s1 and s2. If either s1 or s2 contains a prerelease suffix containing
+ * that offset, then that string will be forced to be on top.
*
- * If both s1 and s2 contain a (different) suffix at that position,
+ * If both s1 and s2 contain a (different) suffix around that position,
* their order is determined by the order of those two suffixes in the
* configuration.
+ * If any of the strings contains more than one different suffixes around
+ * that position, then that string is sorted according to the contained
+ * suffix which comes first in the configuration.
*
* Return non-zero if *diff contains the return value for versioncmp()
*/
@@ -44,10 +47,21 @@ static int swap_prereleases(const char *s1,
for (i = 0; i < prereleases->nr; i++) {
const char *suffix = prereleases->items[i].string;
- if (i1 == -1 && starts_with(s1 + off, suffix))
- i1 = i;
- if (i2 == -1 && starts_with(s2 + off, suffix))
- i2 = i;
+ int j, start, suffix_len = strlen(suffix);
+ if (suffix_len < off)
+ start = off - suffix_len + 1;
+ else
+ start = 0;
+ for (j = start; j <= off; j++)
+ if (i1 == -1 && starts_with(s1 + j, suffix)) {
+ i1 = i;
+ break;
+ }
+ for (j = start; j <= off; j++)
+ if (i2 == -1 && starts_with(s2 + j, suffix)) {
+ i2 = i;
+ break;
+ }
}
if (i1 == -1 && i2 == -1)
return 0;
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* [PATCHv2 3/7] t7004-tag: add version sort tests to show prerelease reordering issues
From: SZEDER Gábor @ 2016-12-08 14:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
Version sort with prerelease reordering sometimes puts tagnames in the
wrong order, when the common part of two compared tagnames ends with
the leading character(s) of one or more configured prerelease
suffixes. Add tests that demonstrate these issues.
The unrelated '--format should list tags as per format given' test
later uses tags matching the same prefix as the version sort tests,
thus was affected by the new tags added for the new tests in this
patch. Change that test to perform its checks on a different set of
tags.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/t7004-tag.sh | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 920a1b4b2..6445aae29 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1538,6 +1538,32 @@ test_expect_success 'reverse version sort with prerelease reordering' '
test_cmp expect actual
'
+test_expect_failure 'version sort with prerelease reordering and common leading character' '
+ test_config versionsort.prereleaseSuffix -before &&
+ git tag foo1.7-before1 &&
+ git tag foo1.7 &&
+ git tag foo1.7-after1 &&
+ git tag -l --sort=version:refname "foo1.7*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.7-before1
+ foo1.7
+ foo1.7-after1
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_failure 'version sort with prerelease reordering, multiple suffixes and common leading character' '
+ test_config versionsort.prereleaseSuffix -before &&
+ git config --add versionsort.prereleaseSuffix -after &&
+ git tag -l --sort=version:refname "foo1.7*" >actual &&
+ cat >expect <<-\EOF &&
+ foo1.7-before1
+ foo1.7-after1
+ foo1.7
+ EOF
+ test_cmp expect actual
+'
+
run_with_limited_stack () {
(ulimit -s 128 && "$@")
}
@@ -1566,13 +1592,11 @@ EOF"
test_expect_success '--format should list tags as per format given' '
cat >expect <<-\EOF &&
- refname : refs/tags/foo1.10
- refname : refs/tags/foo1.3
- refname : refs/tags/foo1.6
- refname : refs/tags/foo1.6-rc1
- refname : refs/tags/foo1.6-rc2
+ refname : refs/tags/v1.0
+ refname : refs/tags/v1.0.1
+ refname : refs/tags/v1.1.3
EOF
- git tag -l --format="refname : %(refname)" "foo*" >actual &&
+ git tag -l --format="refname : %(refname)" "v1*" >actual &&
test_cmp expect actual
'
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* [PATCHv2 1/7] t7004-tag: delete unnecessary tags with test_when_finished
From: SZEDER Gábor @ 2016-12-08 14:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Nguyễn Thái Ngọc Duy, Leho Kraav, git,
SZEDER Gábor
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
The '--force is moot with a non-existing tag name' test creates two
new tags, which are then deleted right after the test is finished,
outside the test_expect_success block, allowing 'git tag -d's output to
pollute the test output.
Use test_when_finished to delete those tags.
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---
t/t7004-tag.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 8b0f71a2a..396cffeeb 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -122,11 +122,11 @@ test_expect_success '--force can create a tag with the name of one existing' '
tag_exists mytag'
test_expect_success '--force is moot with a non-existing tag name' '
+ test_when_finished git tag -d newtag forcetag &&
git tag newtag >expect &&
git tag --force forcetag >actual &&
test_cmp expect actual
'
-git tag -d newtag forcetag
# deleting tags:
--
2.11.0.78.g5a2d011
^ permalink raw reply related
* Re: Serious bug with Git-2.11.0-64-bit and Git-LFS
From: Nick Warr @ 2016-12-08 14:00 UTC (permalink / raw)
To: Lars Schneider; +Cc: git
In-Reply-To: <06520F42-BD49-4349-83B3-74DCA1E260CD@gmail.com>
That looks pretty much like the error we're dealing with, any reason
why going back a point version on Git (not git-lfs) would resolve the
issue however?
On 8 December 2016 at 13:57, Lars Schneider <larsxschneider@gmail.com> wrote:
>
>> On 08 Dec 2016, at 12:46, Nick Warr <nick.warr@bossastudios.com> wrote:
>>
>> Using Git-2.11.0 with the latest git-lfs 1.5.2 (also tested with
>> 1.5.3) cloning from our locally hosted gitlab CE server via HTTPS.
>>
>> When cloning a repo (large, 3.3 gig in git, 10.3 in LFS) for the
>> first time the clone will finish the checkout of the git part, then
>> when it starts downloading the LFS parts it will reliably finish with
>> a smudge filter error.
>>
>> This leaves the repo in an unstable condition, you can then fetch the
>> lfs part without issue, but checking out the lfs files or trying a git
>> reset --hard will continue to spit out the same error. As you can see,
>> the actual error is not particularly useful.
>>
>> fatal: workers/unity/Assets/3rdPartyAssets/FORGE3D/Sci-Fi
>> Effects/Effects/Debris/Meshes/debris_junk.FBX: smudge filter lfs
>> failed
>> Unknown command ""
>>
>> Possibly it's due to the file extension being all capital letters, we
>> did manage to change the error by recommitting the file with a
>> lowercase extension, but it failed on the next file (which also had a
>> capital letter extension).
>>
>> This has happened on multiple fresh windows 10 64 bit installs,
>> different machines and target directories (to hopefully remove the
>> possibility of file permissions) where cloning is taking place.
>>
>> The solution is to back level to Git 2.10.2 and the error disappears.
>>
>> More than willing to provide any further information,
>
> Hi Nick,
>
> debris_junk.FBX is not stored properly in Git LFS.
> I explained the problem in detail here:
> https://github.com/git-lfs/git-lfs/issues/1729
>
> You should add the file properly to GitLFS to fix the problem.
> However, I think this is a regression in GitLFS and I hope it will
> be fixed in the next version.
>
> No change/fix in Git is required.
>
> Cheers,
> Lars
^ 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