Re: [PATCH v2] builtin/remote: quote remote name on error to display empty name
On Thu, 13 Sep 2018 14:51:56 -0700
Junio C Hamano wrote:
> Shulhan writes:
>
> > When adding new remote name with empty string, git will print the
> > following error message,
> >
> > fatal: '' is not a valid remote name\n
> >
> > But when removing remote name with empty string as input, git shows
> > the empty string without quote,
> >
> > fatal: No such remote: \n
> >
> > To make these error messages consistent, quote the name of the
> > remote that we tried and failed to find.
> >
> > Signed-off-by: Shulhan
> > Reviewed-by: Junio C Hamano
> > ---
> > builtin/remote.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Have you run "make test" with this change?
>
> I expect at least 5505.10 to fail without adjustment.
I am really sorry about that. I am pretty sure, I run "make" to check
if source is run successfully before I know the patch was correct, as
the "t/README" said,
Running Tests
-
The easiest way to run tests is to say "make". This runs all
the tests.
I will look into it later, if there is an error on test, I will send
another version of patch.
--
{ "github":"github.com/shuLhan", "site":"kilabit.info" }
Re: with git 1.8.3.1 get only merged tags
On Thu, Sep 13, 2018 at 1:27 PM Ævar Arnfjörð Bjarmason wrote: > > > On Tue, Sep 11 2018, Michal Novotny wrote: > > > I need to emulate git tag --merged with very old git 1.8.3.1. Is that > > somehow possible? > > I am looking for a bash function that would take what git 1.8.3.1 > > offers and return only the tags accessible from the current branch > > Jeff answer the question you had, but I just have one of my own: Is > RedHat stuck on 1.8-era git in some release it's still maintaining, does > this mean that e.g. you're still backporting security fixes to this > 2012-era release? Yes, that's exactly the case with RHEL-7. clime
Fwd: spelling mistake 'rerere' on docs/git-gc
retry -- plain text mode -- Forwarded message - From: Mikkel Hofstedt Juul Date: Fri, 14 Sep 2018 at 14:28 Subject: spelling mistake 'rerere' on docs/git-gc To: Hi See title in sentence: ...invocations of git add, packing refs, pruning reflog, rerere metadata or stale working trees. Thanks, keep up the good work! regards Mikkel
Re: [PATCH v1] fsmonitor: update GIT_TEST_FSMONITOR support
On 9/13/2018 2:54 PM, Ævar Arnfjörð Bjarmason wrote:
On Thu, Sep 13 2018, Ben Peart wrote:
diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
int git_config_get_fsmonitor(void)
{
if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
- core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+ core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 9028b47d92..545438c820 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncomon
pack-objects code
path where deltas larger than this limit require extra memory
allocation for bookkeeping.
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
Naming Tests
I've seen this & will watch out for it, but still, when I'm updating to
"next" in a couple of months I may not be tracking the exact state of
the integration of this patch, and then running with
GIT_FSMONITOR_TEST=... will suddenly be a noop.
So maybe something like this to test-lib.sh as well (or directly in
config.c):
if test -n "$GIT_FSMONITOR_TEST"
then
echo "The GIT_FSMONITOR_TEST variable has been renamed to
GIT_TEST_FSMONITOR"
exit 1
fi
Maybe I'm being too nitpicky and there's only two of us who run the test
with this anyway, and we can deal with it.
I agree that there are probably only 2 people in the world who ever used
this but I'm happy to add the additional test to make it obvious it has
been renamed.
It just rubs me the wrong way that we have a test mode that silently
stops being picked up because a command-line option or env variable got
renamed, especially since we've had it for 4 stable releases, especially
since it's so easy for us to avoid that confusion (just die),
v.s. potential time wasted downstream (wondering why fsmonitor stuff
broke on $SOME_OS even though we're testing for it during package
build...).
Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support
On 9/13/2018 6:08 PM, Junio C Hamano wrote: Thomas Gummerer writes: Thanks, I do think this is a good idea. I do however share Ævar's concern in https://public-inbox.org/git/87h8itkz2h@evledraar.gmail.com/. I have TEST_GIT_INDEX_VERSION=4 set in my config.mak since quite a long time, and had I missed this thread, I would all of a sudden not run the test suite with index format 4 anymore without any notice. I think the suggestion of erroring out if TEST_GIT_INDEX_VERSION is set would be useful in this case (and probably others in which you're renaming these variables). I am not enthused by "you are using an old variable---we fail your build/test". The assumption is that people let config.mak laying around regardless of how new/old the vintage of Git they are building and testing. I do not think you'd want to adjust your config.mak as you switch between 'maint' and 'next. I think it is OK to make it error only if the old one is set without the new one. Then people can have _both_ set to the same value during the period in which the topic sails through pu down to next down to master, after seeing an failure once while building and testing 'pu'. I'll make it a warning if they are both set so that people are reminded to remove the old variable and an error if only the old one is set so people know to update their environment. Btw, I think it would be nice to have all these renaming/documenting variables for the test suite patches in one series, so they are easier to look at with more context. Yeah, even though these three were posted as independent changes, their additions to t/README inevitably conflicted with each other. I'll combine all these into a single patch series. It grew as I discovered more that needed to be updated.
Re: [PATCH 2/3] archive: implement protocol v2 archive command
On Fri, Sep 14 2018, Jonathan Nieder wrote: > Ævar Arnfjörð Bjarmason wrote: >> On Wed, Sep 12 2018, Stefan Beller wrote: > >>> Would asking for a setlocale() on the server side be an unreasonable >>> feature request for the capabilities (in a follow up patch, and then not >>> just for archive but also fetch/push, etc.)? >> >> This would be very nice to have, but as you suggest in some follow-up >> change. > > Indeed, I think we've gone pretty far afield from the goal of this > patch series. > >> I think though that instead of doing setlocale() it would be better to >> pass some flag saying we're operating in a machine-readable mode, and >> then we'd (as part of the protocol defintion) say we're going to emit >> GIT_ERR_UPLOAD_ARCHIVE_EXPECTED_DELIM_PACKET or whatever. > > I think you're suggesting client-side message generation, and that is > one way to handle internationalization of server output. > > The main downside is when the server really does want to provide a > custom error message. For that, we'd need Yeah you can't do it for everything. E.g. hooks will want to spew out custom messages, and this hypothetical protocol extension won't have codes for those. So you'll still need to pass $LANG along. > 1. To propagate LANG to the server, so it knows what human language > to generate messages in. > > 2. On the server side, to produce messages in that language if > available, with an appropriate fallback if not. > > We've been thinking of doing at least (1) using the same trick as > server-options use (cramming it into client capabilities). > > It is difficult to use setlocale for this because it affects the whole > program (problematic for a threaded server) and affects features like > collation order instead of just message generation (problematic for > many things). Does gettext have a variant that takes a locale_t > argument? No, its API is fairly crappy and depends on setlocale(). Keep in mind though that we're not tied to that API. E.g. one way to work around this problem is to simply loop through all the languages we have translations for at server startup, for each one call setlocale() and gettext(), and save the result in a hash table for runtime lookup, then you'd just call sprintf(hash[language][message_id], ...) at runtime. That's all libintl is really doing under the hood, in a roundabout way where calls to setlocale() determine what table we're looking things up in. The reason I opted to go for gettext to begin with was mainly a) it was there b) the ubiquitous availability of tooling for translators when it comes to the *.po files. But the API for looking things up at runtime is fairly small, and easy to replace. We could e.g. replace all of our own gettext.[ch] wrapper with something that works somewhat like what I described above, with some extra build step to extract the relevant data from the *.mo files (or parse the *.po directly). > [...] >> 4) Aside from translation purposes, getting a machine-readable >> "push/pull" etc. mode would be very handy. E.g. now you need to >> parse stderr to see why exactly your push failed (hook denied, or >> non-fast-forward, or non-fast-forward where there was a lock race >> condition? ...). > > Indeed, this is a good reason to provide error codes instead of (in > the case where the message doesn't add anything to it) or alongside > (in case the error message is more specialized) human-oriented error > messages.
[PATCH v1 0/4] Cleanup pass on special test setups
As documented in t/README, the whole test suite can be run to test some special features that cannot be easily covered by a few specific test cases. Not all of these that exist in the code have been named consistantly and documented in r/README which leads to a discoverability problem. Update several of these variables to follow the same naming pattern and document them properly. To facilitate transitioning from the old names to the new names, add logic in t/test-lib.sh to give an error when the old variable is set to let people know they need to update their environment to use the new variable. If the new variable is also set, just give a warning so they can eventually remove the old variable. Base Ref: v2.19.0 Web-Diff: https://github.com/benpeart/git/commit/eff73d737e Checkout: git fetch https://github.com/benpeart/git git-test-cleanup-v1 && git checkout eff73d737e Ben Peart (4): correct typo/spelling error in t/README fsmonitor: update GIT_TEST_FSMONITOR support read-cache: update TEST_GIT_INDEX_VERSION support preload-index: update GIT_FORCE_PRELOAD_TEST support Makefile| 6 +++--- config.c| 2 +- preload-index.c | 3 ++- t/README| 13 - t/t1700-split-index.sh | 2 +- t/t7519-status-fsmonitor.sh | 6 +++--- t/test-lib.sh | 37 +++-- 7 files changed, 57 insertions(+), 12 deletions(-) base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670 -- 2.18.0.windows.1
[PATCH v1 3/4] read-cache: update TEST_GIT_INDEX_VERSION support
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with
the other GIT_TEST_ special setups and properly document its use.
Add logic in t/test-lib.sh to give an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.
Signed-off-by: Ben Peart
---
Makefile | 6 +++---
t/README | 4
t/test-lib.sh | 15 +--
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index 5a969f5830..9e84ef02f7 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,7 @@ all::
# (defaults to "man") if you want to have a different default when
# "git help" is called without a parameter specifying the format.
#
-# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite
+# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite
# with a different indexfile format version. If it isn't set the index
# file format used is index-v[23].
#
@@ -2599,8 +2599,8 @@ endif
ifdef GIT_INTEROP_MAKE_OPTS
@echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst
','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+
endif
-ifdef TEST_GIT_INDEX_VERSION
- @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst
','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+
+ifdef GIT_TEST_INDEX_VERSION
+ @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst
','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+
endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
diff --git a/t/README b/t/README
index 47165f7eab..9b13f6d12e 100644
--- a/t/README
+++ b/t/README
@@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the
fsmonitor
code path for utilizing a file system monitor to speed up detecting
new or changed files.
+GIT_TEST_INDEX_VERSION= exercises the index read/write code path
+for the index version specified. Can be set to any valid version
+(currently 2, 3, or 4).
+
Naming Tests
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0ef111d808..5f5f0f4b55 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,12 +134,23 @@ export EDITOR
GIT_TRACE_BARE=1
export GIT_TRACE_BARE
-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
then
- GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+ GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
export GIT_INDEX_VERSION
fi
+if test -n "$TEST_GIT_INDEX_VERSION"
+then
+ if test -n "$GIT_TEST_INDEX_VERSION"
+ then
+ echo "warning: the TEST_GIT_INDEX_VERSION variable has been
renamed to GIT_TEST_INDEX_VERSION"
+ else
+ echo "error: the TEST_GIT_INDEX_VERSION variable has been
renamed to GIT_TEST_INDEX_VERSION"
+ exit 1
+ fi
+fi
+
if test -n "$GIT_FSMONITOR_TEST"
then
if test -n "$GIT_TEST_FSMONITOR"
--
2.18.0.windows.1
[PATCH v1 4/4] preload-index: update GIT_FORCE_PRELOAD_TEST support
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with
the other GIT_TEST_ special setups and properly document its use.
Add logic in t/test-lib.sh to give an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.
Signed-off-by: Ben Peart
---
preload-index.c | 3 ++-
t/README| 3 +++
t/t7519-status-fsmonitor.sh | 4 ++--
t/test-lib.sh | 11 +++
4 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/preload-index.c b/preload-index.c
index 71cd2437a3..a850e197c2 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -5,6 +5,7 @@
#include "pathspec.h"
#include "dir.h"
#include "fsmonitor.h"
+#include "config.h"
#ifdef NO_PTHREADS
static void preload_index(struct index_state *index,
@@ -84,7 +85,7 @@ static void preload_index(struct index_state *index,
return;
threads = index->cache_nr / THREAD_COST;
- if ((index->cache_nr > 1) && (threads < 2) &&
getenv("GIT_FORCE_PRELOAD_TEST"))
+ if ((index->cache_nr > 1) && (threads < 2) &&
git_env_bool("GIT_TEST_PRELOAD_INDEX", 0))
threads = 2;
if (threads < 2)
return;
diff --git a/t/README b/t/README
index 9b13f6d12e..5670c7aad0 100644
--- a/t/README
+++ b/t/README
@@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write
code path
for the index version specified. Can be set to any valid version
(currently 2, 3, or 4).
+GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path
+by overriding the minimum number of cache entries required per thread.
+
Naming Tests
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,9 @@ do
git config core.preloadIndex $preload_val &&
if test $preload_val = true
then
- GIT_FORCE_PRELOAD_TEST=$preload_val; export
GIT_FORCE_PRELOAD_TEST
+ GIT_TEST_PRELOAD_INDEX=$preload_val; export
GIT_TEST_PRELOAD_INDEX
else
- unset GIT_FORCE_PRELOAD_TEST
+ sane_unset GIT_TEST_PRELOAD_INDEX
fi
'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5f5f0f4b55..3f447b8ddc 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -162,6 +162,17 @@ then
fi
fi
+if test -n "$GIT_FORCE_PRELOAD_TEST"
+then
+ if test -n "$GIT_TEST_PRELOAD_INDEX"
+ then
+ echo "warning: the GIT_FORCE_PRELOAD_TEST variable has been
renamed to GIT_TEST_PRELOAD_INDEX"
+ else
+ echo "error: the GIT_FORCE_PRELOAD_TEST variable has been
renamed to GIT_TEST_PRELOAD_INDEX"
+ exit 1
+ fi
+fi
+
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
--
2.18.0.windows.1
[PATCH v1 1/4] correct typo/spelling error in t/README
Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE Signed-off-by: Ben Peart --- t/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/README b/t/README index 9028b47d92..56a417439c 100644 --- a/t/README +++ b/t/README @@ -315,7 +315,7 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than bytes. -GIT_TEST_OE_DELTA_SIZE= exercises the uncomon pack-objects code +GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. -- 2.18.0.windows.1
[PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.
Add logic in t/test-lib.sh to give an error when the old variable is set to
let people know they need to update their environment to use the new
variable. If the new variable is also set, just give a warning so they can
eventually remove the old variable.
Signed-off-by: Ben Peart
---
config.c| 2 +-
t/README| 4
t/t1700-split-index.sh | 2 +-
t/t7519-status-fsmonitor.sh | 2 +-
t/test-lib.sh | 11 +++
5 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
int git_config_get_fsmonitor(void)
{
if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
- core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+ core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon
pack-objects code
path where deltas larger than this limit require extra memory
allocation for bookkeeping.
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
Naming Tests
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b3b4d83eaf..f6a856f24c 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -6,7 +6,7 @@ test_description='split index mode tests'
# We need total control of index splitting here
sane_unset GIT_TEST_SPLIT_INDEX
-sane_unset GIT_FSMONITOR_TEST
+sane_unset GIT_TEST_FSMONITOR
test_expect_success 'enable split index' '
git config splitIndex.maxPercentChange 100 &&
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
# To run the entire git test suite using fsmonitor:
#
# copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
#
# Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..0ef111d808 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,6 +140,17 @@ then
export GIT_INDEX_VERSION
fi
+if test -n "$GIT_FSMONITOR_TEST"
+then
+ if test -n "$GIT_TEST_FSMONITOR"
+ then
+ echo "warning: the GIT_FSMONITOR_TEST variable has been renamed
to GIT_TEST_FSMONITOR"
+ else
+ echo "error: the GIT_FSMONITOR_TEST variable has been renamed
to GIT_TEST_FSMONITOR"
+ exit 1
+ fi
+fi
+
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
--
2.18.0.windows.1
v2.19.0 Git install doesn't allow VS Code as an editor
Hi, I received the notification to update to the latest 2.19.0. When the installer asks for a default editor, it defaults to vim and when I select either VS Code option, it won't allow me to proceed. I've reproduced this on two machines. VS Code is up to date on both machines. Both machines: Windows 10 Latest VS Code (production)
[PATCH] refs: docstirng typo
---
refs/refs-internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 04425d6d1e..1fe5a7a22f 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -282,7 +282,7 @@ int refs_rename_ref_available(struct ref_store *refs,
*
* // Access information about the current reference:
* if (!(iter->flags & REF_ISSYMREF))
- * printf("%s is %s\n", iter->refname,
oid_to_hex(&iter->oid));
+ * printf("%s is %s\n", iter->refname,
oid_to_hex(iter->oid));
*
* // If you need to peel the reference:
* ref_iterator_peel(iter, &oid);
--
2.18.0
[PATCH v3] builtin/remote: quote remote name on error to display empty name
When adding new remote name with empty string, git will print the
following error message,
fatal: '' is not a valid remote name\n
But when removing remote name with empty string as input, git shows the
empty string without quote,
fatal: No such remote: \n
To make these error messages consistent, quote the name of the remote
that we tried and failed to find.
Signed-off-by: Shulhan
Reviewed-by: Junio C Hamano
---
builtin/remote.c | 6 +++---
t/t5505-remote.sh | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 40c6f8a1b..f7edf7f2c 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -626,7 +626,7 @@ static int mv(int argc, const char **argv)
oldremote = remote_get(rename.old_name);
if (!remote_is_configured(oldremote, 1))
- die(_("No such remote: %s"), rename.old_name);
+ die(_("No such remote: '%s'"), rename.old_name);
if (!strcmp(rename.old_name, rename.new_name) && oldremote->origin !=
REMOTE_CONFIG)
return migrate_file(oldremote);
@@ -762,7 +762,7 @@ static int rm(int argc, const char **argv)
remote = remote_get(argv[1]);
if (!remote_is_configured(remote, 1))
- die(_("No such remote: %s"), argv[1]);
+ die(_("No such remote: '%s'"), argv[1]);
known_remotes.to_delete = remote;
for_each_remote(add_known_remote, &known_remotes);
@@ -861,7 +861,7 @@ static int get_remote_ref_states(const char *name,
states->remote = remote_get(name);
if (!states->remote)
- return error(_("No such remote: %s"), name);
+ return error(_("No such remote: '%s'"), name);
read_branches();
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 241e6a319..d2a2cdd45 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -145,7 +145,7 @@ test_expect_success 'remove remote protects local branches'
'
test_expect_success 'remove errors out early when deleting non-existent
branch' '
(
cd test &&
- echo "fatal: No such remote: foo" >expect &&
+ echo "fatal: No such remote: '\''foo'\''" >expect &&
test_must_fail git remote rm foo 2>actual &&
test_i18ncmp expect actual
)
@@ -173,7 +173,7 @@ test_expect_success 'remove remote with a branch without
configured merge' '
test_expect_success 'rename errors out early when deleting non-existent
branch' '
(
cd test &&
- echo "fatal: No such remote: foo" >expect &&
+ echo "fatal: No such remote: '\''foo'\''" >expect &&
test_must_fail git remote rename foo bar 2>actual &&
test_i18ncmp expect actual
)
--
2.19.0.401.g521b368dc
Git 2.19.01 on Windows crasesh during GC
Problem signature: Problem Event Name: APPCRASH Application Name: git.exe Application Version: 2.19.0.1 Application Timestamp:5b980bc7 Fault Module Name:ntdll.dll Fault Module Version: 6.1.7601.24117 Fault Module Timestamp: 5add228d Exception Code: c005 Exception Offset: 00032964 OS Version: 6.1.7601.2.1.0.256.48 Locale ID:2057 Additional Information 1: 335e Additional Information 2: 335ee83054d6c615e4a7142c362e3dd4 Additional Information 3: 5184 Additional Information 4: 518485c5adbc52c624cc6890056919a6
Re: [PATCH 2/3] archive: implement protocol v2 archive command
Jonathan Nieder writes: >> I think though that instead of doing setlocale() it would be better to >> pass some flag saying we're operating in a machine-readable mode, and >> then we'd (as part of the protocol defintion) say we're going to emit >> GIT_ERR_UPLOAD_ARCHIVE_EXPECTED_DELIM_PACKET or whatever. > > I think you're suggesting client-side message generation, and that is > one way to handle internationalization of server output. > > The main downside is when the server really does want to provide a > custom error message. For that, we'd need > > 1. To propagate LANG to the server, so it knows what human language > to generate messages in. > > 2. On the server side, to produce messages in that language if > available, with an appropriate fallback if not. That is one way to do so, but it does not have to be the only way, I would think. You can send a machine parsable message in pieces, and assemble the parts of speech into a message at the receiving end. Like sending a msgid to identify an entry in the .pot file, and values to be filled in.
Re: [PATCH 2/3] archive: implement protocol v2 archive command
Junio C Hamano wrote: > Jonathan Nieder writes: >>> I think though that instead of doing setlocale() it would be better to >>> pass some flag saying we're operating in a machine-readable mode, and >>> then we'd (as part of the protocol defintion) say we're going to emit >>> GIT_ERR_UPLOAD_ARCHIVE_EXPECTED_DELIM_PACKET or whatever. >> >> I think you're suggesting client-side message generation, and that is >> one way to handle internationalization of server output. >> >> The main downside is when the server really does want to provide a >> custom error message. For that, we'd need >> >> 1. To propagate LANG to the server, so it knows what human language >> to generate messages in. >> >> 2. On the server side, to produce messages in that language if >> available, with an appropriate fallback if not. > > That is one way to do so, but it does not have to be the only way, I > would think. You can send a machine parsable message in pieces, and > assemble the parts of speech into a message at the receiving end. > Like sending a msgid to identify an entry in the .pot file, and > values to be filled in. That works if the same party controls the client and server and the client is up to date enough to know about every message the server would want to send. It doesn't work for - hooks - alternate server implementations - messages involved in an emergency fix - ... etc ... Don't get me wrong: for messages with a machine as an audience, error codes or similar structured errors are a great way to go, and getting client-side generation of messages for humans (not to mention styling, etc) are a nice bonus there. I stand by what's in the message you're replying to, though: if we actually want to be able to consistently provide useful messages to people who do not like to read English, then client-side generation won't get us all the way there. Jonathan
Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support
Ben Peart writes: >> I think it is OK to make it error only if the old one is set without >> the new one. Then people can have _both_ set to the same value >> during the period in which the topic sails through pu down to next >> down to master, after seeing an failure once while building and >> testing 'pu'. >> > > I'll make it a warning if they are both set so that people are > reminded to remove the old variable and an error if only the old one > is set so people know to update their environment. After the topic graduates to a stable release (or two), it would be a good addition to encourage people to clean things up, of course, but doing so too early by warning when they are both set is not a good idea, I would think. It would bring us back to "the user has to futz with config.mak every time switching between 'maint' and 'next'". > I'll combine all these into a single patch series. It grew as I > discovered more that needed to be updated. Thanks. I didn't mind it too much to have them as independent patches. It at least helped me forcing myself to give closer look at them ;-)
Re: [PATCH v2] builtin/remote: quote remote name on error to display empty name
Shulhan writes: > if source is run successfully before I know the patch was correct, as > the "t/README" said, > > Running Tests > - > > The easiest way to run tests is to say "make". This runs all > the tests. t/README says that it is sufficient to run "make" to perform tests, but that implicitly assumes that the people who are reading it are now in the t/ directory. We probably want to clarify the language there, perhaps like so? Thanks. t/README | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/README b/t/README index 8373a27fea..f95624959f 100644 --- a/t/README +++ b/t/README @@ -14,8 +14,8 @@ describes how your test scripts should be organized. Running Tests - -The easiest way to run tests is to say "make". This runs all -the tests. +The easiest way to run tests is to say "make" in this directory (or +"make test" from the top-level). This runs all the tests. *** t-basic.sh *** ok 1 - .git/objects should be empty after git init in an empty repo.
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes:
> +if test -n "$GIT_FSMONITOR_TEST"
> +then
> + if test -n "$GIT_TEST_FSMONITOR"
> + then
> + echo "warning: the GIT_FSMONITOR_TEST variable has been renamed
> to GIT_TEST_FSMONITOR"
> + else
> + echo "error: the GIT_FSMONITOR_TEST variable has been renamed
> to GIT_TEST_FSMONITOR"
> + exit 1
> + fi
> +fi
I would have expected that, because we are now doing multiple pairs
of variables in a single series, we would add a helper function that
can be called like so:
check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
in the earliest step. Perhaps something like this.
check_var_migration () {
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
eval "new_isset=\${${new_name}:+isset}"
case "$old_isset,$new_isset" in
isset,)
echo >&2 "error: $old_name is now $new_name"
exit 1 ;;
isset,isset)
# enable this, once $old_name no longer is valid anywhere
# echo >&2 "warning: $old_name is now $new_name"
# echo >&2 "hint: remove $old_name"
;;
esac
}
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Junio C Hamano writes:
> Ben Peart writes:
>
>> +if test -n "$GIT_FSMONITOR_TEST"
>> +then
>> +if test -n "$GIT_TEST_FSMONITOR"
>> +then
>> +echo "warning: the GIT_FSMONITOR_TEST variable has been renamed
>> to GIT_TEST_FSMONITOR"
>> +else
>> +echo "error: the GIT_FSMONITOR_TEST variable has been renamed
>> to GIT_TEST_FSMONITOR"
>> +exit 1
>> +fi
>> +fi
>
> I would have expected that, because we are now doing multiple pairs
> of variables in a single series, we would add a helper function that
> can be called like so:
>
> check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
>
> in the earliest step. Perhaps something like this.
>
> check_var_migration () {
> old_name=$1 new_name=$2
> eval "old_isset=\${${old_name}:+isset}"
> eval "new_isset=\${${new_name}:+isset}"
> case "$old_isset,$new_isset" in
> isset,)
> echo >&2 "error: $old_name is now $new_name"
> exit 1 ;;
> isset,isset)
> # enable this, once $old_name no longer is valid anywhere
> # echo >&2 "warning: $old_name is now $new_name"
> # echo >&2 "hint: remove $old_name"
> ;;
> esac
> }
Alternatively, we could do this, to warn and then migrate the value
given to the old variable automatically to the new variable and let
the test proceed.
check_var_migration () {
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
eval "new_isset=\${${new_name}:+isset}"
case "$old_isset,$new_isset" in
isset,)
echo >&2 "warning: $old_name is now $new_name"
echo >&2 "hint: set $new_name too during the transition period"
eval "$new_name=\$$old_name"
;;
isset,isset)
# do this later
# echo >&2 "warning: $old_name is now $new_name"
# echo >&2 "hint: remove $old_name"
;;
esac
}
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes:
> diff --git a/config.c b/config.c
> index 3461993f0a..3555c63f28 100644
> --- a/config.c
> +++ b/config.c
> @@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
> int git_config_get_fsmonitor(void)
> {
> if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
> - core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
> + core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
Sorry for not noticing earlier, but unlike 4/4 that changed
getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to
getenv(VAR), meaning "if it is set to any non-empty string, it is
true". Is there a reason for this discrepancy?
I _think_ the renaming should be done without getting mixed with
other changes like the git_env_bool() done in 4/4. The idea to use
git_env_bool() in stead of getenv() may be a good one, but then we
should consistently do so when appropriate, and that would make a
fine theme for another topic.
Re: [PATCH v8 5/7] revision: mark non-user-given objects instead
Matthew DeVore writes:
> Currently, list-objects.c incorrectly treats all root trees of commits
> as USER_GIVEN. Also, it would be easier to mark objects that are
> non-user-given instead of user-given, since the places in the code
> where we access an object through a reference are more obvious than
> the places where we access an object that was given by the user.
>
> Resolve these two problems by introducing a flag NOT_USER_GIVEN that
> marks blobs and trees that are non-user-given, replacing USER_GIVEN.
> (Only blobs and trees are marked because this mark is only used when
> filtering objects, and filtering of other types of objects is not
> supported yet.)
>
> This fixes a bug in that git rev-list behaved differently from git
> pack-objects. pack-objects would *not* filter objects given explicitly
> on the command line and rev-list would filter. This was because the two
> commands used a different function to add objects to the rev_info
> struct. This seems to have been an oversight, and pack-objects has the
> correct behavior, so I added a test to make sure that rev-list now
> behaves properly.
>
> Signed-off-by: Matthew DeVore
>
> fixup of 6defd70de
That's probably meant to go below "---".
> ---
> list-objects.c | 31 +
> revision.c | 1 -
> revision.h | 11 --
> t/t6112-rev-list-filters-objects.sh | 10 ++
> 4 files changed, 37 insertions(+), 16 deletions(-)
>
> diff --git a/list-objects.c b/list-objects.c
> index 243192af5..7a1a0929d 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -53,7 +53,7 @@ static void process_blob(struct traversal_context *ctx,
>
> pathlen = path->len;
> strbuf_addstr(path, name);
> - if (!(obj->flags & USER_GIVEN) && ctx->filter_fn)
> + if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
> r = ctx->filter_fn(LOFS_BLOB, obj,
> path->buf, &path->buf[pathlen],
> ctx->filter_data);
> @@ -120,17 +120,19 @@ static void process_tree_contents(struct
> traversal_context *ctx,
> continue;
> }
>
> - if (S_ISDIR(entry.mode))
> - process_tree(ctx,
> - lookup_tree(the_repository, entry.oid),
> - base, entry.path);
> + if (S_ISDIR(entry.mode)) {
> + struct tree *t = lookup_tree(the_repository, entry.oid);
> + t->object.flags |= NOT_USER_GIVEN;
> + process_tree(ctx, t, base, entry.path);
> + }
> else if (S_ISGITLINK(entry.mode))
> process_gitlink(ctx, entry.oid->hash,
> base, entry.path);
> - else
> - process_blob(ctx,
> - lookup_blob(the_repository, entry.oid),
> - base, entry.path);
> + else {
> + struct blob *b = lookup_blob(the_repository, entry.oid);
> + b->object.flags |= NOT_USER_GIVEN;
> + process_blob(ctx, b, base, entry.path);
> + }
> }
> }
>
> @@ -171,7 +173,7 @@ static void process_tree(struct traversal_context *ctx,
> }
>
> strbuf_addstr(base, name);
> - if (!(obj->flags & USER_GIVEN) && ctx->filter_fn)
> + if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
> r = ctx->filter_fn(LOFS_BEGIN_TREE, obj,
> base->buf, &base->buf[baselen],
> ctx->filter_data);
> @@ -185,7 +187,7 @@ static void process_tree(struct traversal_context *ctx,
> if (!failed_parse)
> process_tree_contents(ctx, tree, base);
>
> - if (!(obj->flags & USER_GIVEN) && ctx->filter_fn) {
> + if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn) {
> r = ctx->filter_fn(LOFS_END_TREE, obj,
> base->buf, &base->buf[baselen],
> ctx->filter_data);
> @@ -301,8 +303,11 @@ static void do_traverse(struct traversal_context *ctx)
>* an uninteresting boundary commit may not have its tree
>* parsed yet, but we are not going to show them anyway
>*/
> - if (get_commit_tree(commit))
> - add_pending_tree(ctx->revs, get_commit_tree(commit));
> + if (get_commit_tree(commit)) {
> + struct tree *tree = get_commit_tree(commit);
> + tree->object.flags |= NOT_USER_GIVEN;
> + add_pending_tree(ctx->revs, tree);
> + }
> ctx->show_commit(commit, ctx->show_data);
>
> if (ctx->revs->tree_blobs_in_commit_order)
> diff --git a/revision
Re: [PATCH v8 7/7] list-objects-filter: implement filter tree:0
Matthew DeVore writes:
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index c0e2bd6a0..14f251de4 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -50,6 +50,20 @@ static int gently_parse_list_objects_filter(
> return 0;
> }
>
> + } else if (skip_prefix(arg, "tree:", &v0)) {
> + unsigned long depth;
> + if (!git_parse_ulong(v0, &depth) || depth != 0) {
> + if (errbuf) {
> + strbuf_init(errbuf, 0);
> + strbuf_addstr(
> + errbuf,
> + _("only 'tree:0' is supported"));
This is not a new issue with this patch, but I think strbuf_init()
at the location of filling done like this is a bad idea. If the
caller gave you an errbuf that is pre-filled with something, we'd
leak memory and lose information. It only makes sense to _init() if
the caller gave us an uninitialized garbage (or a strbuf that has
just been initialized and is empty).
The existing callers seem to do STRBUF_INIT before passing it to
this function, so we probably should not do strbuf_init() here (and
other two places in this function) and simply add to it.
> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
> index bbbe7537d..8eeb85fbc 100755
> --- a/t/t5616-partial-clone.sh
> +++ b/t/t5616-partial-clone.sh
> @@ -154,6 +154,44 @@ test_expect_success 'partial clone with
> transfer.fsckobjects=1 uses index-pack -
> grep "git index-pack.*--fsck-objects" trace
> '
>
> +test_expect_success 'use fsck before and after manually fetching a missing
> subtree' '
> + # push new commit so server has a subtree
> + mkdir src/dir &&
> + echo "in dir" >src/dir/file.txt &&
> + git -C src add dir/file.txt &&
> + git -C src commit -m "file in dir" &&
> + git -C src push -u srv master &&
> + SUBTREE=$(git -C src rev-parse HEAD:dir) &&
> +
> + rm -rf dst &&
> + git clone --no-checkout --filter=tree:0 "file://$(pwd)/srv.bare" dst &&
> + git -C dst fsck &&
> +
> + # Make sure we only have commits, and all trees and blobs are missing.
> + git -C dst rev-list master --missing=allow-any --objects
> >fetched_objects &&
> + awk -f print_1.awk fetched_objects \
> + | xargs -n1 git -C dst cat-file -t >fetched_types &&
Break line after pipe "|", not before, and lose the backslash. You
do not need to over-indent the command on the downstream of the
pipe, i.e.
awk ... |
xargs -n1 git -C ... &&
Same comment applies elsewhere in this patch, not limited to this file.
> + sort fetched_types -u >unique_types.observed &&
Make it a habit not to add dashed options after real arguments, i.e.
sort -u fetched_types
> + echo commit >unique_types.expected &&
> + test_cmp unique_types.observed unique_types.expected &&
Always compare "expect" with "actual", not in the reverse order, i.e.
test_cmp expect actual
not
test_cmp actual expect
This is important because test_cmp reports failures by showing you
an output of "diff expect actual" and from "sh t5616-part*.sh -v"
you can see what additional/excess things were produced by the test
over what is expected, prefixed with "+", and what your code failed
to produce are shown prefixed with "-".
Thanks.
Re: [PATCH v8 7/7] list-objects-filter: implement filter tree:0
Junio C Hamano writes: >> diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh >> index bbbe7537d..8eeb85fbc 100755 >> --- a/t/t5616-partial-clone.sh >> +++ b/t/t5616-partial-clone.sh >> ... > > Break line after pipe "|", not before, and lose the backslash. You > do not need to over-indent the command on the downstream of the > pipe, i.e. > > awk ... | > xargs -n1 git -C ... && > > Same comment applies elsewhere in this patch, not limited to this file. > >> +sort fetched_types -u >unique_types.observed && > > Make it a habit not to add dashed options after real arguments, i.e. > > sort -u fetched_types > >> +echo commit >unique_types.expected && >> +test_cmp unique_types.observed unique_types.expected && > > Always compare "expect" with "actual", not in the reverse order, i.e. > > test_cmp expect actual > > not > > test_cmp actual expect > > This is important because test_cmp reports failures by showing you > an output of "diff expect actual" and from "sh t5616-part*.sh -v" > you can see what additional/excess things were produced by the test > over what is expected, prefixed with "+", and what your code failed > to produce are shown prefixed with "-". I notice that patches to other files like 6112 in this series also spread the above mistakes from existing lines. Please do not view what you see in these two test scripts before you start touching as a good example to follow---rather, treat them as antipattern X-<. 5616 is not as bad as 6112, but they both need to be cleaned up. We could alternatively do a post clean-up, but ideally we should first have a clean-up patch before this series to co. Thanks.
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
On 9/14/2018 1:15 PM, Junio C Hamano wrote:
Ben Peart writes:
diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
int git_config_get_fsmonitor(void)
{
if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
- core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+ core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
Sorry for not noticing earlier, but unlike 4/4 that changed
getenv(VAR) to git_env_bool(VAR, 0) "while at it", this leaves it to
getenv(VAR), meaning "if it is set to any non-empty string, it is
true". Is there a reason for this discrepancy?
The difference here is that core.fsmonitor isn't a boolean value. It is
a string to a command that is executed so it can't be moved over to
get_env_bool().
I _think_ the renaming should be done without getting mixed with
other changes like the git_env_bool() done in 4/4. The idea to use
git_env_bool() in stead of getenv() may be a good one, but then we
should consistently do so when appropriate, and that would make a
fine theme for another topic.
Git for games working group
Hey all, I've been putting together a working group for game studios wanting to use Git. There are a couple of blockers that keep most game and media companies on Perforce or others, but most would love to use git if it were feasible. The biggest tasks I'd like to tackle are: - improvements to large file management (mostly solved by LFS, GVFS) - avoiding excessive binary file conflicts (this is one of the big reasons most studio are on Perforce) Is anyone interested in contributing/offering insights? I suspect most folks here are git users as is, but if you know someone stuck on Perforce, I'd love to chat with them! Happy to field thoughts in this thread or answer other questions about why git doesn't work for games at the moment. Cheers, JA
Re: [PATCH v1 2/4] fsmonitor: update GIT_TEST_FSMONITOR support
Ben Peart writes: > The difference here is that core.fsmonitor isn't a boolean value. It > is a string to a command that is executed so it can't be moved over to > get_env_bool(). Ah, of course ;-) Then please take the following as a review comment for 4/4; checking if each getenv(VAR) should or should not become git_env_bool() and updating them should be done as a separate change for variables whether they are being renamed or not in this series. >> I _think_ the renaming should be done without getting mixed with >> other changes like the git_env_bool() done in 4/4. The idea to use >> git_env_bool() in stead of getenv() may be a good one, but then we >> should consistently do so when appropriate, and that would make a >> fine theme for another topic. >>
Re: Fwd: spelling mistake 'rerere' on docs/git-gc
Hi Mikkel, On Fri, Sep 14, 2018 at 02:31:04PM +0200, Mikkel Hofstedt Juul wrote: > See title > in sentence: > ...invocations of git add, packing refs, pruning reflog, rerere > metadata or stale working trees. I think that 'rerere' in this case, is correct, since it refers bookkeeping from the 'git rerere' command [1], which "reuse[s] recorded resolution[s] of conflict meregs". Thanks, Taylor [1]: https://git-scm.com/docs/git-rerere
Re: v2.19.0 Git install doesn't allow VS Code as an editor
Hi Zachary, On Fri, Sep 14, 2018 at 09:43:43AM -0500, Zachary Bryant wrote: > When the installer asks for a default editor, it defaults to vim and > when I select either VS Code option, it won't allow me to proceed. It sounds like this is an issue pertaining to Git for Windows, which uses an issue tracker that is separate from the mailing list here. Their tracker is at [1], but I'm cc-ing the maintainer here to let him know. Thanks, Taylor [1]: https://github.com/git-for-windows/git/issues
Re: [PATCH] refs: docstirng typo
On Fri, Sep 14, 2018 at 10:45 AM Tao Qingyun wrote: > refs: docstirng typo Typo: s/docstirng/docstring/ > --- Please sign-off your patch. See Documentation/SubmittingPatches. Thanks.
Re: [PATCH 7/9] tests: include detailed trace logs with --write-junit-xml upon failure
Hi Luke, On Wed, 5 Sep 2018, Luke Diamand wrote: > On 5 September 2018 at 13:39, Johannes Schindelin > wrote: > > > > On Wed, 5 Sep 2018, Luke Diamand wrote: > > > >> On 4 September 2018 at 12:09, Johannes Schindelin > >> wrote: > >> > > >> > On Tue, 4 Sep 2018, Eric Sunshine wrote: > >> > > >> >> On Mon, Sep 3, 2018 at 5:10 PM Johannes Schindelin via GitGitGadget > >> >> wrote: > >> >> > So let's do something different in VSTS: let's run all the tests with > >> >> > `--quiet` first, and only if a failure is encountered, try to trace > >> >> > the > >> >> > commands as they are executed. [...] > >> > >> Is this re-running just an individual test on its own or all the tests > >> within a single file? > > > > Upon encountering a failed test, it is re-running the entire test script > > afresh. > > > >> The latter shouldn't need this at all. > > > > Please do not let me die dumb. In other words, I would love for you to > > explain what exactly you mean by that sentence. > > Just re-run the script. You shouldn't need to kill p4d, as each script > starts up its own instance of p4d, and shuts it down when it exits. > > $ cd t > $ ./t9800-git-p4-basic.sh > Ctrl^C That Ctrl^C is not possible in the automated builds (which are the subject of this patch series). What *is* possible is to detect in the test script that there was a breakage, and then re-run the test script. Unfortunately, at this stage the first test script has not exited yet, so your p4d has not exited yet, and we have to clean up in some way before re-running (because the port in use is determined by the test script's number, so we really need to kill the old daemon). And this is exactly what my patches try to achieve here. > $ ./t9800-git-p4-basic.sh -v > > There's a cleanup() function in lib-git-p4.sh which kills the p4d > server, and that's invoked via: > > trap cleanup EXIT > > That's the only cleanup that each of the scripts require AFAIK. Good. However, we really will want to consider introducing something consistent here, something that works for *all* test scripts. IOW if any test script wants to start a process for the life-time of that script, and needs to kill that process at the end, we will want to have a consistent, documented way to register a function (or commands) to be called at the end. And that function (or commands) need to be run also when stopping everything in preparation for a re-run. > >> And the former, I'm not sure will actually work - most of the tests > >> assume some particular p4 state. But perhaps I'm missing something? > > > > No, the former would not work at all. Not only for the p4 tests: Git's > > tests frequently commit the deadly sin of relying on output of one another > > (wreaking havoc e.g. when test cases are skipped due to missing > > prerequisites, and latter test cases relying on their output). It is not > > the only thing that is wrong with the test suite, of course. > > > >> I also think it does look kind of ugly. And if there's one thing I've > >> learned, it's that the ugly hack you write today with the words "we'll > >> tidy this up later" goes on to live with you forever! > > > > Okay. > > > > (And having read lib-git-p4.sh, I kind of see where you learned that.) > > > > But maybe you also have some splendid idea what to do instead? Because > > doing something about it, that we need. We can't just say "oh, the only > > solution we found is ugly, so let's not do it at all". > > > > I am even going so far as to say: unless you have a better idea, it is > > pretty detrimental to criticize the current approach. It is the opposite > > of constructive. > > > > So let's hear some ideas how to improve the situation, m'kay? > > > > Just as a reminder, this is the problem I want to solve: I want to run the > > tests in a light-weight manner, with minimal output, and only in case of > > an error do I want to crank up the verbosity. Instead of wasting most of the > > effort to log everything and then throwing it away in most of the common > > cases, I suggest to re-run the entire test. > > > > What do you suggest to solve this? > > > > I don't know about any other tests, but the git-p4 tests don't take > any longer in verbose mode. That statement is too general to be correct. It may be the case in your setup, but I found that even a redirected, chatty stderr can slow down things on Windows substantially, probably due to the POSIX emulation going on in the background that needs to prepare for all kinds of eventualities (that we do not even need, but the MSYS2 runtime has no way of knowing that). > So one simple solution is to just run it in verbose mode - unless there > are other tests which have more overhead. It is a bit of an overhead. As the entire test run on Windows takes *quite* a while (due to said POSIX emulation issues), I was not able to quantify it exactly. > The trap/exit/cleanup method that the git-p4 tests already use would > seem to be ideally suited to cleaning up every
Re: [PATCH 7/9] tests: include detailed trace logs with --write-junit-xml upon failure
Hi Eric, On Wed, 5 Sep 2018, Eric Sunshine wrote: > On Wed, Sep 5, 2018 at 8:39 AM Johannes Schindelin > wrote: > > So let's hear some ideas how to improve the situation, m'kay? Just as > > a reminder, this is the problem I want to solve: I want to run the > > tests in a light-weight manner, with minimal output, and only in case > > of an error do I want to crank up the verbosity. Instead of wasting > > most of the effort to log everything and then throwing it away in most > > of the common cases, I suggest to re-run the entire test. > > What about the very different approach of capturing the full "verbose" > output the executed tests in addition to whatever is actually output > to the terminal? I fear it is not really possible to do a "verbose but not really" mode. I want the console output to be quiet, there is no use in chatting up the build log with these messages, as we have to run the tests in parallel, so the output would be utterly hard to interpret anyway. At the same time, I want verbose output for use in the test results. It is not really possible to `tee` all output, then "quiet down" the part that makes it into the log. > If a test fails, then (and only then) you can insert the captured > verbose output into the JUnit XML file. Yep. That's what my patch series does. If a test fails, then (and only then) I re-run the script with verbose output that is immediately moved into that JUnit XML file. > This way (if we always have the full verbose output at hand), you don't > need to re-run the test at all. But that way, if we always have the full verbose output, the build log will be unnecessarily verbose and confusing. > I've cc:'d Peff and Jonathan since I believe they are more familiar > with how all the capturing / output-redirection works in the test > suite. Okay. Ciao, Dscho P.S.: An unintended side effect of re-running the tests is to identify flakey tests. I do not yet have a way to represent this outcome in the test result, but I deem this an additional benefit in favor of keeping my current strategy.
Re: Git for games working group
Hi John, On Fri, Sep 14, 2018 at 10:55:39AM -0700, John Austin wrote: > Is anyone interested in contributing/offering insights? I suspect most > folks here are git users as is, but if you know someone stuck on > Perforce, I'd love to chat with them! I'm thrilled that other folks are interested in this, too. I'm not a video game developer myself, but I am the maintainer of Git LFS. If there's a capacity in which I could be useful to this group, I'd be more than happy to offer myself in that capacity. I'm cc-ing in brian carlson, Lars Schneider, and Preben Ingvaldsen on this email, too, since they all server on the core team of the project. Thanks, Taylor
Re: [PATCH 7/9] tests: include detailed trace logs with --write-junit-xml upon failure
Hi Peff, On Wed, 5 Sep 2018, Jeff King wrote: > On Wed, Sep 05, 2018 at 02:38:34PM -0400, Eric Sunshine wrote: > > > On Wed, Sep 5, 2018 at 8:39 AM Johannes Schindelin > > wrote: > > > So let's hear some ideas how to improve the situation, m'kay? Just > > > as a reminder, this is the problem I want to solve: I want to run > > > the tests in a light-weight manner, with minimal output, and only in > > > case of an error do I want to crank up the verbosity. Instead of > > > wasting most of the effort to log everything and then throwing it > > > away in most of the common cases, I suggest to re-run the entire > > > test. > > > > What about the very different approach of capturing the full "verbose" > > output the executed tests in addition to whatever is actually output > > to the terminal? If a test fails, then (and only then) you can insert > > the captured verbose output into the JUnit XML file. This way (if we > > always have the full verbose output at hand), you don't need to re-run > > the test at all. > > > > I've cc:'d Peff and Jonathan since I believe they are more familiar > > with how all the capturing / output-redirection works in the test > > suite. > > I don't think there's much to know beyond what you wrote. The > "--verbose" case does not really cost any more than the non-verbose one, > because the only difference is whether output goes to a file versus > /dev/null. But the commands all call write() regardless. This really only holds true on systems where redirected output is essentially a no-op. For the MSYS2-emulated case, this is incorrect. And as that case is already suffering, time-wise, I would be loathe to pile more onto it. > For --verbose-log, it does of course cost a little extra to run one > `tee` per script, and to write a few kb of logs. I doubt those are > measurable versus the rest of a script run, but I'm happy to be > disproven by numbers. I tried, and failed, to quantify this properly. Essentially because a single test run takes over 1h15m, so I tried to do it in the cloud, but those timings are too variable for anything approaching an accurate measurement. > There are some gymnastics done to re-exec the test script with the same > shell, but AFAIK those are robust and don't cost a lot (again, one extra > process per script run). It *is* actually a bit worse here (but only in the case of failures, which should be the minority of the cases): imagine an expensive test like t0027 that takes something like 14 minutes to run, and then a test failure near the end: re-running with verbose output will cost another 14 minutes. However, the benefit of allowing to pass flakey tests outweighs the costs here, if you ask me. > I'm not overly concerned about the cost of re-running a test, since the > idea is that failed tests should be rare. I would be a little worried > about flaky tests mysteriously righting themselves on the second run (so > you know a failure happened, but you have no good output to describe > it). This is actually a benefit. Pretty much all the times I can remember a test being flakey (with the one notable exception of the O_APPEND issue with GIT_TRACE), the *tests* were buggy. Plus, it is less concerning if a test fails occasionally vs a test that fails all the time. My plan is to indicate the outcome of "Passed upon rerun" in the test output eventually, as soon as there is server-side support for it. (Sidenote: there is technically already server-side support for it, but we would have to generate Visual Studio-style test output, and I had the hunch that some die-hard Linuxers among the core Git developers would take objection to that.) > I do agree that a test_atexit() cleanup would make a lot more sense than > what's in the original patch. And that's nicer than the exit trap we're > using already, because you may note that each caller has to manually > restore the original 'die' handler that test-lib.sh installs. Okay, then. I will work on this. Ciao, Dscho > That would also help with bitrot. If this funky cleanup case only causes > problems with junit output, then other people are likely to forget to do > it, and the mess falls onto the junit folks (i.e., Dscho). But if the > tests start _relying_ on test_atexit() being called (i.e., don't call > stop_git_daemon manually, but assume that the atexit handler does so), > then the responsible authors are a lot more likely to notice and fix it > early. > > -Peff >
Re: [PATCH 2/9] ci/lib.sh: encapsulate Travis-specific things
Hi Sebastian, On Wed, 5 Sep 2018, Sebastian Schuberth wrote: > On 9/3/2018 11:10 PM, Johannes Schindelin via GitGitGadget wrote: > > > +if test -n "$TRAVIS_COMMIT" > > +then > > + # We are running within Travis CI > > Personally, I'd find a check like > > if test "$TRAVIS" = "true" > > more speaking (also see [1]). Good call. Will fix, Dscho > > [1] https://docs.travis-ci.com/user/environment-variables/ > > -- > Sebastian Schuberth > >
[Bug report] Git incorrectly selects language in macos
It doesn't use English when other language is available as a secondary language. Reproducing: 1. Open "Language & Region" in macos settings 2. In "Preferred languages" box, set English as a primary language. 3. Add another language, that git is translated to, as a secondary language, for example, French or German. 4. Run any git command - git will use the secondary language, instead of English. When the secondary language is removed, then git starts using English again. I have git 2.19.0, installed from brew, and my OS is macOS 10.13.6 .
Re: [PATCH v8 5/7] revision: mark non-user-given objects instead
On Fri, Sep 14, 2018 at 10:23 AM Junio C Hamano wrote: > > Matthew DeVore writes: > > > Signed-off-by: Matthew DeVore > > > > fixup of 6defd70de > > That's probably meant to go below "---". > That line shouldn't be there at all, sorry! It came from me putting that text in a commit which was meant to be a fixup of another commit when I ran rebase -i. I asked rebase to make it a "squash" so I could edit the commit message of the earlier commit (6defd70de). Then rebase merged the two descriptions and let me edit them, but I didn't remember to delete the latter commit's message. I probably should have made the earlier commit (6defd70de) a "reword", and the later commit a "fixup", rather than "pick" followed by "squash"
[PATCH v2 0/5] Cleanup pass on special test setups
Changes this round are to use Junio's more elegant script to test and warn
about using old variables and munging which changes are in which commit.
Base Ref: v2.19.0
Web-Diff: https://github.com/benpeart/git/commit/79d62d39e4
Checkout: git fetch https://github.com/benpeart/git git-test-cleanup-v2 && git
checkout 79d62d39e4
### Interdiff (v1..v2):
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 3f447b8ddc..17a56f44ad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,38 +140,27 @@ then
export GIT_INDEX_VERSION
fi
-if test -n "$TEST_GIT_INDEX_VERSION"
-then
- if test -n "$GIT_TEST_INDEX_VERSION"
- then
- echo "warning: the TEST_GIT_INDEX_VERSION variable has been
renamed to GIT_TEST_INDEX_VERSION"
- else
- echo "error: the TEST_GIT_INDEX_VERSION variable has been
renamed to GIT_TEST_INDEX_VERSION"
- exit 1
- fi
-fi
-
-if test -n "$GIT_FSMONITOR_TEST"
-then
- if test -n "$GIT_TEST_FSMONITOR"
- then
- echo "warning: the GIT_FSMONITOR_TEST variable has been renamed
to GIT_TEST_FSMONITOR"
- else
- echo "error: the GIT_FSMONITOR_TEST variable has been renamed
to GIT_TEST_FSMONITOR"
- exit 1
- fi
-fi
+check_var_migration () {
+ old_name=$1 new_name=$2
+ eval "old_isset=\${${old_name}:+isset}"
+ eval "new_isset=\${${new_name}:+isset}"
+ case "$old_isset,$new_isset" in
+ isset,)
+ echo >&2 "warning: $old_name is now $new_name"
+ echo >&2 "hint: set $new_name too during the transition period"
+ eval "$new_name=\$$old_name"
+ ;;
+ isset,isset)
+ # do this later
+ # echo >&2 "warning: $old_name is now $new_name"
+ # echo >&2 "hint: remove $old_name"
+ ;;
+ esac
+}
-if test -n "$GIT_FORCE_PRELOAD_TEST"
-then
- if test -n "$GIT_TEST_PRELOAD_INDEX"
- then
- echo "warning: the GIT_FORCE_PRELOAD_TEST variable has been
renamed to GIT_TEST_PRELOAD_INDEX"
- else
- echo "error: the GIT_FORCE_PRELOAD_TEST variable has been
renamed to GIT_TEST_PRELOAD_INDEX"
- exit 1
- fi
-fi
+check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
+check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
### Patches
Ben Peart (5):
correct typo/spelling error in t/README
preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
fsmonitor: update GIT_TEST_FSMONITOR support
read-cache: update TEST_GIT_INDEX_VERSION support
preload-index: update GIT_FORCE_PRELOAD_TEST support
Makefile| 6 +++---
config.c| 2 +-
preload-index.c | 3 ++-
t/README| 13 -
t/t1700-split-index.sh | 2 +-
t/t7519-status-fsmonitor.sh | 6 +++---
t/test-lib.sh | 26 --
7 files changed, 46 insertions(+), 12 deletions(-)
base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
--
2.18.0.windows.1
[PATCH v2 1/5] correct typo/spelling error in t/README
Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE Signed-off-by: Ben Peart --- t/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/README b/t/README index 9028b47d92..56a417439c 100644 --- a/t/README +++ b/t/README @@ -315,7 +315,7 @@ packs on demand. This normally only happens when the object size is over 2GB. This variable forces the code path on any object larger than bytes. -GIT_TEST_OE_DELTA_SIZE= exercises the uncomon pack-objects code +GIT_TEST_OE_DELTA_SIZE= exercises the uncommon pack-objects code path where deltas larger than this limit require extra memory allocation for bookkeeping. -- 2.18.0.windows.1
[PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
Teach GIT_FORCE_PRELOAD_TEST to take a boolean to turn on or off this test
feature instead of simply testing for existance.
Signed-off-by: Ben Peart
---
preload-index.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/preload-index.c b/preload-index.c
index 71cd2437a3..0a4e2933bb 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -5,6 +5,7 @@
#include "pathspec.h"
#include "dir.h"
#include "fsmonitor.h"
+#include "config.h"
#ifdef NO_PTHREADS
static void preload_index(struct index_state *index,
@@ -84,7 +85,7 @@ static void preload_index(struct index_state *index,
return;
threads = index->cache_nr / THREAD_COST;
- if ((index->cache_nr > 1) && (threads < 2) &&
getenv("GIT_FORCE_PRELOAD_TEST"))
+ if ((index->cache_nr > 1) && (threads < 2) &&
git_env_bool("GIT_FORCE_PRELOAD_TEST", 0))
threads = 2;
if (threads < 2)
return;
--
2.18.0.windows.1
[PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
Rename TEST_GIT_INDEX_VERSION to GIT_TEST_INDEX_VERSION for consistency with
the other GIT_TEST_ special setups and properly document its use.
Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.
Signed-off-by: Ben Peart
---
Makefile | 6 +++---
t/README | 4
t/test-lib.sh | 5 +++--
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index 5a969f5830..9e84ef02f7 100644
--- a/Makefile
+++ b/Makefile
@@ -400,7 +400,7 @@ all::
# (defaults to "man") if you want to have a different default when
# "git help" is called without a parameter specifying the format.
#
-# Define TEST_GIT_INDEX_VERSION to 2, 3 or 4 to run the test suite
+# Define GIT_TEST_INDEX_VERSION to 2, 3 or 4 to run the test suite
# with a different indexfile format version. If it isn't set the index
# file format used is index-v[23].
#
@@ -2599,8 +2599,8 @@ endif
ifdef GIT_INTEROP_MAKE_OPTS
@echo GIT_INTEROP_MAKE_OPTS=\''$(subst ','\'',$(subst
','\'',$(GIT_INTEROP_MAKE_OPTS)))'\' >>$@+
endif
-ifdef TEST_GIT_INDEX_VERSION
- @echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst
','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@+
+ifdef GIT_TEST_INDEX_VERSION
+ @echo GIT_TEST_INDEX_VERSION=\''$(subst ','\'',$(subst
','\'',$(GIT_TEST_INDEX_VERSION)))'\' >>$@+
endif
@if cmp $@+ $@ >/dev/null 2>&1; then $(RM) $@+; else mv $@+ $@; fi
diff --git a/t/README b/t/README
index 47165f7eab..9b13f6d12e 100644
--- a/t/README
+++ b/t/README
@@ -323,6 +323,10 @@ GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the
fsmonitor
code path for utilizing a file system monitor to speed up detecting
new or changed files.
+GIT_TEST_INDEX_VERSION= exercises the index read/write code path
+for the index version specified. Can be set to any valid version
+(currently 2, 3, or 4).
+
Naming Tests
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 653688c067..397eb71578 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,9 +134,9 @@ export EDITOR
GIT_TRACE_BARE=1
export GIT_TRACE_BARE
-if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
then
- GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+ GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
export GIT_INDEX_VERSION
fi
@@ -159,6 +159,7 @@ check_var_migration () {
}
check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
--
2.18.0.windows.1
[PATCH v2 3/5] fsmonitor: update GIT_TEST_FSMONITOR support
Rename GIT_FSMONITOR_TEST to GIT_TEST_FSMONITOR for consistency with the
other GIT_TEST_ special setups and properly document its use.
Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.
Signed-off-by: Ben Peart
---
config.c| 2 +-
t/README| 4
t/t1700-split-index.sh | 2 +-
t/t7519-status-fsmonitor.sh | 2 +-
t/test-lib.sh | 20
5 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/config.c b/config.c
index 3461993f0a..3555c63f28 100644
--- a/config.c
+++ b/config.c
@@ -2278,7 +2278,7 @@ int git_config_get_max_percent_split_change(void)
int git_config_get_fsmonitor(void)
{
if (git_config_get_pathname("core.fsmonitor", &core_fsmonitor))
- core_fsmonitor = getenv("GIT_FSMONITOR_TEST");
+ core_fsmonitor = getenv("GIT_TEST_FSMONITOR");
if (core_fsmonitor && !*core_fsmonitor)
core_fsmonitor = NULL;
diff --git a/t/README b/t/README
index 56a417439c..47165f7eab 100644
--- a/t/README
+++ b/t/README
@@ -319,6 +319,10 @@ GIT_TEST_OE_DELTA_SIZE= exercises the uncommon
pack-objects code
path where deltas larger than this limit require extra memory
allocation for bookkeeping.
+GIT_TEST_FSMONITOR=$PWD/t7519/fsmonitor-all exercises the fsmonitor
+code path for utilizing a file system monitor to speed up detecting
+new or changed files.
+
Naming Tests
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b3b4d83eaf..f6a856f24c 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -6,7 +6,7 @@ test_description='split index mode tests'
# We need total control of index splitting here
sane_unset GIT_TEST_SPLIT_INDEX
-sane_unset GIT_FSMONITOR_TEST
+sane_unset GIT_TEST_FSMONITOR
test_expect_success 'enable split index' '
git config splitIndex.maxPercentChange 100 &&
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index 756beb0d8e..d77012ea6d 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -8,7 +8,7 @@ test_description='git status with file system watcher'
# To run the entire git test suite using fsmonitor:
#
# copy t/t7519/fsmonitor-all to a location in your path and then set
-# GIT_FSMONITOR_TEST=fsmonitor-all and run your tests.
+# GIT_TEST_FSMONITOR=fsmonitor-all and run your tests.
#
# Note, after "git reset --hard HEAD" no extensions exist other than 'TREE'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 44288cbb59..653688c067 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -140,6 +140,26 @@ then
export GIT_INDEX_VERSION
fi
+check_var_migration () {
+ old_name=$1 new_name=$2
+ eval "old_isset=\${${old_name}:+isset}"
+ eval "new_isset=\${${new_name}:+isset}"
+ case "$old_isset,$new_isset" in
+ isset,)
+ echo >&2 "warning: $old_name is now $new_name"
+ echo >&2 "hint: set $new_name too during the transition period"
+ eval "$new_name=\$$old_name"
+ ;;
+ isset,isset)
+ # do this later
+ # echo >&2 "warning: $old_name is now $new_name"
+ # echo >&2 "hint: remove $old_name"
+ ;;
+ esac
+}
+
+check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
+
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
--
2.18.0.windows.1
[PATCH v2 5/5] preload-index: update GIT_FORCE_PRELOAD_TEST support
Rename GIT_FORCE_PRELOAD_TEST to GIT_TEST_PRELOAD_INDEX for consistency with
the other GIT_TEST_ special setups and properly document its use.
Add logic in t/test-lib.sh to give a warning when the old variable is set to
let people know they need to update their environment to use the new
variable.
Signed-off-by: Ben Peart
---
preload-index.c | 2 +-
t/README| 3 +++
t/t7519-status-fsmonitor.sh | 4 ++--
t/test-lib.sh | 1 +
4 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/preload-index.c b/preload-index.c
index 0a4e2933bb..a850e197c2 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -85,7 +85,7 @@ static void preload_index(struct index_state *index,
return;
threads = index->cache_nr / THREAD_COST;
- if ((index->cache_nr > 1) && (threads < 2) &&
git_env_bool("GIT_FORCE_PRELOAD_TEST", 0))
+ if ((index->cache_nr > 1) && (threads < 2) &&
git_env_bool("GIT_TEST_PRELOAD_INDEX", 0))
threads = 2;
if (threads < 2)
return;
diff --git a/t/README b/t/README
index 9b13f6d12e..5670c7aad0 100644
--- a/t/README
+++ b/t/README
@@ -327,6 +327,9 @@ GIT_TEST_INDEX_VERSION= exercises the index read/write
code path
for the index version specified. Can be set to any valid version
(currently 2, 3, or 4).
+GIT_TEST_PRELOAD_INDEX= exercises the preload-index code path
+by overriding the minimum number of cache entries required per thread.
+
Naming Tests
diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
index d77012ea6d..8308d6d5b1 100755
--- a/t/t7519-status-fsmonitor.sh
+++ b/t/t7519-status-fsmonitor.sh
@@ -245,9 +245,9 @@ do
git config core.preloadIndex $preload_val &&
if test $preload_val = true
then
- GIT_FORCE_PRELOAD_TEST=$preload_val; export
GIT_FORCE_PRELOAD_TEST
+ GIT_TEST_PRELOAD_INDEX=$preload_val; export
GIT_TEST_PRELOAD_INDEX
else
- unset GIT_FORCE_PRELOAD_TEST
+ sane_unset GIT_TEST_PRELOAD_INDEX
fi
'
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 397eb71578..17a56f44ad 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -160,6 +160,7 @@ check_var_migration () {
check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
+check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
--
2.18.0.windows.1
Re: [PATCH v2 1/5] correct typo/spelling error in t/README
Hi, Ben Peart wrote: > Subject: correct typo/spelling error in t/README nit: what is the difference between a typo/spelling error and another kind of spelling error? Maybe this could be something like t/README: correct spelling of "uncommon" which makes it crystal clear what the patch will do. > Correct a spelling error in the documentation for GIT_TEST_OE_DELTA_SIZE The commit message should consist of complete sentences, so this is missing a period. Alternatively, I think it would be fine to omit the sentence altogether. > Signed-off-by: Ben Peart > --- > t/README | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) This appears to be the only usage of uncomon in the code base. Thanks for fixing it. With or without the commit message tweaks mentioned above, Reviewed-by: Jonathan Nieder Thanks.
Re: [PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
Ben Peart wrote: > Subject: preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean Reading this subject line alone (e.g. in "git log --oneline" output), it's not obvious to me what this patch will do. What behavior change does it make / what will it make newly possible? Maybe something like: preload-index: use git_env_bool() not getenv() for customization GIT_FORCE_PRELOAD_TEST is only checked for presence by using getenv(). Use git_env_bool() instead so that GIT_FORCE_PRELOAD_TEST=false can work as expected. > Teach GIT_FORCE_PRELOAD_TEST to take a boolean to turn on or off this test > feature instead of simply testing for existance. > > Signed-off-by: Ben Peart > --- > preload-index.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Can you say a little about how this came up? Was it just noticed while reading the code, or did it come up in practice? I wonder if a more useful knob would be a GIT_FORCE_PRELOAD_THREADS setting, but that's orthogonal to this change. Thanks and hope that helps, Jonathan
Re: Git for games working group
Hey Taylor, Great to have your support! I think LFS has done a great job so far solving the large file issue. I've been working myself on strategies for handling binary conflicts, and particularly how to do it in a git-friendly way (ie. avoiding as much centralization as possible and playing into the commit/branching model of git). I've got to a loose design that I like, but it'd be good to get some feedback, as well as hearing what other game devs would want in a binary conflict system. - John On Fri, Sep 14, 2018 at 12:00 PM Taylor Blau wrote: > > Hi John, > > On Fri, Sep 14, 2018 at 10:55:39AM -0700, John Austin wrote: > > Is anyone interested in contributing/offering insights? I suspect most > > folks here are git users as is, but if you know someone stuck on > > Perforce, I'd love to chat with them! > > I'm thrilled that other folks are interested in this, too. I'm not a > video game developer myself, but I am the maintainer of Git LFS. If > there's a capacity in which I could be useful to this group, I'd be more > than happy to offer myself in that capacity. > > I'm cc-ing in brian carlson, Lars Schneider, and Preben Ingvaldsen on > this email, too, since they all server on the core team of the project. > > Thanks, > Taylor >
Re: Git for games working group
Hey Taylor, Great to have your support! I think LFS has done a great job so far solving the large file issue. I've been working myself on strategies for handling binary conflicts, and particularly how to do it in a git-friendly way (ie. avoiding as much centralization as possible and playing into the commit/branching model of git). I've got to a loose design that I like, but it'd be good to get some feedback, as well as hearing what other game devs would want in a binary conflict system. - John On Fri, Sep 14, 2018 at 12:00 PM Taylor Blau wrote: > > Hi John, > > On Fri, Sep 14, 2018 at 10:55:39AM -0700, John Austin wrote: > > Is anyone interested in contributing/offering insights? I suspect most > > folks here are git users as is, but if you know someone stuck on > > Perforce, I'd love to chat with them! > > I'm thrilled that other folks are interested in this, too. I'm not a > video game developer myself, but I am the maintainer of Git LFS. If > there's a capacity in which I could be useful to this group, I'd be more > than happy to offer myself in that capacity. > > I'm cc-ing in brian carlson, Lars Schneider, and Preben Ingvaldsen on > this email, too, since they all server on the core team of the project. > > Thanks, > Taylor >
Re: Git for games working group
On Fri, Sep 14 2018, John Austin wrote: > - improvements to large file management (mostly solved by LFS, GVFS) There's also the nascent "don't fetch all the blobs" work-in-progress clone mode which might be of interest to you: https://blog.github.com/2018-09-10-highlights-from-git-2-19/#partial-clones > - avoiding excessive binary file conflicts (this is one of the big > reasons most studio are on Perforce) Is this just a reference to the advisory locking mode perforce/cvs etc. have or is there something else at play here?
Re: [PATCH v2 2/5] preload-index: teach GIT_FORCE_PRELOAD_TEST to take a boolean
Jonathan Nieder writes: > Maybe something like: > > preload-index: use git_env_bool() not getenv() for customization > > GIT_FORCE_PRELOAD_TEST is only checked for presence by using getenv(). > Use git_env_bool() instead so that GIT_FORCE_PRELOAD_TEST=false can > work as expected. That is much better description. Also $ cd t && GIT_FORCE_PRELOAD_TEST=t ./t-basic.sh would have allowed us to enable the feature in the older world, but I suspect it would instead fail the test, saying 't is not a bool nor int'. So strictly speaking, it is a backward incompatible change. I am not sure if I like it. >> Teach GIT_FORCE_PRELOAD_TEST to take a boolean to turn on or off this test >> feature instead of simply testing for existance. s/existance/existence/?
What's cooking in git.git (Sep 2018, #03; Fri, 14)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. The tip of 'next' hasn't been rewound yet, but the states of many topics that were marked to "Cook in 'next'" read "Will merge to 'master'" now. I'll probably start merging a handful of topics that have been in 'next' down to 'master' tonight (they appear at the bottom of "log --first-parent master..pu" output). The three GSoC "rewrite in C" topics are still unclassified in this "What's cooking" report, but I am hoping that we can have them in 'next' sooner rather than later. I got an impression that Dscho wanted a chance for the final clean-up on some of them, so I am not doing anything hasty yet at this moment, though. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [New Topics] * ab/fsck-skiplist (2018-09-12) 10 commits - fsck: support comments & empty lines in skipList - fsck: use oidset instead of oid_array for skipList - fsck: use strbuf_getline() to read skiplist file - fsck: add a performance test for skipList - fsck: add a performance test - fsck: document that skipList input must be unabbreviated - fsck: document and test commented & empty line skipList input - fsck: document and test sorted skipList input - fsck tests: add a test for no skipList input - fsck tests: setup of bogus commit object Update fsck.skipList implementation and documentation. Will merge to 'next'. * bc/hash-independent-tests (2018-09-13) 12 commits - t5318: use test_oid for HASH_LEN - t1407: make hash size independent - t1406: make hash-size independent - t1405: make hash size independent - t1400: switch hard-coded object ID to variable - t1006: make hash size independent - t0064: make hash size independent - t0027: make hash size independent - t0002: abstract away SHA-1 specific constants - t: update tests for SHA-256 - t: use hash translation table - t: add test functions to translate hash-related values Various tests have been updated to make it easier to swap the hash function used for object identification. Will merge to 'next'. * bp/mv-submodules-with-fsmonitor (2018-09-12) 1 commit - git-mv: allow submodules and fsmonitor to work together When fsmonitor is in use, after operation on submodules updates .gitmodules, we lost track of the fact that we did so and relied on stale fsmonitor data. Will merge to 'next'. * bp/read-cache-parallel (2018-09-13) 6 commits - SQUASH??? - read-cache: clean up casting and byte decoding - read-cache.c: optimize reading index format v4 - read-cache: load cache entries on worker threads - read-cache: load cache extensions on a worker thread - eoie: add End of Index Entry (EOIE) extension A new extension to the index file has been introduced, which allows the file to be read in parallel. Will merge to 'next' after squashing the fix in. * ds/coverage-diff (2018-09-12) 1 commit - contrib: add coverage-diff script The result of coverage test can be combined with "git blame" to check the test coverage of code introduced recently with a new 'coverage-diff' tool (in contrib/). Expecting a reroll. * ds/format-patch-range-diff-test (2018-09-12) 1 commit - t3206-range-diff.sh: cover single-patch case (this branch uses es/format-patch-interdiff and es/format-patch-rangediff.) Will merge to 'next'. * ds/multi-pack-verify (2018-09-13) 11 commits - fsck: verify multi-pack-index - multi-pack-index: report progress during 'verify' - multi-pack-index: verify object offsets - multi-pack-index: fix 32-bit vs 64-bit size check - multi-pack-index: verify oid lookup order - multi-pack-index: verify oid fanout order - multi-pack-index: verify missing pack - multi-pack-index: verify packname order - multi-pack-index: verify corrupt chunk lookup table - multi-pack-index: verify bad header - multi-pack-index: add 'verify' verb (this branch uses ds/multi-pack-index.) "git multi-pack-index" learned to detect corruption in the .midx file it uses, and this feature has been integrated into "git fsck". Will merge to 'next'. * en/sequencer-empty-edit-result-aborts (2018-09-13) 1 commit - sequencer: fix --allow-empty-message behavior, make it smarter "git rebase" etc. in Git 2.19 fails to abort when given an empty commit log message as result of editing, which has been corrected. Will merge to 'next'. * en/update-ref-no-deref-stdin (2018-09-12) 2 commits - update-ref: allow --no-deref with --stdin - update-ref: fix type of update_flags variable to match its usage "git update-ref" learned to make both "--no-deref" and "--stdin" work at the same time. Will merge to 'next'. * jt/l
Re: [Bug report] Git incorrectly selects language in macos
On Fri, Sep 14 2018, Niko Dzhus wrote: > It doesn't use English when other language is available as a secondary > language. > > Reproducing: > > 1. Open "Language & Region" in macos settings > 2. In "Preferred languages" box, set English as a primary language. > 3. Add another language, that git is translated to, as a secondary > language, for example, French or German. > 4. Run any git command - git will use the secondary language, instead > of English. > > When the secondary language is removed, then git starts using English again. > > I have git 2.19.0, installed from brew, and my OS is macOS 10.13.6 . What's the output of these two commands for you: 1. locale 2. env | grep -e LC -e LANG We don't do any such magic ourselves, so whatever this is is down to how i18n in general works on your system, do you have any other translated command-line program that works differently? I suspect there's some DWYM logic here that always treats English as a secondary language. Do you also e.g. get the same results if you select say Swedish as a primary language and German as a secondary? I.e. a Git in German, as opposed to Swedish?
Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
Ben Peart writes:
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 653688c067..397eb71578 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -134,9 +134,9 @@ export EDITOR
> GIT_TRACE_BARE=1
> export GIT_TRACE_BARE
>
> -if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
> +if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
> then
> - GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
> + GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
> export GIT_INDEX_VERSION
> fi
Is this done a bit before ...
> @@ -159,6 +159,7 @@ check_var_migration () {
> }
>
> check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
> +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
... this has a chance to kick in to say things like "Whoa you have
TEST_GIT_INDEX_VERSION that is an old spelling of
GIT_TEST_INDEX_VERSION", isn't it?
> # Add libc MALLOC and MALLOC_PERTURB test
> # only if we are not executing the test with valgrind
Re: [PATCH v2 4/5] read-cache: update TEST_GIT_INDEX_VERSION support
Junio C Hamano writes:
> Ben Peart writes:
>
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 653688c067..397eb71578 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -134,9 +134,9 @@ export EDITOR
>> GIT_TRACE_BARE=1
>> export GIT_TRACE_BARE
>>
>> -if test -n "${TEST_GIT_INDEX_VERSION:+isset}"
>> +if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
>> then
>> -GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
>> +GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
>> export GIT_INDEX_VERSION
>> fi
>
> Is this done a bit before ...
>
>> @@ -159,6 +159,7 @@ check_var_migration () {
>> }
>>
>> check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
>> +check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
>
> ... this has a chance to kick in to say things like "Whoa you have
> TEST_GIT_INDEX_VERSION that is an old spelling of
> GIT_TEST_INDEX_VERSION", isn't it?
So, the obvious fix would look like the patch below.
One problem with warning is that
$ TEST_GIT_INDEX_VERSION=4 sh ./t-basic.sh
(or any other depreated variable set without its modern counterpart
set) would fail due to extra output produced to the standard error
stream.
t/test-lib.sh | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 17a56f44ad..8ef86e05a3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -134,12 +134,6 @@ export EDITOR
GIT_TRACE_BARE=1
export GIT_TRACE_BARE
-if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
-then
- GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
- export GIT_INDEX_VERSION
-fi
-
check_var_migration () {
old_name=$1 new_name=$2
eval "old_isset=\${${old_name}:+isset}"
@@ -162,6 +156,13 @@ check_var_migration GIT_FSMONITOR_TEST GIT_TEST_FSMONITOR
check_var_migration TEST_GIT_INDEX_VERSION GIT_TEST_INDEX_VERSION
check_var_migration GIT_FORCE_PRELOAD_TEST GIT_TEST_PRELOAD_INDEX
+# Use specific version of the index file format
+if test -n "${GIT_TEST_INDEX_VERSION:+isset}"
+then
+ GIT_INDEX_VERSION="$GIT_TEST_INDEX_VERSION"
+ export GIT_INDEX_VERSION
+fi
+
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
Re: [RFC PATCH v4 3/3] t0014: Introduce alias testing suite
On 08.09.18 01:38, Eric Sunshine wrote:
On Fri, Sep 7, 2018 at 6:44 PM Tim Schumacher wrote:
Introduce a testing suite that is dedicated to aliases.
For now, check only if nested aliases work and if looping
aliases are detected successfully.
The looping aliases check for mixed execution is there but
expected to fail because there is no check in place yet.
Signed-off-by: Tim Schumacher
---
Unfortunately I don't have a fix for the last one yet, so I
marked it as expect_failure. The problem is that the test suite
is waiting a full minute until it aborts the running command
(which I guess should not take that long, as it blocks the whole
test suite for that span of time).
Should I try to decrease the timeout or should I remove that
test completely until I manage to get external calls fixed?
Perhaps just comment out that test for now and add a comment above it
explaining why it's commented out.
That will probably be the easiest thing to do. I commented it out for
now, added a short information about that to the code itself and a longer
explanation to the commit message.
As a last thing, is there any better way to use single quotes
than to write '"'"'? It isn't that bad, but it is hard to read,
especially for bash newcomers.
You should backslash-escape the quotes ("foo \'bar\' baz"), however,
in this case, it would make sense to use regex's with 'grep' to check
that you got the expected error message rather than reproducing the
message literally here in the script.
Backslash-escaping didn't work, that resulted in some parsing error.
I'm using i18ngrep now to search for the part of a message, which
eliminates the need for quotes completely.
More below.
diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='git command aliasing'
+
+. ./test-lib.sh
+
+test_expect_success 'setup environment' '
+ git init
+'
"git init" is invoked automatically by the test framework, so no need
for this test. You can drop it.
+test_expect_success 'nested aliases - internal execution' '
+ git config alias.nested-internal-1 nested-internal-2 &&
+ git config alias.nested-internal-2 status
+'
This isn't actually testing anything, is it? It's setting up the
aliases but never actually invoking them. I would have expected the
next line to actually run a command ("git nested-internal-1") and the
line after that to check that you got the expected output (whatever
"git status" would emit). Output from "git status" isn't necessarily
the easiest to test, though, so perhaps pick a different Git command
for testing (something for which the result can be very easily checked
-- maybe "git rm" or such).
Whoops, I didn't know when that went missing. I added it into a new version
of this patch.
Also, I decided to keep `git status`, because it seemed to be the only
command which doesn't need any files to produce some checkable output.
Checking the "On branch" message should be enough to confirm that the
command works as intended.
+test_expect_success 'nested aliases - mixed execution' '
+ git config alias.nested-external-1 "!git nested-external-2" &&
+ git config alias.nested-external-2 status
+'
Same observation.
+test_expect_success 'looping aliases - internal execution' '
+ git config alias.loop-internal-1 loop-internal-2 &&
+ git config alias.loop-internal-2 loop-internal-3 &&
+ git config alias.loop-internal-3 loop-internal-2 &&
+ test_must_fail git loop-internal-1 2>output &&
+ grep -q "fatal: alias loop detected: expansion of '"'"'loop-internal-1'"'"' does not
terminate" output &&
Don't bother using -q with 'grep'. Output is hidden already by the
test framework in normal mode, and not hidden when running in verbose
mode. And, the output of 'grep' might be helpful when debugging the
test if something goes wrong.
As noted above, you can use regex to match the expected error rather
than exactly duplicating the text of the message.
Finally, use 'test_i18ngrep' instead of 'grep' in order to play nice
with localization.
+ rm output
Tests don't normally bother cleaning up their output files like this
since such output can be helpful when debugging the test if something
goes wrong. (You'd want to use test_when_finished to cleanup anyhow,
but you don't need it in this case.)
I incorporated both of these suggestions.
+'
This is the first multi-patch series that I submitted, so I'm unsure if I
should send the updated patch only or if I should send the complete series
again as v5. Any pointers to what the correct procedure for this case is would
be appreciated.
Thanks for looking at this.
Tim
[PATCH] midx.c: mark a file-local symbol as static
Signed-off-by: Ramsay Jones
---
Hi Derrick,
If you need to re-roll your 'ds/multi-pack-verify' branch, could you
please squash this into the relevant patch (commit 64cbf3df21,
"multi-pack-index: add 'verify' verb", 2018-09-13).
[noticed by sparse].
Thanks.
ATB,
Ramsay Jones
midx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/midx.c b/midx.c
index 4d4c930522..713d6f9dde 100644
--- a/midx.c
+++ b/midx.c
@@ -926,7 +926,7 @@ void clear_midx_file(const char *object_dir)
free(midx);
}
-int verify_midx_error;
+static int verify_midx_error;
static void midx_report(const char *fmt, ...)
{
--
2.19.0
[PATCH] read-cache.c: fix a sparse warning
At one time, the POSIX standard required the type used to represent
a thread handle (pthread_t) be an arithmetic type. This is no longer
the case, probably because different platforms used to regularly
ignore that requirement. For example, on cygwin a pthread_t is a
pointer to a structure (a quite common choice), whereas on Linux it
is defined as an 'unsigned long int'.
On cygwin, but not on Linux, 'sparse' currently complains about an
initialiser used on a 'struct load_index_extensions' variable, whose
first field may be a pthread handle (if not compiled with NO_PTHREADS
set).
In order to fix the warning, move the (conditional) pthread field to
the end of the struct and change the initialiser to use a NULL, since
the new (unconditional) first field is a pointer type.
Signed-off-by: Ramsay Jones
---
Hi Ben,
If you need to re-roll your 'bp/read-cache-parallel' branch, could you
please squash this into the relevant patch (commit a090af334,
"read-cache: load cache extensions on a worker thread", 2018-09-12).
Thanks.
ATB,
Ramsay Jones
read-cache.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index b27dc01696..6254291742 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1908,13 +1908,13 @@ static void write_eoie_extension(struct strbuf *sb,
git_hash_ctx *eoie_context,
struct load_index_extensions
{
-#ifndef NO_PTHREADS
- pthread_t pthread;
-#endif
struct index_state *istate;
const char *mmap;
size_t mmap_size;
unsigned long src_offset;
+#ifndef NO_PTHREADS
+ pthread_t pthread;
+#endif
};
static void *load_index_extensions(void *data)
@@ -2145,7 +2145,7 @@ int do_read_index(struct index_state *istate, const char
*path, int must_exist)
const struct cache_header *hdr;
const char *mmap;
size_t mmap_size;
- struct load_index_extensions p = { 0 };
+ struct load_index_extensions p = { NULL };
unsigned long extension_offset = 0;
#ifndef NO_PTHREADS
int cpus, nr_threads;
--
2.19.0
Re: Git for games working group
> There's also the nascent "don't fetch all the blobs" work-in-progress > clone mode which might be of interest to you: > https://blog.github.com/2018-09-10-highlights-from-git-2-19/#partial-clones Yes! I've been pretty excited about this functionality. It drives a lot of GVFS/VFS for Git under the hood. I think it's a great solution to the repo-size issue. > Is this just a reference to the advisory locking mode perforce/cvs > etc. have or is there something else at play here? Good catch. I actually phrased this precisely to avoid calling it "File Locking". An essential example would be a team of 5 audio designers working together on the SFX for a game. If one designer wants to add a layer of ambience to 40% of the .wav files, they have to coordinate with everyone else on the project manually. Without coordination this developer will clobber any changes made to these files while he worked on them. File Locking is the way that Perforce manages this, where a developer can exclusively block modifications on a set of files across the entire team. File locking is just one solution to the problem. It's also one that doesn't play well with git's decentralized structure and branching model. I would state the problem more generally: Developers need some way to know, as early as possible, if modifying a file will cause conflicts upstream. Optionally this knowledge can block modifying the file directly (if we're certain there's already a conflicting version of the file on a different branch). JA
[PATCH v1 0/2] Cleanup tests for test_cmp argument ordering and "|" placement
As requested in: https://public-inbox.org/git/xmqqmuskas3a@gitster-ct.c.googlers.com/ this patchset corrects ordering of test_cmp arguments and | placement. The request didn't explicitly state whether all the tests should be cleaned up, but I did clean up as much as I reasonably could. The linked mail above also requested cleaning up flag/positional arg ordering so that flags appear first. The amount of wrong ordering I could find was small, so I put that cleanup in the other patchset. Matthew DeVore (2): t/*: fix pipe placement and remove \'s t/*: fix ordering of expected/observed arguments t/lib-gpg.sh | 4 +- t/t-basic.sh | 2 +- t/t0021-conversion.sh | 4 +- t/t1006-cat-file.sh| 8 +- t/t1300-config.sh | 9 +- t/t1303-wacky-config.sh| 4 +- t/t2101-update-index-reupdate.sh | 2 +- t/t3200-branch.sh | 2 +- t/t3320-notes-merge-worktrees.sh | 4 +- t/t3400-rebase.sh | 8 +- t/t3417-rebase-whitespace-fix.sh | 6 +- t/t3702-add-edit.sh| 4 +- t/t3903-stash.sh | 8 +- t/t3905-stash-include-untracked.sh | 2 +- t/t4025-hunk-header.sh | 2 +- t/t4117-apply-reject.sh| 6 +- t/t4124-apply-ws-rule.sh | 30 +- t/t4138-apply-ws-expansion.sh | 2 +- t/t5317-pack-objects-filter-objects.sh | 364 ++--- t/t5318-commit-graph.sh| 2 +- t/t5500-fetch-pack.sh | 5 +- t/t5616-partial-clone.sh | 30 +- t/t5701-git-serve.sh | 14 +- t/t5702-protocol-v2.sh | 10 +- t/t6023-merge-file.sh | 12 +- t/t6027-merge-binary.sh| 4 +- t/t6031-merge-filemode.sh | 2 +- t/t6112-rev-list-filters-objects.sh| 227 --- t/t7201-co.sh | 4 +- t/t7406-submodule-update.sh| 8 +- t/t7508-status.sh | 2 +- t/t7800-difftool.sh| 2 +- t/t9100-git-svn-basic.sh | 2 +- t/t9101-git-svn-props.sh | 4 +- t/t9133-git-svn-nested-git-repo.sh | 6 +- t/t9600-cvsimport.sh | 2 +- t/t9603-cvsimport-patchsets.sh | 4 +- t/t9604-cvsimport-timestamps.sh| 4 +- 38 files changed, 452 insertions(+), 363 deletions(-) -- 2.19.0.397.gdd90340f6a-goog
[PATCH v1 1/2] t/*: fix pipe placement and remove \'s
Where ever there was code in the tests like this:
foo \
| bar
such as:
git rev-list HEAD \
| grep $COMMIT
replace it with this:
foo |
bar
And add a blank line before and after the pipe where it aids readability
(it usually does).
Signed-off-by: Matthew DeVore
---
t/lib-gpg.sh | 4 +-
t/t1006-cat-file.sh| 8 +-
t/t1300-config.sh | 5 +-
t/t5317-pack-objects-filter-objects.sh | 330 ++---
t/t5500-fetch-pack.sh | 5 +-
t/t5616-partial-clone.sh | 30 ++-
t/t6112-rev-list-filters-objects.sh| 203 ---
t/t9101-git-svn-props.sh | 4 +-
8 files changed, 339 insertions(+), 250 deletions(-)
diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index 3fe02876c..2b8b81ac9 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -57,8 +57,8 @@ then
echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
--passphrase-fd 0 --pinentry-mode loopback \
--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
- gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
- | grep fingerprint: | cut -d" " -f4 | tr -d '\n' > \
+ gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K |
+ grep fingerprint: | cut -d" " -f4 | tr -d '\n' > \
${GNUPGHOME}/trustlist.txt &&
echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 7f19d591f..a0fa926d3 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -218,8 +218,8 @@ test_expect_success "--batch-check for a non-existent hash"
'
test "0042 missing
0084 missing" = \
"$( ( echo 0042;
- echo_without_newline 0084; ) \
- | git cat-file --batch-check)"
+ echo_without_newline 0084; ) |
+ git cat-file --batch-check)"
'
test_expect_success "--batch for an existent and a non-existent hash" '
@@ -227,8 +227,8 @@ test_expect_success "--batch for an existent and a
non-existent hash" '
$tag_content
missing" = \
"$( ( echo $tag_sha1;
- echo_without_newline ; ) \
- | git cat-file --batch)"
+ echo_without_newline ; ) |
+ git cat-file --batch)"
'
test_expect_success "--batch-check for an empty line" '
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index cdf1fed5d..5869d6cb6 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1770,8 +1770,9 @@ test_expect_success '--show-origin stdin with file
include' '
cat >expect <<-EOF &&
file:$INCLUDE_DIR/stdin.include include
EOF
- echo "[include]path=\"$INCLUDE_DIR\"/stdin.include" \
- | git config --show-origin --includes --file - user.stdin
>output &&
+ echo "[include]path=\"$INCLUDE_DIR\"/stdin.include" |
+ git config --show-origin --includes --file - user.stdin >output &&
+
test_cmp expect output
'
diff --git a/t/t5317-pack-objects-filter-objects.sh
b/t/t5317-pack-objects-filter-objects.sh
index 6710c8bc8..ce69148ec 100755
--- a/t/t5317-pack-objects-filter-objects.sh
+++ b/t/t5317-pack-objects-filter-objects.sh
@@ -20,17 +20,20 @@ test_expect_success 'setup r1' '
'
test_expect_success 'verify blob count in normal packfile' '
- git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 \
- | awk -f print_2.awk \
- | sort >expected &&
+ git -C r1 ls-files -s file.1 file.2 file.3 file.4 file.5 |
+ awk -f print_2.awk |
+ sort >expected &&
+
git -C r1 pack-objects --rev --stdout >all.pack <<-EOF &&
HEAD
EOF
git -C r1 index-pack ../all.pack &&
- git -C r1 verify-pack -v ../all.pack \
- | grep blob \
- | awk -f print_1.awk \
- | sort >observed &&
+
+ git -C r1 verify-pack -v ../all.pack |
+ grep blob |
+ awk -f print_1.awk |
+ sort >observed &&
+
test_cmp observed expected
'
@@ -39,23 +42,27 @@ test_expect_success 'verify blob:none packfile has no
blobs' '
HEAD
EOF
git -C r1 index-pack ../filter.pack &&
- git -C r1 verify-pack -v ../filter.pack \
- | grep blob \
- | awk -f print_1.awk \
- | sort >observed &&
+
+ git -C r1 verify-pack -v ../filter.pack |
+ grep blob |
+ awk -f print_1.awk |
+ sort >observed &&
+
nr=$(wc -l expected &&
- git -C r1 verify-pack
[PATCH v1 2/2] t/*: fix ordering of expected/observed arguments
This fixes various places where the ordering was obviously wrong and it
was either related to other patches in this patchset or was easy
find with grep.
Signed-off-by: Matthew DeVore
---
t/t-basic.sh | 2 +-
t/t0021-conversion.sh | 4 +--
t/t1300-config.sh | 4 +--
t/t1303-wacky-config.sh| 4 +--
t/t2101-update-index-reupdate.sh | 2 +-
t/t3200-branch.sh | 2 +-
t/t3320-notes-merge-worktrees.sh | 4 +--
t/t3400-rebase.sh | 8 +++---
t/t3417-rebase-whitespace-fix.sh | 6 ++---
t/t3702-add-edit.sh| 4 +--
t/t3903-stash.sh | 8 +++---
t/t3905-stash-include-untracked.sh | 2 +-
t/t4025-hunk-header.sh | 2 +-
t/t4117-apply-reject.sh| 6 ++---
t/t4124-apply-ws-rule.sh | 30 +++
t/t4138-apply-ws-expansion.sh | 2 +-
t/t5317-pack-objects-filter-objects.sh | 34 +-
t/t5318-commit-graph.sh| 2 +-
t/t5701-git-serve.sh | 14 +--
t/t5702-protocol-v2.sh | 10
t/t6023-merge-file.sh | 12 -
t/t6027-merge-binary.sh| 4 +--
t/t6031-merge-filemode.sh | 2 +-
t/t6112-rev-list-filters-objects.sh| 24 +-
t/t7201-co.sh | 4 +--
t/t7406-submodule-update.sh| 8 +++---
t/t7508-status.sh | 2 +-
t/t7800-difftool.sh| 2 +-
t/t9100-git-svn-basic.sh | 2 +-
t/t9133-git-svn-nested-git-repo.sh | 6 ++---
t/t9600-cvsimport.sh | 2 +-
t/t9603-cvsimport-patchsets.sh | 4 +--
t/t9604-cvsimport-timestamps.sh| 4 +--
33 files changed, 113 insertions(+), 113 deletions(-)
diff --git a/t/t-basic.sh b/t/t-basic.sh
index 850f651e4..224c098a8 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -1018,7 +1018,7 @@ test_expect_success SHA1 'validate git diff-files output
for a know cache/work t
:12 12 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c
M path3/subp3/file3sym
EOF
git diff-files >current &&
- test_cmp current expected
+ test_cmp expected current
'
test_expect_success 'git update-index --refresh should succeed' '
diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
index 308cd28f3..fd5f1ac64 100755
--- a/t/t0021-conversion.sh
+++ b/t/t0021-conversion.sh
@@ -166,10 +166,10 @@ test_expect_success expanded_in_repo '
rm -f expanded-keywords expanded-keywords-crlf &&
git checkout -- expanded-keywords &&
- test_cmp expanded-keywords expected-output &&
+ test_cmp expected-output expanded-keywords &&
git checkout -- expanded-keywords-crlf &&
- test_cmp expanded-keywords-crlf expected-output-crlf
+ test_cmp expected-output-crlf expanded-keywords-crlf
'
# The use of %f in a filter definition is expanded to the path to
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 5869d6cb6..e2cd50ecf 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -1001,7 +1001,7 @@ EOF
test_expect_success 'value continued on next line' '
git config --list > result &&
- test_cmp result expect
+ test_cmp expect result
'
cat > .git/config <<\EOF
@@ -1882,7 +1882,7 @@ test_expect_success '--replace-all does not invent
newlines' '
Qkey = b
EOF
git config --replace-all abc.key b &&
- test_cmp .git/config expect
+ test_cmp expect .git/config
'
test_done
diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 3b92083e1..e664e 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -14,7 +14,7 @@ setup() {
check() {
echo "$2" >expected
git config --get "$1" >actual 2>&1
- test_cmp actual expected
+ test_cmp expected actual
}
# 'check section.key regex value' verifies that the entry for
@@ -22,7 +22,7 @@ check() {
check_regex() {
echo "$3" >expected
git config --get "$1" "$2" >actual 2>&1
- test_cmp actual expected
+ test_cmp expected actual
}
test_expect_success 'modify same key' '
diff --git a/t/t2101-update-index-reupdate.sh b/t/t2101-update-index-reupdate.sh
index 685ec4563..6c32d42c8 100755
--- a/t/t2101-update-index-reupdate.sh
+++ b/t/t2101-update-index-reupdate.sh
@@ -73,7 +73,7 @@ test_expect_success 'update-index --update from subdir' '
100644 $(git hash-object dir1/file3) 0 dir1/file3
100644 $file2 0 file2
EOF
- test_cmp current expected
+ test_cmp expected current
'
test_expect_success 'update-index --update with pathspec' '
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 93f21ab07..478b82cf9 100755
--- a/t/t3200-branch.sh
+++ b/t
Re: [PATCH v8 7/7] list-objects-filter: implement filter tree:0
On Fri, Sep 14, 2018 at 10:47 AM Junio C Hamano wrote:
>
> Junio C Hamano writes:
>
> > Break line after pipe "|", not before, and lose the backslash. You
> > do not need to over-indent the command on the downstream of the
> > pipe, i.e.
> >
> > awk ... |
> > xargs -n1 git -C ... &&
> >
> > Same comment applies elsewhere in this patch, not limited to this file.
> >
> >> +sort fetched_types -u >unique_types.observed &&
> >
> > Make it a habit not to add dashed options after real arguments, i.e.
> >
> > sort -u fetched_types
> >
Done. I'm not sure why I made this mistake, since I usually prefer to
order flags before positional args. I didn't actually clean this up in
existing code as I did other mistakes, since it is very hard to find
and do thoroughly.
> >> +echo commit >unique_types.expected &&
> >> +test_cmp unique_types.observed unique_types.expected &&
> >
> > Always compare "expect" with "actual", not in the reverse order, i.e.
> >
> > test_cmp expect actual
> >
> > not
> >
> > test_cmp actual expect
> >
Done.
> > This is important because test_cmp reports failures by showing you
> > an output of "diff expect actual" and from "sh t5616-part*.sh -v"
> > you can see what additional/excess things were produced by the test
> > over what is expected, prefixed with "+", and what your code failed
> > to produce are shown prefixed with "-".
Hmm... I didn't know aout the -v flag. That's quite good to know, thanks!
>
> I notice that patches to other files like 6112 in this series also
> spread the above mistakes from existing lines. Please do not view
> what you see in these two test scripts before you start touching as
> a good example to follow---rather, treat them as antipattern X-<.
> 5616 is not as bad as 6112, but they both need to be cleaned up.
>
> We could alternatively do a post clean-up, but ideally we should
> first have a clean-up patch before this series to co.
I cleaned up existing tests in a new patchset here:
https://public-inbox.org/git/cover.1536969438.git.matv...@google.com/T/#t
- that new patch corrects the pipe placement and test_cmp argument
ordering.
There is no dependency between this patchset and the new one, though I
assume you want to commit the clean-up once first so maintain
consistency.
Here is an interdiff for this particular patch series (I replaced \t
with 8 spaces so it would be readable after my mail client mangles
it):
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 14f251de4..e8da2e858 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -30,7 +30,6 @@ static int gently_parse_list_objects_filter(
if (filter_options->choice) {
if (errbuf) {
-strbuf_init(errbuf, 0);
strbuf_addstr(
errbuf,
_("multiple filter-specs cannot be combined"));
@@ -54,7 +53,6 @@ static int gently_parse_list_objects_filter(
unsigned long depth;
if (!git_parse_ulong(v0, &depth) || depth != 0) {
if (errbuf) {
-strbuf_init(errbuf, 0);
strbuf_addstr(
errbuf,
_("only 'tree:0' is supported"));
@@ -85,10 +83,9 @@ static int gently_parse_list_objects_filter(
return 0;
}
-if (errbuf) {
-strbuf_init(errbuf, 0);
+if (errbuf)
strbuf_addf(errbuf, "invalid filter-spec '%s'", arg);
-}
+
memset(filter_options, 0, sizeof(*filter_options));
return 1;
}
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index f02b9ae37..5bc5b4445 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -216,7 +216,7 @@ test_expect_success 'missing non-root tree object
and rev-list' '
rm -rf repo &&
test_create_repo repo &&
mkdir repo/dir &&
-echo foo > repo/dir/foo &&
+echo foo >repo/dir/foo &&
git -C repo add dir/foo &&
git -C repo commit -m "commit dir/foo" &&
diff --git a/t/t5317-pack-objects-filter-objects.sh
b/t/t5317-pack-objects-filter-objects.sh
index 7a4d49ea1..510d3537f 100755
--- a/t/t5317-pack-objects-filter-objects.sh
+++ b/t/t5317-pack-objects-filter-objects.sh
@@ -61,7 +61,7 @@ test_expect_success 'verify normal and blob:none
packfiles have same commits/tre
test_expect_success 'get an error for missing tree object' '
git init r5 &&
-echo foo > r5/foo &&
+echo foo >r5/foo &&
git -C r5 add foo &&
git -C r5 commit -m "foo" &&
del=$(git -C r5 rev-parse HEAD^{tree} | sed "s|..|&/|") &&
@@ -97,7 +97,7 @@ test_expect_success 'grab tree directly when using tree:0' '
git -C r1 verify-pack -v ../commitsonly.pack >objs &&
Re: [Bug report] Git incorrectly selects language in macos
Tried what you suggested - it seems, it only ignores English. In you example, with Swedish as primary and German as secondary, git uses Swedish. With more that one secondary language, the one with a higher priority is being used, as expected. I also tried using non-generic English (English-UK and English-US), but they also get ignored. Terminal commands return the following: ➜ ~ locale LANG= LC_COLLATE="C" LC_CTYPE="UTF-8" LC_MESSAGES="C" LC_MONETARY="C" LC_NUMERIC="C" LC_TIME="C" LC_ALL= ➜ ~ env | grep -e LC -e LANG LC_CTYPE=UTF-8 ➜ ~ It doesn't change with primary/secondary language switching. I don't have any manual overrides in my .zshrc and .zprofile for those neither. On Sat, Sep 15, 2018 at 12:57 AM Ævar Arnfjörð Bjarmason wrote: > > > On Fri, Sep 14 2018, Niko Dzhus wrote: > > > It doesn't use English when other language is available as a secondary > > language. > > > > Reproducing: > > > > 1. Open "Language & Region" in macos settings > > 2. In "Preferred languages" box, set English as a primary language. > > 3. Add another language, that git is translated to, as a secondary > > language, for example, French or German. > > 4. Run any git command - git will use the secondary language, instead > > of English. > > > > When the secondary language is removed, then git starts using English again. > > > > I have git 2.19.0, installed from brew, and my OS is macOS 10.13.6 . > > What's the output of these two commands for you: > > 1. locale > 2. env | grep -e LC -e LANG > > We don't do any such magic ourselves, so whatever this is is down to how > i18n in general works on your system, do you have any other translated > command-line program that works differently? > > I suspect there's some DWYM logic here that always treats English as a > secondary language. > > Do you also e.g. get the same results if you select say Swedish as a > primary language and German as a secondary? I.e. a Git in German, as > opposed to Swedish?
[PATCH v2] refs: docstring typo
Signed-off-by: Tao Qingyun
---
refs/refs-internal.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 04425d6d1e..1fe5a7a22f 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -282,7 +282,7 @@ int refs_rename_ref_available(struct ref_store *refs,
*
* // Access information about the current reference:
* if (!(iter->flags & REF_ISSYMREF))
- * printf("%s is %s\n", iter->refname,
oid_to_hex(&iter->oid));
+ * printf("%s is %s\n", iter->refname,
oid_to_hex(iter->oid));
*
* // If you need to peel the reference:
* ref_iterator_peel(iter, &oid);
--
2.19.0
Re: [Bug report] Git incorrectly selects language in macos
Looks like the issue appeared after updating git from brew. I tried it on a different mac laptop, git 2.18 still used English, but after updating to 2.19 it started using secondary language. A quick search revealed that brew changed how it builds git recently. I think, it just didn't include i18n by default before, so I never noticed this. Here's the history of formula changes: https://github.com/Homebrew/homebrew-core/commits/master/Formula/git.rb Also, I found this thread https://github.com/Homebrew/homebrew-core/issues/31980 Anybody here familiar enough with the build process and dependencies of git to pinpoint what exactly is causing this and how to fix it?... On Fri, Sep 14, 2018 at 10:08 PM Niko Dzhus wrote: > > It doesn't use English when other language is available as a secondary > language. > > Reproducing: > > 1. Open "Language & Region" in macos settings > 2. In "Preferred languages" box, set English as a primary language. > 3. Add another language, that git is translated to, as a secondary > language, for example, French or German. > 4. Run any git command - git will use the secondary language, instead > of English. > > When the secondary language is removed, then git starts using English again. > > I have git 2.19.0, installed from brew, and my OS is macOS 10.13.6 .
