make always rebuilds some targets
[redirected from off-list conversation] On Sun, Dec 21, 2014 at 01:57:55PM -0700, Tim Harper wrote: I've noticed an issue in the builds script for 2.2.1 in which `make all` seems to invalidate itself. The first and second invocation of make all will compile all sources, but the 3rd finally recognizes the cache and uses it. This negatively impacts my ability to use `make strip`. Anyone else notice this? Can you be more specific about invalidate here? I'd guess from your mention of make strip that it's rebuilding the C code repeatedly. I don't see that, but I do notice two other problems running make repeatedly: 1. We also rebuild git-instaweb. It depends on gitweb, but that's a subdirectory target. I'm not sure if we should just drop this dependency completely, or if the intent is make sure gitweb is built. I do not see anything in the build rule that actually depends on gitweb, though. I think it could have been dropped in ff2e2cd (git-instaweb: Simplify build dependency on gitweb, 2011-05-07). +cc Jakub for that. 2. The perl scripts all depend on GIT-BUILD-OPTIONS as of e204b00 (Makefile: have perl scripts depend on NO_PERL setting, 2014-11-18). This is a good thing in general, but we build GIT-BUILD-OPTIONS each time we run, which results in all of the perl scripts being rebuilt. It really needs the usual see what it would look like, and do not update the file if it didn't change behavior that we do for things like GIT-SCRIPT-DEFINES. The patch below fixes it for me, but we might be able to do something less messy. diff --git a/Makefile b/Makefile index 7482a4d..54f1026 100644 --- a/Makefile +++ b/Makefile @@ -2041,45 +2041,46 @@ GIT-LDFLAGS: FORCE # that runs GIT-BUILD-OPTIONS, and then again to protect it # and the first level quoting from the shell that runs echo. GIT-BUILD-OPTIONS: FORCE - @echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' $@ - @echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' $@ - @echo DIFF=\''$(subst ','\'',$(subst ','\'',$(DIFF)))'\' $@ - @echo PYTHON_PATH=\''$(subst ','\'',$(PYTHON_PATH_SQ))'\' $@ - @echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' $@ - @echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' $@ - @echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' $@ - @echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' $@ - @echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' $@ - @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' $@ + @echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' $@+ + @echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' $@+ + @echo DIFF=\''$(subst ','\'',$(subst ','\'',$(DIFF)))'\' $@+ + @echo PYTHON_PATH=\''$(subst ','\'',$(PYTHON_PATH_SQ))'\' $@+ + @echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' $@+ + @echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' $@+ + @echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' $@+ + @echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' $@+ + @echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' $@+ + @echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' $@+ ifdef TEST_OUTPUT_DIRECTORY - @echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' $@ + @echo TEST_OUTPUT_DIRECTORY=\''$(subst ','\'',$(subst ','\'',$(TEST_OUTPUT_DIRECTORY)))'\' $@+ endif ifdef GIT_TEST_OPTS - @echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' $@ + @echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' $@+ endif ifdef GIT_TEST_CMP - @echo GIT_TEST_CMP=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_CMP)))'\' $@ + @echo GIT_TEST_CMP=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_CMP)))'\' $@+ endif ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT - @echo GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease $@ + @echo GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease $@+ endif - @echo NO_GETTEXT=\''$(subst ','\'',$(subst ','\'',$(NO_GETTEXT)))'\' $@ - @echo GETTEXT_POISON=\''$(subst ','\'',$(subst ','\'',$(GETTEXT_POISON)))'\' $@ + @echo NO_GETTEXT=\''$(subst ','\'',$(subst ','\'',$(NO_GETTEXT)))'\' $@+ + @echo GETTEXT_POISON=\''$(subst ','\'',$(subst ','\'',$(GETTEXT_POISON)))'\' $@+ ifdef GIT_PERF_REPEAT_COUNT - @echo GIT_PERF_REPEAT_COUNT=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPEAT_COUNT)))'\' $@ + @echo GIT_PERF_REPEAT_COUNT=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPEAT_COUNT)))'\' $@+ endif ifdef GIT_PERF_REPO - @echo GIT_PERF_REPO=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPO)))'\' $@ + @echo GIT_PERF_REPO=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPO)))'\' $@+ endif ifdef
Re: XDL_FAST_HASH can be very slow
I have been working with Peff on this and have more results to share. For background, xdl_hash_record is a hashing function, producing an unsigned long from an input string terminated by either a newline or the end of the mmap'd file. The original xdl_hash_record is essentially DJB hash, which does a multiplication, load, and xor for each byte of the input. Commit 6942efc introduces an XDL_FAST_HASH version of the same function that is clearly inspired by the DJB hash, but it does only one multiplication, load, and xor for each 8 bytes of input -- i.e., fewer loads, but also a lot less bit mixing. Less mixing means far more collisions, leading to the performance problems with evil-icons. It's not clear to me if the XDL_FAST_HASH version intended to match the output of the DJB hash function, but it doesn't at all. Peff has been experimenting with using two modern hash functions, FNV and Murmur3. In theory, these should produce fewer collisions than DJB, but in his measurements, they didn't run diff any faster than plain DJB. They do fix the evil-icons problem. I have implemented two simpler possibilities, both of which fix the problems diffing the evil-icons repository: 1. An XDL_FAST_HASH implementation that matches the output of the DJB hash exactly. Its performance is basically the same as DJB, because the only thing is does differently is load 8 bytes at a time instead of 1. It does all the same ALU operations as DJB. 2. Using (hash % prime_number) instead of (hash ((1hbits)-1)) to map hash values to buckets in the hash table. This helps because there's entropy in the high bits of the hash values that's lost completely if we just mask off the low hbits bits. I've chosen prime numbers that are close to the power-of-two sizes of the table -- e.g., 32749 instead of 32768 -- so very little space is wasted. Applying this change to the XDL_FAST_HASH hash function makes it perform as well as DJB and Murmur3. That is, it eliminates the performance problems with the evil-icons repo. I evaluated several of the hash functions according to how deep the chains are in each hash bucket, when diffing the evil-icons repo. DJB, Murmur3, and XDL_FAST_HASH%prime all produce near-optimal scattering, with the longest chain between 29 and 34 elements long. XDL_FAST_HASH as implemented in the current git tree -- with bit-masking instead of modulo-prime -- produces 100 buckets with chain lengths over 4000. Most of the other buckets are empty. Each of these long chains takes quadratic time to build and linear time to traverse, which presumably is where the terrible performance for evil-icons comes from. The bottom line is, I think XDL_FAST_HASH needs to go, because it has poorly understood collision behavior with pretty bad worst cases. I don't have strong feelings about what should replace it -- original DJB, a fixed XDL_FAST_HASH, Murmur3, or something else. All of the replacements have good collision behavior and good behavior on the repos I've tested, but appear to be a few percent slower in the common case. --Patrick -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XDL_FAST_HASH can be very slow
Patrick Reynolds p...@github.com writes: The original xdl_hash_record is essentially DJB hash, which does a multiplication, load, and xor for each byte of the input. Commit 6942efc introduces an XDL_FAST_HASH version of the same function that is clearly inspired by the DJB hash, but it does only one multiplication, load, and xor for each 8 bytes of input -- i.e., fewer loads, but also a lot less bit mixing. Less mixing means far more collisions, leading to the performance problems with evil-icons. It's not clear to me if the XDL_FAST_HASH version intended to match the output of the DJB hash function, but it doesn't at all. Note that XDL_FAST_HASH is just a ripoff of the hashing scheme that Linus socially engineered on G+ around that time. I didn't do any of the hash genealogy that you did here, and it now shows. The orginal patches are linked from 6942efc (xdiff: load full words in the inner loop of xdl_hash_record, 2012-04-06): https://lkml.org/lkml/2012/3/2/452 https://lkml.org/lkml/2012/3/5/6 The code still exists: https://github.com/torvalds/linux/blob/master/fs/namei.c#L1678 I have implemented two simpler possibilities, both of which fix the problems diffing the evil-icons repository: I think it would be best to separate three goals here: 1. hash function throughput 2. quality of the hash values 3. avoiding collision attacks XDL_FAST_HASH was strictly an attempt to improve throughput, and fairly successful at that (6942efc (xdiff: load full words in the inner loop of xdl_hash_record, 2012-04-06) quotes an 8% improvement on 'git log -p'). You are now addressing quality. I have no idea how you ran into this, but if you are reworking things already, I think it would be good to also randomize whatever hash you put in so as to give some measure of protection against collision attacks. 1. An XDL_FAST_HASH implementation that matches the output of the DJB hash exactly. Its performance is basically the same as DJB, because the only thing is does differently is load 8 bytes at a time instead of 1. It does all the same ALU operations as DJB. I don't think there's a point in having such a function, since it would mean a lot of code for no throughput gain. Let's just remove XDL_FAST_HASH and the original hashing scheme in favor of a better hash function. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] configure.ac,trace.c: check for CLOCK_MONOTONIC
On Sun, Dec 21, 2014 at 8:12 PM, brian m. carlson sand...@crustytoothpaste.net wrote: On Sun, Dec 21, 2014 at 10:53:35AM -0800, Reuben Hawkins wrote: CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3 systems being used in production. This change makes compiling git less tedious on older platforms. While I'm not opposed to this change, I expect that you'll find there's lots of subtle breakage when trying to compile (and run the testsuite for) git on RHEL 3. I've spoken up before to prevent breakage on RHEL/CentOS 5 (since $DAYJOB still supports it), but I'm not sure anyone's looking out for something as old as RHEL 3. I expect you'll probably have some pain points with perl and curl, among others. Yes, there are pain points with perl and curl. Those I've disable with other compile options when building on RHEL3, but reducing the number of options I have to set manually and increasing the number of automatic checks with configure is helpful. Sometime over the next few days I'll submit a v2 of the patches with Eric's comments taken into account. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Misbehaving git bisect bad HEAD
Running git bisect bad should be the same as git bisect bad HEAD, shouldn't it? When replaying this bisect log on the Linux kernel tree: git bisect start # bad: [97bf6af1f928216fd6c5a66e8a57bfa95a659672] Linux 3.19-rc1 git bisect bad 97bf6af1f928216fd6c5a66e8a57bfa95a659672 # good: [b2776bf7149bddd1f4161f14f79520f17fc1d71d] Linux 3.18 git bisect good b2776bf7149bddd1f4161f14f79520f17fc1d71d # good: [70e71ca0af244f48a5dcf56dc435243792e3a495] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next git bisect good 70e71ca0af244f48a5dcf56dc435243792e3a495 # good: [988adfdffdd43cfd841df734664727993076d7cb] Merge branch 'drm-next' of git://people.freedesktop.org/~airlied/linux git bisect good 988adfdffdd43cfd841df734664727993076d7cb # good: [b024793188002b9eed452b5f6a04d45003ed5772] staging: rtl8723au: phy_SsPwrSwitch92CU() was never called with bRegSSPwrLvl != 1 git bisect good b024793188002b9eed452b5f6a04d45003ed5772 # good: [66dcff86ba40eebb5133cccf450878f2bba102ef] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm git bisect good 66dcff86ba40eebb5133cccf450878f2bba102ef # bad: [88a57667f2990f00b019d46c8426441c9e516d51] Merge branch 'perf-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip git bisect bad 88a57667f2990f00b019d46c8426441c9e516d51 # good: [0ec28c37c21a2b4393692e832e11a7573ac545e2] Merge tag 'media/v3.19-2' of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media git bisect good 0ec28c37c21a2b4393692e832e11a7573ac545e2 # good: [c0f486fde3f353232c1cc2fd4d62783ac782a467] Merge tag 'pm+acpi-3.19-rc1-2' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm git bisect good c0f486fde3f353232c1cc2fd4d62783ac782a467 # bad: [34b85e3574424beb30e4cd163e6da2e2282d2683] Merge tag 'powerpc-3.19-2' of git://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux git bisect bad 34b85e3574424beb30e4cd163e6da2e2282d2683 # good: [64ec45bff6b3dade2643ed4c0f688a15ecf46ea2] Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost git bisect good 64ec45bff6b3dade2643ed4c0f688a15ecf46ea2 Running git bisect bad gives this: $ git bisect bad Bisecting: 6 revisions left to test after this (roughly 3 steps) [ec2aef5a8d3c14272f7a2d29b34f1f8e71f2be5b] power/perf/hv-24x7: Use kmem_cache_free() instead of kfree Running git bisect bad HEAD instead gives this: $ git bisect bad HEAD Bisecting: a merge base must be tested [56548fc0e86cb9156af7a7e1f15ba78f251dafaf] powerpc/powernv: Return to cpu offline loop when finished in KVM guest This is git 2.2.1. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-prompt: preserve command exit status
Signed-off-by: Tony Finch d...@dotat.at
---
contrib/completion/git-prompt.sh | 4
1 file changed, 4 insertions(+)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c5473dc..5fe69d0 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -288,6 +288,7 @@ __git_eread ()
# In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
__git_ps1 ()
{
+ local exit=$?
local pcmode=no
local detached=no
local ps1pc_start='\u@\h:\w '
@@ -511,4 +512,7 @@ __git_ps1 ()
else
printf -- $printf_format $gitstring
fi
+
+ # preserve exit status
+ return $exit
}
--
2.1.0.rc1.12.g1e9b79d
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Misbehaving git bisect bad HEAD
On Mon, Dec 22, 2014 at 3:04 PM, Andreas Schwab sch...@linux-m68k.org wrote:
Running git bisect bad should be the same as git bisect bad HEAD,
shouldn't it?
Yeah, it should.
When replaying this bisect log on the Linux kernel tree:
git bisect start
# bad: [97bf6af1f928216fd6c5a66e8a57bfa95a659672] Linux 3.19-rc1
git bisect bad 97bf6af1f928216fd6c5a66e8a57bfa95a659672
# good: [b2776bf7149bddd1f4161f14f79520f17fc1d71d] Linux 3.18
git bisect good b2776bf7149bddd1f4161f14f79520f17fc1d71d
# good: [70e71ca0af244f48a5dcf56dc435243792e3a495] Merge
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next
git bisect good 70e71ca0af244f48a5dcf56dc435243792e3a495
# good: [988adfdffdd43cfd841df734664727993076d7cb] Merge branch 'drm-next' of
git://people.freedesktop.org/~airlied/linux
git bisect good 988adfdffdd43cfd841df734664727993076d7cb
# good: [b024793188002b9eed452b5f6a04d45003ed5772] staging: rtl8723au:
phy_SsPwrSwitch92CU() was never called with bRegSSPwrLvl != 1
git bisect good b024793188002b9eed452b5f6a04d45003ed5772
# good: [66dcff86ba40eebb5133cccf450878f2bba102ef] Merge tag 'for-linus' of
git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect good 66dcff86ba40eebb5133cccf450878f2bba102ef
# bad: [88a57667f2990f00b019d46c8426441c9e516d51] Merge branch
'perf-urgent-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 88a57667f2990f00b019d46c8426441c9e516d51
# good: [0ec28c37c21a2b4393692e832e11a7573ac545e2] Merge tag 'media/v3.19-2'
of git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
git bisect good 0ec28c37c21a2b4393692e832e11a7573ac545e2
# good: [c0f486fde3f353232c1cc2fd4d62783ac782a467] Merge tag
'pm+acpi-3.19-rc1-2' of
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect good c0f486fde3f353232c1cc2fd4d62783ac782a467
# bad: [34b85e3574424beb30e4cd163e6da2e2282d2683] Merge tag 'powerpc-3.19-2'
of git://git.kernel.org/pub/scm/linux/kernel/git/mpe/linux
git bisect bad 34b85e3574424beb30e4cd163e6da2e2282d2683
# good: [64ec45bff6b3dade2643ed4c0f688a15ecf46ea2] Merge tag 'for_linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
git bisect good 64ec45bff6b3dade2643ed4c0f688a15ecf46ea2
Running git bisect bad gives this:
$ git bisect bad
Bisecting: 6 revisions left to test after this (roughly 3 steps)
[ec2aef5a8d3c14272f7a2d29b34f1f8e71f2be5b] power/perf/hv-24x7: Use
kmem_cache_free() instead of kfree
Running git bisect bad HEAD instead gives this:
$ git bisect bad HEAD
Bisecting: a merge base must be tested
[56548fc0e86cb9156af7a7e1f15ba78f251dafaf] powerpc/powernv: Return to cpu
offline loop when finished in KVM guest
This is git 2.2.1.
I think it is a very old bug.
The following patch should fix it:
diff --git a/git-bisect.sh b/git-bisect.sh
index 6cda2b5..26a336a 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -200,7 +200,8 @@ is_expected_rev() {
check_expected_revs() {
for _rev in $@; do
- if ! is_expected_rev $_rev
+ _parsed=$(git rev-parse --verify $_rev)
+ if ! is_expected_rev $_parsed
then
rm -f $GIT_DIR/BISECT_ANCESTORS_OK
rm -f $GIT_DIR/BISECT_EXPECTED_REV
I will send a proper patch later.
Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Supporting a few more usecases with remote helpers
Mike Hommey m...@glandium.org writes: 1. I think it should, as long as the given sha1 is reachable from the public heads, but that's offtopic here. Sounds vaguely familiar: http://thread.gmane.org/gmane.comp.version-control.git/178814/focus=179007 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/1] http: Add Accept-Language header if possible
Changes since v5 From Junio C Hamano's review: * The tests use `ls-remote` instead of `clone` for tests; I copied the test code from ba8e63dc30a80656fddc616f714fb217ad220c04. * Set cached_accept_langauge to NULL after free it. From Eric Sunshine's review: * get_accept_language() returns a pointer to const char instead of strbuf; the type of cached_accept_language also has been changed to char* from strbuf* * write_accept_language(), which is extracted from get_accept_language(), respects MAX_SIZE_OF_HEADER. * The for-loop in write_accept_language() works correctly if lang_begin points an empty string. From Jeff King's advice: * get_preferred_languages() considers LC_MESSAGES only if NO_GETTEXT is not defined. * Remove the tests for LC_MESSAGES, LANG and LC_ALL. Yi EungJun (1): http: Add Accept-Language header if possible http.c | 173 + remote-curl.c | 2 + t/t5550-http-fetch-dumb.sh | 32 + 3 files changed, 207 insertions(+) -- 2.2.0.375.gcd18ce6.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/1] http: Add Accept-Language header if possible
From: Yi EungJun eungjun...@navercorp.com
Add an Accept-Language header which indicates the user's preferred
languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.
Examples:
LANGUAGE= -
LANGUAGE=ko:en - Accept-Language: ko, en;q=0.9, *;q=0.1
LANGUAGE=ko LANG=en_US.UTF-8 - Accept-Language: ko, *;q=0.1
LANGUAGE= LANG=en_US.UTF-8 - Accept-Language: en-US, *;q=0.1
This gives git servers a chance to display remote error messages in
the user's preferred language.
Limit the number of languages to 1,000 because q-value must not be
smaller than 0.001, and limit the length of Accept-Language header to
4,000 bytes for some HTTP servers which cannot accept such long header.
Signed-off-by: Yi EungJun eungjun...@navercorp.com
---
http.c | 173 +
remote-curl.c | 2 +
t/t5550-http-fetch-dumb.sh | 32 +
3 files changed, 207 insertions(+)
diff --git a/http.c b/http.c
index 040f362..7a77708 100644
--- a/http.c
+++ b/http.c
@@ -68,6 +68,8 @@ static struct curl_slist *no_pragma_header;
static struct active_request_slot *active_queue_head;
+static char *cached_accept_language;
+
size_t fread_buffer(char *ptr, size_t eltsize, size_t nmemb, void *buffer_)
{
size_t size = eltsize * nmemb;
@@ -515,6 +517,11 @@ void http_cleanup(void)
cert_auth.password = NULL;
}
ssl_cert_password_required = 0;
+
+ if (cached_accept_language) {
+ free(cached_accept_language);
+ cached_accept_language = NULL;
+ }
}
struct active_request_slot *get_active_slot(void)
@@ -986,6 +993,166 @@ static void extract_content_type(struct strbuf *raw,
struct strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
}
+/*
+ * Guess the user's preferred languages from the value in LANGUAGE environment
+ * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
+ *
+ * The result can be a colon-separated list like ko:ja:en.
+ */
+static const char *get_preferred_languages(void)
+{
+ const char *retval;
+
+ retval = getenv(LANGUAGE);
+ if (retval *retval)
+ return retval;
+
+#ifndef NO_GETTEXT
+ retval = setlocale(LC_MESSAGES, NULL);
+ if (retval *retval
+ strcmp(retval, C)
+ strcmp(retval, POSIX))
+ return retval;
+#endif
+
+ return NULL;
+}
+
+static void write_accept_language(struct strbuf *buf)
+{
+ const char *lang_begin, *pos;
+ int q, max_q;
+ int num_langs;
+ int decimal_places;
+ const int CODESET_OR_MODIFIER = 1;
+ const int LANGUAGE_TAG = 2;
+ const int SEPARATOR = 3;
+ int is_q_factor_required = 0;
+ int parse_state = 0;
+ char q_format[32];
+ /*
+* MAX_LANGS must not be larger than 1,000. If it is larger than that,
+* q-value will be smaller than 0.001, the minimum q-value the HTTP
+* specification allows [1].
+*
+* [1]: http://tools.ietf.org/html/rfc7231#section-5.3.1
+*/
+ const int MAX_LANGS = 1000;
+ const int MAX_SIZE_OF_HEADER = 4000;
+ const int MAX_SIZE_OF_ASTERISK_ELEMENT = 11; /* for , *;q=0.001 */
+ int last_size = 0;
+
+ lang_begin = get_preferred_languages();
+
+ /* Don't add Accept-Language header if no language is preferred. */
+ if (!lang_begin)
+ return;
+
+ /* Count number of preferred lang_begin to decide precision of
q-factor. */
+ for (num_langs = 1, pos = lang_begin; *pos; pos++)
+ if (*pos == ':')
+ num_langs++;
+
+ /* Decide the precision for q-factor on number of preferred lang_begin.
*/
+ num_langs += 1; /* for '*' */
+
+ if (MAX_LANGS num_langs)
+ num_langs = MAX_LANGS;
+
+ for (max_q = 1, decimal_places = 0;
+ max_q num_langs;
+ decimal_places++, max_q *= 10)
+ ;
+
+ sprintf(q_format, ;q=0.%%0%dd, decimal_places);
+
+ q = max_q;
+
+ strbuf_addstr(buf, Accept-Language: );
+
+ /*
+* Convert a list of colon-separated locale values [1][2] to a list of
+* comma-separated language tags [3] which can be used as a value of
+* Accept-Language header.
+*
+* [1]: http://pubs.opengroup.org/onlinepubs/007908799/xbd/envvar.html
+* [2]:
http://www.gnu.org/software/libc/manual/html_node/Using-gettextized-software.html
+* [3]: http://tools.ietf.org/html/rfc7231#section-5.3.5
+*/
+ for (pos = lang_begin; ; pos++) {
+ if (!*pos || *pos == ':') {
+ if (is_q_factor_required) {
+ /* Put a q-factor only if it is less than 1.0.
*/
+ if (q max_q)
+ strbuf_addf(buf, q_format, q);
+
+
[PATCH 0/2] Let `git remote add` play nicer with url.url.insteadOf
Anastas Dancha reported that it is not possible to add a remote when there is already a url.url.insteadOf = nick setting in $HOME/.gitconfig. While it makes sense to prevent surprises when a user adds a remote and it fetches from somewhere completely different, it makes less sense to prevent adding a remote when it is actually the same that was specified in the config. Therefore we add just another check that let's `git remote add` continue when the existing remote's URL is identical to the specified one. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de Johannes Schindelin (2): git remote add: allow re-adding remotes with the same URL Add a regression test for 'git remote add existing same-url' builtin/remote.c | 3 ++- t/t5505-remote.sh | 5 + 2 files changed, 7 insertions(+), 1 deletion(-) -- 2.0.0.rc3.9669.g840d1f9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Add a regression test for 'git remote add existing same-url'
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- t/t5505-remote.sh | 5 + 1 file changed, 5 insertions(+) diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index ac79dd9..17c6330 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -1113,4 +1113,9 @@ test_extra_arg set-url origin newurl oldurl # prune takes any number of args # update takes any number of args +test_expect_success 'add remote matching the insteadOf URL' ' + git config url@example.com.insteadOf backup + git remote add backup x...@example.com +' + test_done -- 2.0.0.rc3.9669.g840d1f9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] git remote add: allow re-adding remotes with the same URL
When adding a remote, we make sure that the remote does not exist already. For convenience, we allow re-adding remotes with the same URLs. This also handles the case that there is an [url ...] insteadOf setting in the config. It might seem like a mistake to compare against remote-url[0] without verifying that remote-url_nr =1, but at this point a missing URL has been filled by the name already, therefore url_nr cannot be zero. Noticed by Anastas Dancha. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- builtin/remote.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/remote.c b/builtin/remote.c index 46ecfd9..9168c83 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -180,7 +180,8 @@ static int add(int argc, const char **argv) url = argv[1]; remote = remote_get(name); - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || + if (remote (remote-url_nr 1 || (strcmp(name, remote-url[0]) + strcmp(url, remote-url[0])) || remote-fetch_refspec_nr)) die(_(remote %s already exists.), name); -- 2.0.0.rc3.9669.g840d1f9 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-prompt: preserve command exit status
Tony Finch d...@dotat.at writes:
Signed-off-by: Tony Finch d...@dotat.at
---
contrib/completion/git-prompt.sh | 4
1 file changed, 4 insertions(+)
diff --git a/contrib/completion/git-prompt.sh
b/contrib/completion/git-prompt.sh
index c5473dc..5fe69d0 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -288,6 +288,7 @@ __git_eread ()
# In this mode you can request colored hints using
GIT_PS1_SHOWCOLORHINTS=true
__git_ps1 ()
{
+ local exit=$?
local pcmode=no
local detached=no
local ps1pc_start='\u@\h:\w '
@@ -511,4 +512,7 @@ __git_ps1 ()
else
printf -- $printf_format $gitstring
fi
+
+ # preserve exit status
+ return $exit
}
H. I thought The patch trivially makes sense! Why didn't
anybody notice this before?!?, but then noticed that I never
suffered from an obvious consequence from the current lack of the
exit-code-preserving:
: gitster git.git/master; echo $PS1
: \h \W$(__git_ps1 /%s);
: gitster git.git/master; false
: gitster git.git/master; echo $?
1
And it does not seem that it is needed, at least for my use
pattern:
: gitster git.git/master; ps1func () { echo What Now: ; exit 8; }
: gitster git.git/master; PS1='$(ps1func)'
What Now: echo $?
0
What Now: (exit 6)
What Now: echo $?
6
Also it does not seem that it is needed for the other style to use
PROMPT_COMMAND:
: gitster git.git/master; sh
$ . contrib/completion/git-prompt.sh
$ PROMPT_COMMAND='__git_ps1 \h \W ; '
gitster git.git (master); (exit 13)
gitster git.git (master); echo $?
13
gitster git.git (master); true
gitster git.git (master); echo $?
0
So, what are you fixing? In other words, please describe how it
fails in the log message.
Puzzled...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/18] Introduce fsck options
Hi Junio,
On Wed, 10 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
Subject: Re: [PATCH 01/18] Introduce fsck options
please make it easier to cluster and spot the series in the eventual
shortlog by giving a common prefix to the patches, e.g.
fsck: introduce fsck_options struct
I use the fsck: prefix consistently now.
+static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
+static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
Is it a good idea to allow walker to be strict but obj verifier to
be not (or vice versa)? I am wondering why this is not a single
struct with two callback function pointers.
Unfortunately not. There are two different walkers used, and in fact,
fsck_walk_options() is only used to walk the objects, not to fsck them.
Now, I could use only one struct and set the walker, but that is not
thread-safe, and while code is not threaded yet AFAICT, it might be in the
future. That is why I decided to be rather safe than sorry. If you want it
differently, please just say the word, I will make it so.
+struct fsck_options {
+ fsck_walk_func walk;
+ fsck_error error_func;
+ int strict:1;
A signed 1-bit-wide bitfield can hold its sign-bit and nothing else,
no?
unsigned strict:1;
Oops. Right. For some reason, it worked here, though... Fixed!
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/18] Introduce fsck options
Johannes Schindelin johannes.schinde...@gmx.de writes: Is it a good idea to allow walker to be strict but obj verifier to be not (or vice versa)? I am wondering why this is not a single struct with two callback function pointers. Unfortunately not. There are two different walkers used, and in fact, fsck_walk_options() is only used to walk the objects, not to fsck them. Now, I could use only one struct and set the walker, but that is not thread-safe, and while code is not threaded yet AFAICT, it might be in the future. That is why I decided to be rather safe than sorry. If you want it differently, please just say the word, I will make it so. Thanks for explaining; I just found that the reason behind the design choice was unclear and wanted to know. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] git remote add: allow re-adding remotes with the same URL
Johannes Schindelin johannes.schinde...@gmx.de writes: When adding a remote, we make sure that the remote does not exist already. For convenience, we allow re-adding remotes with the same URLs. This also handles the case that there is an [url ...] insteadOf setting in the config. It might seem like a mistake to compare against remote-url[0] without verifying that remote-url_nr =1, but at this point a missing URL has been filled by the name already, therefore url_nr cannot be zero. Noticed by Anastas Dancha. Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de --- builtin/remote.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/remote.c b/builtin/remote.c index 46ecfd9..9168c83 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -180,7 +180,8 @@ static int add(int argc, const char **argv) url = argv[1]; remote = remote_get(name); - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || + if (remote (remote-url_nr 1 || (strcmp(name, remote-url[0]) + strcmp(url, remote-url[0])) || remote-fetch_refspec_nr)) die(_(remote %s already exists.), name); When we need to fold an overlong line, it is easier to read if the line is folded at an operator with higher precedence, i.e. this line if (A (B || (C D) || E)) folded like this if (A (B || (C D) || E)) is harder to read than when folded like this if (A (B || (C D) || E)) So, it is an error if we have remote and if (1) URL for the remote is defined already twice or more; or (2) we are adding a nickname (i.e. not a URL) and it is different from what we already have; or (3) we already have fetch_refspec The way I read the log message's rationale was that this is to allow replacing an existing remote's URL; wouldn't checking the existence of fetch_refspec go against that goal? Puzzled. Either the code is wrong or I am mislead by the explanation in the log. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] Documentation/SubmittingPatches: Explain the rationale of git notes
Stefan Beller sbel...@google.com writes: This adds an explanation of why you want to have the --notes option given to git format-patch. Signed-off-by: Stefan Beller sbel...@google.com --- Notes: with optionally update Documentation/SubmittingPatches to point at the file. That file actually talks about notes already. I am sending two patches touching that file, one of them explaining the --notes workflow rationale and one of them just changing white spaces. A few weeks ago I wanted to patch format-patch to remove change ids. This is not needed any more for the git workflow as I disabled them and do not upload any patches to an optional Gerrit code review server anymore. I do like the workflow using --notes as well from a developers perspective as I take literally notes for my own sanity. I wonder if I should add a config format.notes = [default-off, always, on-if-non-empty] so I don't need always add --notes manually to the command line. Thanks, Stefan Documentation/SubmittingPatches | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index fa71b5f..16b5d65 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -176,7 +176,11 @@ message starts, you can put a From: line to name that person. You often want to add additional explanation about the patch, other than the commit message itself. Place such cover letter material between the three dash lines and the diffstat. Git-notes -can also be inserted using the `--notes` option. +can also be inserted using the `--notes` option. If you are one Because the previous sentence is talking about the cover letter to describe the overall series, it probably is a good idea to add after the three-dashes of each patch after ... using the notes option for clarity, perhaps? +of those developers who cannot write perfect code the first time +and need multiple iterations of review and discussion, you are +encouraged to use the notes to describe the changes between the +different versions of a patch. Perhaps s/you are encouraged to/you may want to/? It might be better to be even more explicit, e.g. you may want to keep track of the changes between the versions using the notes and then use `--notes` when preparing the series for submission? Do not attach the patch as a MIME attachment, compressed or not. Do not let your e-mail client send quoted-printable. Do not let Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Documentation/SubmittingPatches: unify whitespace/tabs for the DCO
Stefan Beller sbel...@google.com writes: The Developers Certificate of Origin has a mixture of tabs and white spaces which is annoying to view if your editor explicitly views white space characters. Signed-off-by: Stefan Beller sbel...@google.com --- Notes: I'll also try to send it upstream to linux. Thanks. Documentation/SubmittingPatches | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 16b5d65..6124f34 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -258,15 +258,15 @@ pretty simple: if you can certify the below: person who certified (a), (b) or (c) and I have not modified it. - (d) I understand and agree that this project and the contribution - are public and that a record of the contribution (including all - personal information I submit with it, including my sign-off) is - maintained indefinitely and may be redistributed consistent with - this project or the open source license(s) involved. +(d) I understand and agree that this project and the contribution +are public and that a record of the contribution (including all +personal information I submit with it, including my sign-off) is +maintained indefinitely and may be redistributed consistent with +this project or the open source license(s) involved. then you just add a line saying - Signed-off-by: Random J Developer ran...@developer.example.org +Signed-off-by: Random J Developer ran...@developer.example.org This line can be automatically added by Git if you run the git-commit command with the -s option. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] clean: typo fixed
Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/5] update_unicode.sh: set UNICODE_DIR only once
dev+...@drbeat.li writes:
From: Beat Bolli dev+...@drbeat.li
The value is the same on both uniset invocations, so Don't Repeat
Yourself applies.
Since we're in a subshell already, there's no need to unset UNICODE_DIR
at the end.
Strictly speaking, you are not introducing your own subshell to
prevent the environment from leaking (i.e. you used {...} not
(...) in the previous step). The reason you can do this is
because the generation of UNICODEWIDTH_H file is the last thing in
the subshell.
I'll reword it to Since this is done as the last command, ...
Thanks.
Signed-off-by: Beat Bolli dev+...@drbeat.li
---
update_unicode.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/update_unicode.sh b/update_unicode.sh
index c1c876c..bed8916 100755
--- a/update_unicode.sh
+++ b/update_unicode.sh
@@ -27,12 +27,13 @@ fi
fi
make
) {
+ UNICODE_DIR=. export UNICODE_DIR
echo static const struct interval zero_width[] = {
- UNICODE_DIR=. ./uniset/uniset --32 cat:Me,Mn,Cf +
U+1160..U+11FF - U+00AD |
+ ./uniset/uniset --32 cat:Me,Mn,Cf + U+1160..U+11FF - U+00AD |
grep -v plane
echo };
echo static const struct interval double_width[] = {
- UNICODE_DIR=. ./uniset/uniset --32 eaw:F,W
+ ./uniset/uniset --32 eaw:F,W
echo };
} $UNICODEWIDTH_H
)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-prompt: preserve value of $? inside shell prompt
If you have a prompt which displays the command exit status,
__git_ps1 without this change corrupts it, although it has
the correct value in the parent shell:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(__git_ps1) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 0 $ echo $?
1
~/src/git (master) 0 $
There is a slightly ugly workaround:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(x=$?; __git_ps1; exit $x) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 1 $
This change makes the workaround unnecessary.
Signed-off-by: Tony Finch d...@dotat.at
---
contrib/completion/git-prompt.sh | 4
1 file changed, 4 insertions(+)
I hope that explains it properly :-)
diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index c5473dc..5fe69d0 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -288,6 +288,7 @@ __git_eread ()
# In this mode you can request colored hints using GIT_PS1_SHOWCOLORHINTS=true
__git_ps1 ()
{
+ local exit=$?
local pcmode=no
local detached=no
local ps1pc_start='\u@\h:\w '
@@ -511,4 +512,7 @@ __git_ps1 ()
else
printf -- $printf_format $gitstring
fi
+
+ # preserve exit status
+ return $exit
}
--
2.2.1.68.g56d9796
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] receive-pack: move execute_commands_non_atomic before execute_commands
Stefan Beller sbel...@google.com writes: Notes: This patch is new with v6 of the series As execute_commands_non_atomic is larger than execute_commands, the diff is not moving around execute_commands_non_atomic, but execute_commands. ;-) Next time perhaps try --patience to decide between with and without which one reads better? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv6 0/7] atomic pushes
Will queue; thanks for a pleasant read. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Bad file descriptor on filtering empty files
William Throwe w...@cornell.edu writes:
In git 2.2.0 (also tested on 2.2.0.65.g9abc44b), if an external
smudge/clean filter is called on an empty file git reports something
like:
error: copy-fd: read returned Bad file descriptor
error: cannot feed the input to external filter cat
error: external filter cat failed
Test case:
mkdir bug
cd bug
git init
git config filter.cat.clean cat
git config filter.cat.smudge cat
echo '* filter=cat' .gitattributes
touch a
git add a
This started in 9035d75a2be9d80d82676504d69553245017f6d4, which
introduced the possible call to copy_fd in code called from
apply_filter. It appears that NULL as the src argument to apply_filter
is being used both as a sentinel value to indicate that the fd should be
used instead and also as a representation of the contents of an empty
file. I suggest switching to using fd == -1 as the sentinel as shown in
the patch below.
Thanks,
Will
William, thanks for raising this issue.
Steffen, comments?
diff --git a/convert.c b/convert.c
index 9a5612e..0509ac1 100644
--- a/convert.c
+++ b/convert.c
@@ -355,7 +355,7 @@ static int filter_buffer_or_fd(int in, int out, void
*data)
sigchain_push(SIGPIPE, SIG_IGN);
- if (params-src) {
+ if (params-fd == -1) {
write_err = (write_in_full(child_process.in, params-src,
params-size) 0);
} else {
write_err = copy_fd(params-fd, child_process.in);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: add --irreversible-delete for format-patch
Eric Wong normalper...@yhbt.net writes: Normally I would use -D, but send-email (which normally passes options to format-patch) interprets the -D as a case-insensitive abbreviation for --dry-run, preventing format-patch from seeing -D. Is this nonstandard option that is designed to produce an inapplicable patch so widely used to warrant a completion? I'd actually understand it if this were to complete git show and friends, but not for format-patch. I'd actually think we might be better off forbidding its use for the format-patch command (but not for other commands in the git log family), not just at the completion level but at the command line argument parser level. Hmph... Signed-off-by: Eric Wong normalper...@yhbt.net --- Case-insensitivity strikes again! : What a wacky default for Getopt::Long... And it's probably too late for us to disable case-insensitivity in CLI parsing for send-email. contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 2fece98..41d8ff8 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1257,6 +1257,7 @@ __git_format_patch_options= --not --all --cover-letter --no-prefix --src-prefix= --dst-prefix= --inline --suffix= --ignore-if-in-upstream --subject-prefix= --output-directory --reroll-count --to= --quiet --notes + --irreversible-delete _git_format_patch () -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
The awk statements previously used in this test weren't compatible
with the native versions of awk on Solaris:
echo dir | /bin/awk -v c=0 '$1 {++c} END {print c}'
awk: syntax error near line 1
awk: bailing out near line 1
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Instead of modifying the awk code to work, use wc -w instead as that
is both adequate and simpler.
Signed-off-by: Ben Walton bdwal...@gmail.com
---
t/t0090-cache-tree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 067f4c6..f2b1c9c 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
# We want to count only foo because it's the only direct child
subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq)
- subtree_count=$(echo $subtrees|awk -v c=0 '$1 {++c} END {print c}')
+ subtree_count=$(echo $subtrees|wc -w)
entries=$(git ls-files|wc -l)
printf SHA $dir (%d entries, %d subtrees)\n $entries
$subtree_count
for subtree in $subtrees
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: add --irreversible-delete for format-patch
Junio C Hamano gits...@pobox.com wrote: Eric Wong normalper...@yhbt.net writes: Normally I would use -D, but send-email (which normally passes options to format-patch) interprets the -D as a case-insensitive abbreviation for --dry-run, preventing format-patch from seeing -D. Is this nonstandard option that is designed to produce an inapplicable patch so widely used to warrant a completion? I'd actually understand it if this were to complete git show and friends, but not for format-patch. I'd actually think we might be better off forbidding its use for the format-patch command (but not for other commands in the git log family), not just at the completion level but at the command line argument parser level. Hmph... I was actually hoping Paul would resurrect his work on getting apply to understand --irreversible-delete: http://mid.gmane.org/1343939748-12256-1-git-send-email-paul.gortma...@windriver.com ($gmane/202789) I find this option useful to reduce mail traffic for others to review changes which are already pushed to a public repo. Signed-off-by: Eric Wong normalper...@yhbt.net --- Case-insensitivity strikes again! : What a wacky default for Getopt::Long... And it's probably too late for us to disable case-insensitivity in CLI parsing for send-email. contrib/completion/git-completion.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 2fece98..41d8ff8 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1257,6 +1257,7 @@ __git_format_patch_options= --not --all --cover-letter --no-prefix --src-prefix= --dst-prefix= --inline --suffix= --ignore-if-in-upstream --subject-prefix= --output-directory --reroll-count --to= --quiet --notes + --irreversible-delete _git_format_patch () -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 1/1] http: Add Accept-Language header if possible
Yi EungJun semtlen...@gmail.com writes:
From: Yi EungJun eungjun...@navercorp.com
Add an Accept-Language header which indicates the user's preferred
languages defined by $LANGUAGE, $LC_ALL, $LC_MESSAGES and $LANG.
Examples:
LANGUAGE= -
LANGUAGE=ko:en - Accept-Language: ko, en;q=0.9, *;q=0.1
LANGUAGE=ko LANG=en_US.UTF-8 - Accept-Language: ko, *;q=0.1
LANGUAGE= LANG=en_US.UTF-8 - Accept-Language: en-US, *;q=0.1
This gives git servers a chance to display remote error messages in
the user's preferred language.
Limit the number of languages to 1,000 because q-value must not be
smaller than 0.001, and limit the length of Accept-Language header to
4,000 bytes for some HTTP servers which cannot accept such long header.
Signed-off-by: Yi EungJun eungjun...@navercorp.com
Overall, this one is a much more pleasant read than the previous
rounds.
@@ -986,6 +993,166 @@ static void extract_content_type(struct strbuf *raw,
struct strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
}
+/*
+ * Guess the user's preferred languages from the value in LANGUAGE
environment
+ * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
+ *
+ * The result can be a colon-separated list like ko:ja:en.
+ */
+static const char *get_preferred_languages(void)
+{
+ const char *retval;
+
+ retval = getenv(LANGUAGE);
+ if (retval *retval)
+ return retval;
+
+#ifndef NO_GETTEXT
+ retval = setlocale(LC_MESSAGES, NULL);
+ if (retval *retval
+ strcmp(retval, C)
+ strcmp(retval, POSIX))
+ return retval;
+#endif
A tangent.
I wonder if we want to have something silly like this:
#ifndef NO_GETTEXT
#define setlocale(x, y) NULL /* or C??? */
#endif
in a common header (e.g. gettext.h) to avoid sprinkling #ifdefs in
our code. While I do not think we call setlocale() that often to
warrant such a trick, we already do something very similar to make
git_setup_gettext() a no-op in NO_GETTEXT builds in that header
file, and the change in this patch to remote-curl.c does take
advantage of it already, so...
+static void write_accept_language(struct strbuf *buf)
+{
+ const char *lang_begin, *pos;
+ int q, max_q;
+ int num_langs;
+ int decimal_places;
+ const int CODESET_OR_MODIFIER = 1;
+ const int LANGUAGE_TAG = 2;
+ const int SEPARATOR = 3;
Another tangent, but I think we tend to use either #define or enum
for constants, not const int, in our codebase, for symbolic
constants. In order to define a set of symbolic constants limited
to a function scope, the const int way may be nicer than the other
two methods we have traditionally used. Perhaps we should promote
such use more widely, write our new code following this example, and
migrate existing ones over time? I dunno.
...
+ /* Decide the precision for q-factor on number of preferred lang_begin.
*/
+ num_langs += 1; /* for '*' */
+
+ if (MAX_LANGS num_langs)
+ num_langs = MAX_LANGS;
+
+ for (max_q = 1, decimal_places = 0;
+ max_q num_langs;
+ decimal_places++, max_q *= 10)
+ ;
So, if we have 10 languages (num_langs == 10), decimal_places
becomes 1, max_q becomes 10 ...
+ sprintf(q_format, ;q=0.%%0%dd, decimal_places);
... and we will use q=0.%01d as the format. This is OK because
the first one is given without q= so we use the format only nine
times, counting down from 0.9 to 0.1 in 0.1 increments.
Sounds OK to me (I always miscount this kind of loop and need to
think aloud while doing a sanity check).
+ for (pos = lang_begin; ; pos++) {
+ if (!*pos || *pos == ':') {
+ if (is_q_factor_required) {
+ /* Put a q-factor only if it is less than 1.0.
*/
+ if (q max_q)
+ strbuf_addf(buf, q_format, q);
+
+ if (q 1)
+ q--;
When does this if statement not trigger? It seems to me that it
will stop decrementing only if you have very many languages (e.g.
num_langs was clipped to MAX_LANGS), and at that point you would not
want to scan and add more languages---is there a reason why you keep
going in such a case and not break out of the loop, i.e.
if (q-- 1)
break;
or something like that?
+ ...
+ if (buf-len MAX_SIZE_OF_HEADER -
MAX_SIZE_OF_ASTERISK_ELEMENT) {
+ strbuf_remove(buf, last_size, buf-len - last_size);
+ break;
+ }
+
+ if (!*pos)
+ break;
Alternatively use one of these breaks when q goes below 1, perhaps?
+/*
+ * Get an Accept-Language header which indicates user's preferred languages.
+ *
+ * This function always return non-NULL string as strbuf_detach()
Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt
Tony Finch d...@dotat.at writes:
If you have a prompt which displays the command exit status,
__git_ps1 without this change corrupts it, although it has
the correct value in the parent shell:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(__git_ps1) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 0 $ echo $?
1
~/src/git (master) 0 $
There is a slightly ugly workaround:
~/src/git (master) 0 $ set | grep ^PS1
PS1='\w$(x=$?; __git_ps1; exit $x) $? \$ '
~/src/git (master) 0 $ false
~/src/git (master) 1 $
This change makes the workaround unnecessary.
Signed-off-by: Tony Finch d...@dotat.at
---
contrib/completion/git-prompt.sh | 4
1 file changed, 4 insertions(+)
I hope that explains it properly :-)
Yes. I wouldn't have spent 20 minutes experimenting with various
hypothetical use cases if the above were there in the first place.
Thanks. Will queue.
diff --git a/contrib/completion/git-prompt.sh
b/contrib/completion/git-prompt.sh
index c5473dc..5fe69d0 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -288,6 +288,7 @@ __git_eread ()
# In this mode you can request colored hints using
GIT_PS1_SHOWCOLORHINTS=true
__git_ps1 ()
{
+ local exit=$?
local pcmode=no
local detached=no
local ps1pc_start='\u@\h:\w '
@@ -511,4 +512,7 @@ __git_ps1 ()
else
printf -- $printf_format $gitstring
fi
+
+ # preserve exit status
+ return $exit
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git update-ref --stdin : too many open files
Loic Dachary l...@dachary.org writes: Hi, Steps to reproduce: $ git --version git version 1.9.1 $ wc -l /tmp/1 9090 /tmp/1 $ head /tmp/1 delete refs/pull/1/head create refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61 delete refs/pull/1/merge create refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e ... $ ulimit -n 1024 $ git update-ref --stdin /tmp/1 fatal: Unable to create /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too many open files $ head -20 /tmp/1 | git update-ref --stdin $ echo $? 0 The workaround is to increase ulimit -n git update-ref --stdin should probably close some files. Cheers Sounds like the recent ref update in a transaction issue to me. Stefan, want to take a look? I think we do need to keep the .lock files without renaming while in transaction, but we do not have to keep them open, so I suspect that a fix may be to split the commit function into two (one to close but not rename, the other to finalize by renaming) or something. Also the version of transaction series we have queued seem to lock these refs very late in the process, but as we discussed privately a few weeks ago, we would want to move the lock much earlier, when the first update is attempted. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/18] Provide a function to parse fsck message IDs
Hi Junio,
On Wed, 10 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
This function will be used in the next commits to allow the user to
ask fsck to handle specific problems differently, e.g. demoting certain
errors to warnings. It has to handle partial strings because we would
like to be able to parse, say, '--strict=missing-email=warn' command
lines.
To make the parsing robust, we generate strings from the enum keys, and we
will match both lower-case, dash-separated values as well as camelCased
ones (e.g. both missing-email and missingEmail will match the
MISSING_EMAIL key).
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
fsck.c | 32
1 file changed, 32 insertions(+)
diff --git a/fsck.c b/fsck.c
index 3cea034..05b146c 100644
--- a/fsck.c
+++ b/fsck.c
@@ -63,6 +63,38 @@ enum fsck_msg_id {
FSCK_MSG_MAX
};
+#define STR(x) #x
+#define MSG_ID_STR(x) STR(x),
+static const char *msg_id_str[FSCK_MSG_MAX + 1] = {
+ FOREACH_MSG_ID(MSG_ID_STR)
+ NULL
+};
I wondered what the ugly macro was in the previous step, but as a
way to keep these two lists in sync it makes sense.
I added a comment to the commit message.
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings
Hi Junio,
On Wed, 10 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
There are legacy repositories out there whose older commits and tags
have issues that prevent pushing them when 'receive.fsckObjects' is set.
One real-life example is a commit object that has been hand-crafted to
list two authors.
Often, it is not possible to fix those issues without disrupting the
work with said repositories, yet it is still desirable to perform checks
by setting `receive.fsckObjects = true`. This commit is the first step
to allow demoting specific fsck issues to mere warnings.
The function added by this commit parses a list of settings in the form:
missing-email=warn,bad-name=warn,...
Unfortunately, the FSCK_WARN/FSCK_ERROR flag is only really heeded by
git fsck so far, but other call paths (e.g. git index-pack --strict)
error out *always* no matter what type was specified. Therefore, we
need to take extra care to default to all FSCK_ERROR in those cases.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
fsck.c | 58 ++
fsck.h | 7 +--
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index 05b146c..9e6d70f 100644
--- a/fsck.c
+++ b/fsck.c
@@ -97,9 +97,63 @@ static int parse_msg_id(const char *text, int len)
int fsck_msg_type(enum fsck_msg_id msg_id, struct fsck_options *options)
{
+ if (options-strict_mode msg_id = 0 msg_id FSCK_MSG_MAX)
+ return options-strict_mode[msg_id];
+ if (options-strict)
+ return FSCK_ERROR;
return msg_id FIRST_WARNING ? FSCK_ERROR : FSCK_WARN;
}
Hmm, if you are later going to allow demoting (hopefully also promoting)
error to warn, etc., would the comparison between msg_id and FIRST_WARNING
make much sense?
A later patch indeed adds that option. The reason the comparison still
makes sense is that the pure infos do not return at all so far, but all of
the reported warnings are fatal in strict mode (i.e. when
receive.fsckObjects = true). In another later patch it is made possible to
promote even infos (such as 'missing tagger') to warnings or even errors,
and that is when the return FSCK_ERROR is changed to return msg_id
FIRST_INFO ? FSCK_ERROR : FSCK_WARN.
In other words, at some point wouldn't we be better off with
something like this
struct {
enum id;
const char *id_string;
enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
} possible_fsck_errors[];
I considered that, and Michael Haggerty also suggested that in a private
mail. However, I find that there is a clear hierarchy in the default
messages: fatal errors, errors, warnings and infos. This should be
reflected by the order IMHO.
But I guess it would make a lot of sense to insert those levels as special
enum values to make it harder to forget to adjust, say, #define
FIRST_WARNING FSCK_MSG_BAD_FILEMODE when introducing another warning that
sorts before said ID alphabetically. In other words, I think that we can
really afford to put something like
...
FUNC(UNKNOWN_TYPE) \
FUNC(ZERO_PADDED_DATE) \
FUNC(___WARNINGS) \
FUNC(BAD_FILEMODE) \
FUNC(EMPTY_NAME) \
...
at the price of making the parsing a little more complicated and wasting a
slight bit of more space for the msg_id_str array.
What do you think?
Ciao,
Dscho
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/18] Allow demoting errors to warnings via receive.fsck.key = warn
Hi Junio, On Wed, 10 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: For example, missing emails in commit and tag objects can be demoted to mere warnings with git config receive.fsck.missing-email warn No punctuations in the first and the last level of configuration variable names, please. I.e. s/missing-email/missingEmail/ or something. Fixed. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
Ben Walton bdwal...@gmail.com writes:
The awk statements previously used in this test weren't compatible
with the native versions of awk on Solaris:
echo dir | /bin/awk -v c=0 '$1 {++c} END {print c}'
awk: syntax error near line 1
awk: bailing out near line 1
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Instead of modifying the awk code to work, use wc -w instead as that
is both adequate and simpler.
Hmm, why wc -w not wc -l, though? Is somebody squashing a
one-elem-per-line output from ls-files onto a single line?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/18] fsck: handle multiple authors in commits specially
Hi Junio,
On Wed, 10 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
This problem has been detected in the wild, and is the primary reason
to introduce an option to demote certain fsck errors to warnings. Let's
offer to ignore this particular problem specifically.
...
+ while (skip_prefix(buffer, author , buffer)) {
+ err = report(options, commit-object,
FSCK_MSG_MULTIPLE_AUTHORS, invalid format - multiple 'author' lines);
+ if (err)
+ return err;
If we have an option to demote this to a warning, wouldn't we want
to do the same fsck_ident() on that secondary author line?
Good point! I changed the following to use fsck_ident() instead:
+ /* require_end_of_header() ensured that there is a newline */
+ buffer = strchr(buffer, '\n') + 1;
+ }
if (!skip_prefix(buffer, committer , buffer))
return report(options, commit-object,
FSCK_MSG_MISSING_COMMITTER, invalid format - expected 'committer' line);
err = fsck_ident(buffer, commit-object, options);
Ciao,
Dscho
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/18] Disallow demoting grave fsck errors to warnings
Hi Junio, On Wed, 10 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Some kinds of errors are intrinsically unrecoverable (e.g. errors while uncompressing objects). It does not make sense to allow demoting them to mere warnings. Makes sense, but the patch makes it look as if this is an oops, we should have done the list in patch 02/18 in this order from the beginning. Can we reorder the patches? I considered this already, but it would more be like a squash than a reordering. And when I squashed the patches, the story did not read as clearly to me as it does now. However, if you think this argument is too weak, I will squash them. Is it too weak? Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes:
In other words, at some point wouldn't we be better off with
something like this
struct {
enum id;
const char *id_string;
enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
} possible_fsck_errors[];
I considered that, and Michael Haggerty also suggested that in a private
mail. However, I find that there is a clear hierarchy in the default
messages: fatal errors, errors, warnings and infos.
I am glad I am not alone ;-)
These classes are ordered from more severe to less, but I do not
think it makes much sense to force the default view of if you
customize to demote a questionable Q that is classified as an error
by default as an warning, you must demote all the other ones that we
deem less serious than Q, which come earlier (or later---I do not
remember which) in our predefined list. So in that sense, I do not
consider that various kinds of questionables fsck can detect are
hierarchical at all.
I do agree that it makes it easier to code the initialization of
such an array to have up to this point we assign the level 'fatal
error' by default constants. Then the initialization can become
for (i = 0; i FIRST_WARN; i++)
possible_fsck_errors[i].error_level = FSCK_INFO;
while (i FIRST_ERROR)
possible_fsck_errors[i++].error_level = FSCK_WARN;
while (i ARRAY_SIZE(possible_fsck_errors))
possible_fsck_errors[i++].error_level = FSCK_ERROR;
or something. So I am not against the FIRST_WARNING constant at
all, but I find it very questionable in a fully customizable system
to use such a constant anywhere other than the initialization time.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/18] fsck: allow upgrading fsck warnings to errors
Hi Junio, On Wed, 10 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: The 'invalid tag name' and 'missing tagger entry' warnings can now be upgraded to errors by setting receive.fsck.invalid-tag-name and receive.fsck.missing-tagger-entry to 'error'. Hmm, why can't all warnings promotable to errors, or are the above two mentioned only as examples? Those were the only ones that were always shown as warnings but never treated as errors. There is a third one coming, as part of the patches that will let fsck warn about NTFS-incompatible file names, but I want to get this patch series integrated into git.git first. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
Ben Walton wrote:
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
Thanks. Weird. Does
awk -v c=0 '$1 != {++c} END {print c}'
work better?
[...]
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
# We want to count only foo because it's the only direct child
subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq)
- subtree_count=$(echo $subtrees|awk -v c=0 '$1 {++c} END {print c}')
+ subtree_count=$(echo $subtrees|wc -w)
entries=$(git ls-files|wc -l)
printf SHA $dir (%d entries, %d subtrees)\n $entries
$subtree_count
Some implementations of wc add a trailing space, causing
printf: 1 : invalid number
Using
printf SHA $dir (%d entries, %d subtrees)\n $entries $subtree_count
(with no quotes around $subtree_count) would avoid trouble, though
that's a little subtle.
Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
Junio C Hamano wrote:
Ben Walton bdwal...@gmail.com writes:
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Instead of modifying the awk code to work, use wc -w instead as that
is both adequate and simpler.
Hmm, why wc -w not wc -l, though? Is somebody squashing a
one-elem-per-line output from ls-files onto a single line?
The old code was trying to skip empty lines.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/18] fsck: allow upgrading fsck warnings to errors
Johannes Schindelin johannes.schinde...@gmx.de writes:
Hi Junio,
On Wed, 10 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
The 'invalid tag name' and 'missing tagger entry' warnings can now be
upgraded to errors by setting receive.fsck.invalid-tag-name and
receive.fsck.missing-tagger-entry to 'error'.
Hmm, why can't all warnings promotable to errors, or are the above
two mentioned only as examples?
Those were the only ones that were always shown as warnings but never
treated as errors.
Sorry but I don't quite understand this comment; I suspect the root
cause might be that we have different mental models on these
tweakable error severities.
Because I come from the school To these N kinds of events you can
independently assign different (i.e. info, warn, error) outcomes,
moving the FIRST_{INFO,WARNING,...} position in the array would only
affect what happens by default, never hindering the user's ability
to tweak (in other words, there is no linkage between now you can
tweak and the order of events in the list, the latter of which only
would affect what the default severity of each event is).
It appears that your design is from a different mental model and the
order and position in that list has more significance than what the
default severity of each event is but how much the severity can be
tweaked, or something, which I somehow find incomprehensible.
Puzzled...
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-prompt: preserve value of $? inside shell prompt
Junio C Hamano gits...@pobox.com wrote: Yes. I wouldn't have spent 20 minutes experimenting with various hypothetical use cases if the above were there in the first place. Sorry for wasting your time, and thanks for reviewing the patch. (I am so used to having $? in my prompt it took me ages to find and explain the problem too... Sigh!) Tony. -- f.anthony.n.finch d...@dotat.at http://dotat.at/ Trafalgar: Easterly 6 to gale 8 far southeast, otherwise northeasterly veering southeasterly 4 or 5. Slight or moderate, occasionally rough at first in south. Occasional drizzle. Good, occasionally moderate. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] git receive-pack: support excluding objects from fsck'ing
Hi Junio,
On Wed, 10 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
The optional new config option `receive.fsck.skip-list` specifies the path
to a file listing the names, i.e. SHA-1s, one per line, of objects that
are to be ignored by `git receive-pack` when `receive.fsckObjects = true`.
This is extremely handy in case of legacy repositories where it would
cause more pain to change incorrect objects than to live with them
(e.g. a duplicate 'author' line in an early commit object).
The intended use case is for server administrators to inspect objects
that are reported by `git push` as being too problematic to enter the
repository, and to add the objects' SHA-1 to a (preferably sorted) file
when the objects are legitimate, i.e. when it is determined that those
problematic objects should be allowed to enter the server.
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
---
builtin/receive-pack.c | 9 +++
fsck.c | 59
+++--
fsck.h | 2 ++
t/t5504-fetch-receive-strict.sh | 12 +
4 files changed, 80 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 111e514..5169f1f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -110,6 +110,15 @@ static int receive_pack_config(const char *var, const
char *value, void *cb)
return 0;
}
+ if (starts_with(var, receive.fsck.skip-list)) {
s/skip-list/skiplist/;
+ const char *path = is_absolute_path(value) ?
+ value : git_path(%s, value);
+ if (fsck_strict_mode.len)
+ strbuf_addch(fsck_strict_mode, ',');
+ strbuf_addf(fsck_strict_mode, skip-list=%s, path);
+ return 0;
+ }
+
if (starts_with(var, receive.fsck.)) {
if (fsck_strict_mode.len)
strbuf_addch(fsck_strict_mode, ',');
diff --git a/fsck.c b/fsck.c
index 154f361..00693f2 100644
--- a/fsck.c
+++ b/fsck.c
@@ -7,6 +7,7 @@
#include tag.h
#include fsck.h
#include refs.h
+#include sha1-array.h
#define FOREACH_MSG_ID(FUNC) \
/* fatal errors */ \
@@ -56,7 +57,9 @@
FUNC(ZERO_PADDED_FILEMODE) \
/* infos (reported as warnings, but ignored by default) */ \
FUNC(INVALID_TAG_NAME) \
- FUNC(MISSING_TAGGER_ENTRY)
+ FUNC(MISSING_TAGGER_ENTRY) \
+ /* special value */ \
+ FUNC(SKIP_LIST)
This feels like a kludge to me without comment on what special
value means. Does it mean this object has an error (which by
default is ignored) of being on the skip list? Should we be able
to optionally warn an object on the skip-list exists with the same
mechansim the rest of the series uses to tweak the error level?
I addressed both concerns – I hope... ;-)
Ciao,
Dscho
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Hi Junio, On Wed, 10 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: We already have support in `git receive-pack` to deal with some legacy repositories which have non-fatal issues. Let's make `git fsck` itself useful with such repositories, too, by allowing users to ignore known issues, or at least demote those issues to mere warnings. Example: `git -c fsck.missing-email=ignore fsck` would hide problems with missing emails in author, committer and tagger lines. Hopefully I do not have to repeat myself, but please do not have punctuations in the first and the last level of configuration variable names, i.e. fsck.missingEmail, not mising-email. I vetted the complete patch series and think I caught them all. Or do you want the error messages to also use camelCased IDs, i.e. warning in tag $tag: missingTaggerEntry: invalid format ... instead of warning in tag $tag: missing-tagger-entry: invalid format ... ? It is easy to do, but looks slightly uglier to this developer's eyes... Should these be tied to receive-pack ones in any way? E.g. if you set fsck.missingEmail to ignore, you do not have to do the same for receive and accept a push with the specific error turned off? Not a rhetorical question. I can see it argued both ways. The justification to defend the position of not tying these two I would have is so that I can be more strict to newer breakages (i.e. not accepting a push that introduce a new breakage by not ignoring with receive.fsck.*) while allowing breakages that are already present. The justification for the opposite position is to make it more convenient to write a consistent policy. Whichever way is chosen, we would want to see the reason left in the log message so that people do not have to wonder what the original motivation was when they decide if it is a good idea to change this part of the code. Hmm. I really tried very hard to separate the fsck.* from the receive.* settings because the two code paths already behave differently: many warnings reported (and ignored) by fsck are fatal errors when pushing with receive.fsckObjects=true. My understanding was that these differences are deliberate, and my interpretation was that the fsck and the receive settings were considered to be fundamentally different, even if the same fsck machinery performs the validation. If you agree, I would rephrase this line of argument and add it to the commit message. Do you agree? Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
Jonathan Nieder jrnie...@gmail.com writes:
Junio C Hamano wrote:
Ben Walton bdwal...@gmail.com writes:
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Instead of modifying the awk code to work, use wc -w instead as that
is both adequate and simpler.
Hmm, why wc -w not wc -l, though? Is somebody squashing a
one-elem-per-line output from ls-files onto a single line?
The old code was trying to skip empty lines.
Ahh, I misread the original.
Your suggestion to explicitly check $1 != makes sense to me now.
To be blunt, I do not have much sympathy to those who insist using
/usr/bin versions of various tools on Solaris that are overriden by
xpg variants, but it is somewhat disturbing that the one from xpg4
does not work.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings
Hi Junio,
On Mon, 22 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
In other words, at some point wouldn't we be better off with
something like this
struct {
enum id;
const char *id_string;
enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
} possible_fsck_errors[];
I considered that, and Michael Haggerty also suggested that in a private
mail. However, I find that there is a clear hierarchy in the default
messages: fatal errors, errors, warnings and infos.
I am glad I am not alone ;-)
These classes are ordered from more severe to less, but I do not
think it makes much sense to force the default view of if you
customize to demote a questionable Q that is classified as an error
by default as an warning, you must demote all the other ones that we
deem less serious than Q, which come earlier (or later---I do not
remember which) in our predefined list. So in that sense, I do not
consider that various kinds of questionables fsck can detect are
hierarchical at all.
Oh, but please understand that this hierarchy only applies to the default
settings. All of these settings can be overridden individually – and the
first override will initialize a full array with the default settings.
So the order really only plays a role for the defaults, no more.
I do agree that it makes it easier to code the initialization of
such an array to have up to this point we assign the level 'fatal
error' by default constants. Then the initialization can become
for (i = 0; i FIRST_WARN; i++)
possible_fsck_errors[i].error_level = FSCK_INFO;
while (i FIRST_ERROR)
possible_fsck_errors[i++].error_level = FSCK_WARN;
while (i ARRAY_SIZE(possible_fsck_errors))
possible_fsck_errors[i++].error_level = FSCK_ERROR;
or something. So I am not against the FIRST_WARNING constant at
all, but I find it very questionable in a fully customizable system
to use such a constant anywhere other than the initialization time.
This is indeed the case. The code we are discussing comes after the
if (options-strict_mode)
return options-strict_mode[msg_id];
In other words, once the overrides are in place, the default settings are
skipped entirely.
Ciao,
Dscho
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: Or do you want the error messages to also use camelCased IDs, i.e. warning in tag $tag: missingTaggerEntry: invalid format ... instead of warning in tag $tag: missing-tagger-entry: invalid format ... ? It is easy to do, but looks slightly uglier to this developer's eyes... Do you really need to know what I think? Can I say The latter is probably slightly better, but both look ugly to me? If we want a human readable message warning: tag object lacks tagger field '$tag' would be my preference. But I personally think it may not be necessary to give a pretty message that later can go through l10n here. If we take that position, it is just a machine-readble error token, so I'd say whichever way you find more readable is OK. Should these be tied to receive-pack ones in any way? E.g. if you set fsck.missingEmail to ignore, you do not have to do the same for receive and accept a push with the specific error turned off? Not a rhetorical question. I can see it argued both ways. The justification to defend the position of not tying these two I would have is so that I can be more strict to newer breakages (i.e. not accepting a push that introduce a new breakage by not ignoring with receive.fsck.*) while allowing breakages that are already present. The justification for the opposite position is to make it more convenient to write a consistent policy. Whichever way is chosen, we would want to see the reason left in the log message so that people do not have to wonder what the original motivation was when they decide if it is a good idea to change this part of the code. Hmm. I really tried very hard to separate the fsck.* from the receive.* settings because the two code paths already behave differently:... If you agree, I would rephrase this line of argument and add it to the commit message. Do you agree? Yeah, that reasoning sounds sensible. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes:
Hi Junio,
On Mon, 22 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
In other words, at some point wouldn't we be better off with
something like this
struct {
enum id;
const char *id_string;
enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
} possible_fsck_errors[];
I considered that, and Michael Haggerty also suggested that in a private
mail. However, I find that there is a clear hierarchy in the default
messages: fatal errors, errors, warnings and infos.
I am glad I am not alone ;-)
...
Oh, but please understand that this hierarchy only applies to the default
settings. All of these settings can be overridden individually – and the
first override will initialize a full array with the default settings.
But that means that the runtime needs to switch between two code
with and without override, no?
if (options-strict_mode)
return options-strict_mode[msg_id];
In other words, I think this is misleading and unnecessary
optimization for the full array allocation. A code that uses an
array of a struct like the above that Michael and I independently
suggested would initialize once with or without an override and then
at the runtime there is no if the array is there use it
conditional.
I do not know why Michael suggested the same thing, but the reason
why I prefer that arrangement is because I think it would be easier
to read and maintain.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/18] fsck: allow upgrading fsck warnings to errors
Hi Junio,
On Mon, 22 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
On Wed, 10 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
The 'invalid tag name' and 'missing tagger entry' warnings can now be
upgraded to errors by setting receive.fsck.invalid-tag-name and
receive.fsck.missing-tagger-entry to 'error'.
Hmm, why can't all warnings promotable to errors, or are the above
two mentioned only as examples?
Those were the only ones that were always shown as warnings but never
treated as errors.
Sorry but I don't quite understand this comment; I suspect the root
cause might be that we have different mental models on these
tweakable error severities.
Because I come from the school To these N kinds of events you can
independently assign different (i.e. info, warn, error) outcomes,
moving the FIRST_{INFO,WARNING,...} position in the array would only
affect what happens by default, never hindering the user's ability
to tweak (in other words, there is no linkage between now you can
tweak and the order of events in the list, the latter of which only
would affect what the default severity of each event is).
We agree on this mental model.
The only problem this patch tries to fix is that the warnings about a
missing tagger and about invalid tag names were never leading to an error.
They were purely printed, but then ignored. So what this patch does is to
add if (err) return err; handling for those two warnings.
As a consequence, the ordering of message IDs needs to be fixed because
the non-fatal warnings were ordered alphabetically before, but now the
non-fatal warnings are extracted so that we can give them the appropriate
FSCK_WARN by defauly – even in the git-receive-pack case.
In other words, the value assigned to those two warnings was completely
ignored before, which was the reason why it did not matter that we
assigned them to report FSCK_ERRORs in the git-receive-pack case before:
they were still only printed out and never stopped any tag from entering
the host's repository.
I could change the ordering in the patch that introduces the message IDs,
of course, but it would be even more puzzling if those two messages, of
all, were not ordered alphabetically with the others.
Ciao,
Dscho
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Hi Junio, On Mon, 22 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Or do you want the error messages to also use camelCased IDs, i.e. warning in tag $tag: missingTaggerEntry: invalid format ... instead of warning in tag $tag: missing-tagger-entry: invalid format ... ? It is easy to do, but looks slightly uglier to this developer's eyes... Do you really need to know what I think? Well, but yes ;-) Can I say The latter is probably slightly better, but both look ugly to me? Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though... If we want a human readable message warning: tag object lacks tagger field '$tag' would be my preference. But I personally think it may not be necessary to give a pretty message that later can go through l10n here. If we take that position, it is just a machine-readble error token, so I'd say whichever way you find more readable is OK. Okay, I will leave it as-is, then. Should these be tied to receive-pack ones in any way? E.g. if you set fsck.missingEmail to ignore, you do not have to do the same for receive and accept a push with the specific error turned off? Not a rhetorical question. I can see it argued both ways. The justification to defend the position of not tying these two I would have is so that I can be more strict to newer breakages (i.e. not accepting a push that introduce a new breakage by not ignoring with receive.fsck.*) while allowing breakages that are already present. The justification for the opposite position is to make it more convenient to write a consistent policy. Whichever way is chosen, we would want to see the reason left in the log message so that people do not have to wonder what the original motivation was when they decide if it is a good idea to change this part of the code. Hmm. I really tried very hard to separate the fsck.* from the receive.* settings because the two code paths already behave differently:... If you agree, I would rephrase this line of argument and add it to the commit message. Do you agree? Yeah, that reasoning sounds sensible. I added this paragraph: In the same spirit that `git receive-pack`'s usage of the fsck machinery differs from `git fsck`'s – some of the non-fatal warnings in `git fsck` are fatal with `git receive-pack` when receive.fsckObjects = true, for example – we strictly separate the fsck.* from the receive.fsck.* settings. Ciao, Dscho
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though... Ahh, OK, then dashed form would not work as a configuration variable names, so missingTaggerEntry would be the only usable option. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] receive-pack.c: add protocol support to negotiate atomic-push
On Fri, Dec 19, 2014 at 2:38 PM, Stefan Beller sbel...@google.com wrote: From: Ronnie Sahlberg sahlb...@google.com This adds support to the protocol between send-pack and receive-pack to * allow receive-pack to inform the client that it has atomic push capability * allow send-pack to request atomic push back. There is currently no setting in send-pack to actually request that atomic pushes are to be used yet. This only adds protocol capability not ability for the user to activate it. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Stefan Beller sbel...@google.com --- diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index 6d5424c..4f8a7bf 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -244,6 +245,14 @@ respond with the 'quiet' capability to suppress server-side progress reporting if the local progress reporting is also being suppressed (e.g., via `push -q`, or if stderr does not go to a tty). +atomic +-- + +If the server sends the 'atomic' capability it is capable of accepting +atomic pushes. If the pushing client requests this capability, the server +will update the refs in one atomic transaction. Either all refs are Not itself worth a re-send, but if you re-send for some other reason... one atomic still smacks of redundancy; an atomic sounds better. +updated or none. + allow-tip-sha1-in-want -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings
Hi Junio,
On Mon, 22 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
On Mon, 22 Dec 2014, Junio C Hamano wrote:
Johannes Schindelin johannes.schinde...@gmx.de writes:
In other words, at some point wouldn't we be better off with
something like this
struct {
enum id;
const char *id_string;
enum error_level { FSCK_PASS, FSCK_WARN, FSCK_ERROR };
} possible_fsck_errors[];
I considered that, and Michael Haggerty also suggested that in a private
mail. However, I find that there is a clear hierarchy in the default
messages: fatal errors, errors, warnings and infos.
I am glad I am not alone ;-)
...
Oh, but please understand that this hierarchy only applies to the default
settings. All of these settings can be overridden individually – and the
first override will initialize a full array with the default settings.
But that means that the runtime needs to switch between two code
with and without override, no?
if (options-strict_mode)
return options-strict_mode[msg_id];
In other words, I think this is misleading and unnecessary
optimization for the full array allocation. A code that uses an
array of a struct like the above that Michael and I independently
suggested would initialize once with or without an override and then
at the runtime there is no if the array is there use it
conditional.
I do not know why Michael suggested the same thing, but the reason
why I prefer that arrangement is because I think it would be easier
to read and maintain.
Well, I disagree that it would be easier to maintain, because it appears
to me that the clear hierarchy keeps things simple. For example if some
clearly fatal error is clustered with non-fatal ones due to alphabetical
ordering, it is much harder to spot when it is marked as a demoteable
error by mistake.
For example, try to spot the error here:
...
F(ALMOST_HAPPY, INFO) \
F(CANNOT_RECOVER, ERROR) \
F(COFFEE_IS_EMPTY, WARN) \
F(JUST_BEING_CHATTY, INFO) \
F(LIFE_IS_GOOD, INFO) \
F(MISSING_SOMETHING_VITAL, FATAL_ERROR) \
F(NEED_TO_SLEEP, WARN) \
F(SOMETHING_WENT_WRONG, ERROR) \
...
Personally, I find it very, very hard to spot that CANNOT_RECOVER is
marked as a mere ERROR instead of a FATAL_ERROR. Even if it is nicely
alphabetically ordered.
I will sleep over this, though. Maybe I can come up with a solution that
makes all three of us happy.
Ciao,
Dscho
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Hi Junio, On Mon, 22 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though... Ahh, OK, then dashed form would not work as a configuration variable names, so missingTaggerEntry would be the only usable option. Except that cunning me has made it so that both missing-tagger-entry *and* missingTaggerEntry work... Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] send-pack.c: add --atomic command line argument
On Fri, Dec 19, 2014 at 2:38 PM, Stefan Beller sbel...@google.com wrote:
From: Ronnie Sahlberg sahlb...@google.com
This adds support to send-pack to negotiate and use atomic pushes
iff the server supports it. Atomic pushes are activated by a new command
line flag --atomic.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index b564a77..b961e5a 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -282,6 +282,30 @@ free_return:
return update_seen;
}
+
+static int atomic_push_failure(struct send_pack_args *args,
+ struct ref *remote_refs,
+ struct ref *failing_ref)
+{
+ struct ref *ref;
+ /* Mark other refs as failed */
+ for (ref = remote_refs; ref; ref = ref-next) {
+ if (!ref-peer_ref !args-send_mirror)
+ continue;
+
+ switch (ref-status) {
+ case REF_STATUS_EXPECTING_REPORT:
+ ref-status = REF_STATUS_ATOMIC_PUSH_FAILED;
+ continue;
+ default:
+ ; /* do nothing */
+ }
+ }
+ error(atomic push failed for ref %s. status: %d\n,
+ failing_ref-name, failing_ref-status);
+ return -1;
Not itself worth a re-send, but if you do re-send for some other reason...
return error(...);
would be more idiomatic (as mentioned in the previous review).
+}
+
int send_pack(struct send_pack_args *args,
int fd[], struct child_process *conn,
struct ref *remote_refs,
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 16/18] fsck: support demoting errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: Hi Junio, On Mon, 22 Dec 2014, Junio C Hamano wrote: Johannes Schindelin johannes.schinde...@gmx.de writes: Of course you can say that! ;-) The problem these ugly messages try to solve is to give the user a hint which setting to change if they want to override the default behavior, though... Ahh, OK, then dashed form would not work as a configuration variable names, so missingTaggerEntry would be the only usable option. Except that cunning me has made it so that both missing-tagger-entry *and* missingTaggerEntry work... Then the missing-tagger-entry side needs to be dropped. The naming does not conform to the way how we name our officially supported configuration variables. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/18] Offer a function to demote fsck errors to warnings
Johannes Schindelin johannes.schinde...@gmx.de writes: For example, try to spot the error here: ... F(ALMOST_HAPPY, INFO) \ F(CANNOT_RECOVER, ERROR) \ F(COFFEE_IS_EMPTY, WARN) \ F(JUST_BEING_CHATTY, INFO) \ F(LIFE_IS_GOOD, INFO) \ F(MISSING_SOMETHING_VITAL, FATAL_ERROR) \ F(NEED_TO_SLEEP, WARN) \ F(SOMETHING_WENT_WRONG, ERROR) \ ... But that is not what is being suggested at all. I already said that FIRST_SOMETHING is fine as a measure to initialize, didn't I? I am only saying that if you have a place to store customized level, you should initialize that part with default levels and always look it up from that place at runtime. It is perfectly fine for the initialization step to take advantage of the ordering and FIRST_SOMETHING constants. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Dec 2014, #04; Mon, 22)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Quite a few topics have been merged to 'master' as the third batch for this cycle, on top of the recent case sensitive .Git fix that has been publicized very widely. The next release which is expected to be a small one is taking shape. 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 -- [Graduated to master] * cc/interpret-trailers-more (2014-11-10) 4 commits (merged to 'next' on 2014-12-15 at 77f6c6a) + trailer: add test with an old style conflict block + trailer: reuse ignore_non_trailer() to ignore conflict lines + commit: make ignore_non_trailer() non static + Merge branch 'jc/conflict-hint' into cc/interpret-trailers-more (this branch uses jc/conflict-hint.) git interpret-trailers learned to properly handle the Conflicts: block at the end. * ch/new-gpg-drops-rfc-1991 (2014-12-12) 4 commits (merged to 'next' on 2014-12-15 at 32d7d50) + tests: squelch noise from GPG machinery set-up + tests: replace binary GPG keyrings with ASCII-armored keys + tests: skip RFC1991 tests for gnupg 2.1 + tests: create gpg homedir on the fly Recent GPG changes the keyring format and drops support for RFC1991 formatted signatures, breaking our existing tests. * dm/compat-s-ifmt-for-zos (2014-12-04) 1 commit (merged to 'next' on 2014-12-15 at 0eb2fe6) + compat: convert modes to use portable file type values Long overdue departure from the assumption that S_IFMT is shared by everybody made in 2005. * dw/shell-basename-dashdash-before-stripping-leading-dash-from-login (2014-11-25) 1 commit (merged to 'next' on 2014-12-15 at 42937b7) + git-sh-setup.sh: use dashdash with basename call * jc/conflict-hint (2014-10-28) 4 commits (merged to 'next' on 2014-12-15 at b72475f) + merge sequencer: turn Conflicts: hint into a comment + builtin/commit.c: extract ignore_non_trailer() helper function + merge sequencer: unify codepaths that write Conflicts: hint + builtin/merge.c: drop a parameter that is never used (this branch is used by cc/interpret-trailers-more.) Unlike all the other hints given in the commit log editor, the list of conflicted paths were appended at the end without commented out. * jc/exec-cmd-system-path-leak-fix (2014-11-30) 1 commit (merged to 'next' on 2014-12-15 at f926ee5) + system_path(): always return free'able memory to the caller The function sometimes returned a non-freeable memory and some other times returned a piece of memory that must be freed. * jc/hook-cleanup (2014-12-01) 1 commit (merged to 'next' on 2014-12-15 at f5759d0) + run-command.c: retire unused run_hook_with_custom_index() Remove unused code. * jc/refer-to-t-readme-from-submitting-patches (2014-11-24) 2 commits (merged to 'next' on 2014-12-15 at 0e88699) + t/README: justify why ! grep foo is sufficient + SubmittingPatches: refer to t/README for tests * jg/prompt-localize-temporary (2014-12-12) 1 commit (merged to 'next' on 2014-12-15 at bb9cac9) + git-prompt.sh: make $f local to __git_eread() git-prompt (in contrib/) used a variable from the global scope, possibly contaminating end-user's namespace. * jk/always-allow-large-packets (2014-12-10) 1 commit (merged to 'next' on 2014-12-15 at c3fb2c8) + pkt-line: allow writing of LARGE_PACKET_MAX buffers git push and git fetch did not communicate an overlong refname correctly. * jk/colors (2014-12-09) 6 commits (merged to 'next' on 2014-12-15 at 20b045f) + parse_color: drop COLOR_BACKGROUND macro + diff-highlight: allow configurable colors + parse_color: recognize no$foo to clear the $foo attribute + parse_color: support 24-bit RGB values + parse_color: refactor color storage + Merge branch 'jn/parse-config-slot' into jk/colors diff-highlight filter (in contrib/) allows its color output to be customized via configuration variables. * jk/commit-date-approxidate (2014-12-11) 2 commits (merged to 'next' on 2014-12-15 at 047530e) + commit: always populate GIT_AUTHOR_* variables + commit: loosen ident checks when generating template Recent update to git commit broke amending an existing commit with bogus author/committer lines without a valid e-mail address. * jk/credential-quit (2014-12-04) 2 commits (merged to 'next' on 2014-12-15 at 4cfd999) + prompt: respect GIT_TERMINAL_PROMPT to disable terminal prompts + credential: let helpers tell us to quit Credential helpers are asked in turn until one of them give positive response, which is cumbersome to turn off when you need to run Git in an automated setting. The credential helper interface learned to allow a helper to say stop, don't ask other helpers. Also GIT_TERMINAL_PROMPT environment can be set to false to disable
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
From: Ben Walton bdwal...@gmail.com
The awk statements previously used in this test weren't compatible
with the native versions of awk on Solaris:
echo dir | /bin/awk -v c=0 '$1 {++c} END {print c}'
awk: syntax error near line 1
awk: bailing out near line 1
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Work it around by using $1 != to state more explicitly that we
are skipping empty lines.
Helped-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ben Walton bdwal...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Then let's queue this, perhaps?
t/t0090-cache-tree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 067f4c6..601d02d 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
# We want to count only foo because it's the only direct child
subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq)
- subtree_count=$(echo $subtrees|awk -v c=0 '$1 {++c} END {print c}')
+ subtree_count=$(echo $subtrees|awk -v c=0 '$1 != {++c} END {print
c}')
entries=$(git ls-files|wc -l)
printf SHA $dir (%d entries, %d subtrees)\n $entries
$subtree_count
for subtree in $subtrees
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
Junio C Hamano gits...@pobox.com writes:
From: Ben Walton bdwal...@gmail.com
The awk statements previously used in this test weren't compatible
with the native versions of awk on Solaris:
echo dir | /bin/awk -v c=0 '$1 {++c} END {print c}'
awk: syntax error near line 1
awk: bailing out near line 1
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Work it around by using $1 != to state more explicitly that we
are skipping empty lines.
Helped-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ben Walton bdwal...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
* Then let's queue this, perhaps?
heh, not like that without updating the subject, perhaps like this:
Subject: t0090: tweak awk statement for Solaris /usr/xpg4/bin/awk
Sorry for the noise.
t/t0090-cache-tree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 067f4c6..601d02d 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
# We want to count only foo because it's the only direct child
subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq)
- subtree_count=$(echo $subtrees|awk -v c=0 '$1 {++c} END {print c}')
+ subtree_count=$(echo $subtrees|awk -v c=0 '$1 != {++c} END {print
c}')
entries=$(git ls-files|wc -l)
printf SHA $dir (%d entries, %d subtrees)\n $entries
$subtree_count
for subtree in $subtrees
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
Junio C Hamano gits...@pobox.com writes:
From: Ben Walton bdwal...@gmail.com
The awk statements previously used in this test weren't compatible
with the native versions of awk on Solaris:
echo dir | /bin/awk -v c=0 '$1 {++c} END {print c}'
awk: syntax error near line 1
awk: bailing out near line 1
echo dir | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo dir | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Work it around by using $1 != to state more explicitly that we
are skipping empty lines.
By the way, I was hoping (eh, what kind of hope is that???) that $1
alone is not a kosher POSIX way but a GNUism, but that does not seem
to be the case. POSIX has this [*1*]
When an expression is used in a Boolean context, if it has a
numeric value, a value of zero shall be treated as false and any
other value shall be treated as true. Otherwise, a string value
of the null string shall be treated as false and any other value
shall be treated as true. A Boolean context shall be one of the
following:
and among the Boolean context listed is:
* An expression used as a pattern (as in Overall Program Structure)
So the example with /usr/xpg4/bin/awk does not seem to be a
behaviour from a conformant implementationd, and it seems to be
correct to label this as work it around by ... (not avoid using
GNUism).
We learn new things every day (not that I really wanted to learn
glitches in various implementations of awk) ;-).
Thanks.
[Reference]
*1* http://pubs.opengroup.org/onlinepubs/9699919799/utilities/awk.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Use wc instead of awk to count subtrees in t0090-cache-tree
Junio C Hamano wrote:
Junio C Hamano gits...@pobox.com writes:
From: Ben Walton bdwal...@gmail.com
The awk statements previously used in this test weren't compatible
with the native versions of awk on Solaris:
echo dir | /bin/awk -v c=0 '$1 {++c} END {print c}'
awk: syntax error near line 1
awk: bailing out near line 1
If I were doing it, I'd leave the above four lines out --- they are
describing an unrelated problem. I wonder if we should make the test
harness respect SANE_TOOL_PATH to avoid that kind of problem in the
future.
[...]
heh, not like that without updating the subject, perhaps like this:
Subject: t0090: tweak awk statement for Solaris /usr/xpg4/bin/awk
With the updated subject,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
fast-import's notemodify doesn't work the same as git notes
Hi,
There are two major differences between adding notes with fast-import
and git notes, one of which is a serious problem:
- fast-import doesn't want to add notes for non commits, while git notes
does.
- fast-import and git notes have different, conflicting fanouts:
- take e.g. the git repo (there needs to be a lot of commits to start
to see the problem)
- run the following to create notes for every commit:
(echo 'blob';
echo 'mark :1';
echo 'data 0';
echo 'commit refs/notes/foo';
echo 'committer foo 0 +';
echo 'data 0';
git rev-list --all | awk '{print N :1, $1}';
echo) | git fast-import
- pick a random commit. I'll pick
bbcefffcea9789e4a1a2023a1c778e2c07db77a7 as it is current master as
of writing. Take the first two characters of that sha1, and look at
the ls-tree:
git ls-tree refs/notes/foo bb/
You'll see a number of blobs.
- Now, remove the note for that commit with git notes:
git notes --ref foo remove bbcefffcea9789e4a1a2023a1c778e2c07db77a7
- ls-tree again, you'll now see a number of trees instead of blobs,
because git notes will have done a fanout. - git notes does fanouts
for much less items than fast-import does.
- Re-add a note for that commit with fast-import:
git fast-import EOF
blob
mark :1
data 0
commit refs/notes/foo
committer foo 0 +
data 0
from refs/notes/foo^0
N :1 bbcefffcea9789e4a1a2023a1c778e2c07db77a7
EOF
- ls-tree again, and you'll see a number of trees and *one* blob, for
bb/cefffcea9789e4a1a2023a1c778e2c07db77a7
- See the thread starting with 20141126004242.ga13...@glandium.org,
this type of notes branch make things very slow.
- Now, if you take an even bigger repository (as long as there are more
than 65536 commits, that's good ; I guess the linux kernel
qualifies, I've been checking with a mozilla-central clone), and
create exactly 65535 notes with git fast-import, you'll end up with
a 1-level tree (2/38). Add one more note, and the entire tree turns
into a 2-level tree (2/2/36). git notes would only add a level to
the tree containing the added note. git notes's behavior scales
better, because think about what happens on the next fanout with
fast-import... adding one note would need to create millions of trees.
Cheers,
Mike
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/5] update_unicode.sh: set UNICODE_DIR only once
On 22.12.14 19:02, Junio C Hamano wrote:
dev+...@drbeat.li writes:
From: Beat Bolli dev+...@drbeat.li
The value is the same on both uniset invocations, so Don't Repeat
Yourself applies.
Since we're in a subshell already, there's no need to unset UNICODE_DIR
at the end.
Strictly speaking, you are not introducing your own subshell to
prevent the environment from leaking (i.e. you used {...} not
(...) in the previous step). The reason you can do this is
because the generation of UNICODEWIDTH_H file is the last thing in
the subshell.
I don't introduce a new one, but we're still in the outer subshell that
starts on line 12 ( cd unicode .
I'll reword it to Since this is done as the last command, ...
Thanks.
Signed-off-by: Beat Bolli dev+...@drbeat.li
---
update_unicode.sh | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/update_unicode.sh b/update_unicode.sh
index c1c876c..bed8916 100755
--- a/update_unicode.sh
+++ b/update_unicode.sh
@@ -27,12 +27,13 @@ fi
fi
make
) {
+UNICODE_DIR=. export UNICODE_DIR
echo static const struct interval zero_width[] = {
-UNICODE_DIR=. ./uniset/uniset --32 cat:Me,Mn,Cf +
U+1160..U+11FF - U+00AD |
+./uniset/uniset --32 cat:Me,Mn,Cf + U+1160..U+11FF - U+00AD |
grep -v plane
echo };
echo static const struct interval double_width[] = {
-UNICODE_DIR=. ./uniset/uniset --32 eaw:F,W
+./uniset/uniset --32 eaw:F,W
echo };
} $UNICODEWIDTH_H
)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Saving space/network on common repos
btw, just FYI, the scheme you lay out here doesn't actually work as-is. The problem is the config file, which has an entry like: worktree = ../../../mysubmodule This depends on the config file living in ./git/modules/mysubmodule/config. But the proposed scheme moves the config file to mysubmodule/.git/config, and the relative path is broken. I'm not sure what the best solution is; the cleanest one requires a pretty substantial rewrite of git-new-workdir (not that it's such a giant piece of code), and separating out new_workdir from new_gitdir. The smallest one involves having some way to suppress the final 'git checkout -f' (which is the only thing in this script that needs the worktree entry to resolve somewhere) to allow for post-script cleanup. craig On Wed, Dec 17, 2014 at 4:07 PM, Jonathan Nieder jrnie...@gmail.com wrote: Craig Silverstein wrote: On Wed, Dec 17, 2014 at 2:32 PM, Jonathan Nieder jrnie...@gmail.com wrote: Craig Silverstein wrote: Question 4) Is there a practical way to set up submodules so they can use the same object-sharing framework that the main repo does? It's possible to do, but we haven't written a nice UI for it yet. (In other words, you can do this by cloning with --no-recurse-submodules and manually creating the submodule workdir in the appropriate place. Hmm, let me see if I understand you right -- you're suggesting that when cloning my reference repo, I do git clone --no-recurse-submodules my repo for (path, url) in `parse-.gitmodules`: git clone url path # this is psuedocode, obviously :-) and then when I want to create a new workdir, I do something like: cd reference_repo git new-workdir /var/workspace1 for (path, url) in `parse-.gitmodules`: cd path git new-workdir /var/workspace1/path ? Basically, I'm going back to the old git way of having each submodule have its own .git directory, rather than having it have a .git file with a 'gitdir' entry. Am I understanding this right? Basically. The initial clone can still use --recurse-submodules. When you create a new workdir you'd create new workdirs for the submodules by hand. A 'git submodule foreach' command in the initial repo can take care of the `parse-.gitmodules` part. [...] Also, it seems to me there's the possibility, with git-newdir, that if several of the workspaces try to fetch at the same time they could step on each others' toes. Is that a problem? I know there's a push lock but I don't believe there's a fetch lock, and I could imagine git getting unhappy if two fetches happened in the same repo at the same time. That's a good question. If concurrent fetches cause trouble then I'd consider it a bug (it's not too different from multiple concurrent pushes to the same repository, which is a very common thing to do), but I haven't looked carefully into whether such bugs exist. Hopefully someone else can chime in. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7] receive-pack.c: receive-pack.c: use a single ref_transaction for atomic pushes
On Fri, Dec 19, 2014 at 2:38 PM, Stefan Beller sbel...@google.com wrote:
Update receive-pack to use an atomic transaction iff the client negotiated
that it wanted atomic-push. This leaves the default behavior to be the old
non-atomic one ref at a time update. This is to cause as little disruption
as possible to existing clients. It is unknown if there are client scripts
that depend on the old non-atomic behavior so we make it opt-in for now.
If it turns out over time that there are no client scripts that depend on the
old behavior we can change git to default to use atomic pushes and instead
offer an opt-out argument for people that do not want atomic pushes.
Notes and observations below. None of them are particularly
actionable. If Junio is happy with the current round, and if you don't
have some other reason to re-roll, then consider them food for thought
for future patches.
Inspired-by: Ronnie Sahlberg sahlb...@google.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Stefan Beller sbel...@google.com
---
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e76e5d5..710cd7f 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1044,11 +1047,72 @@ static void reject_updates_to_hidden(struct command
*commands)
}
}
+static void execute_commands_non_atomic(struct command *commands,
+ struct shallow_info *si);
+
+
+static void execute_commands_atomic(struct command *commands,
+ struct shallow_info *si)
+{
+ struct command *cmd;
+ struct strbuf err = STRBUF_INIT;
+ const char *reported_error = atomic push failure;
+ int checked_connectivity = 1;
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ rp_error(%s, err.buf);
+ reported_error = transaction failed to start;
+ goto failure;
+ }
+
+ for (cmd = commands; cmd; cmd = cmd-next) {
+ if (cmd-error_string)
+ goto failure;
+
+ if (cmd-skip_update)
+ goto failure;
These checks are common to the atomic and non-atomic cases. To reduce
code duplication between the two cases, you could factor them out to a
cmd_okay() helper function (or some such name) which checks both
conditions.
+ cmd-error_string = update(cmd, si);
+
+ if (cmd-error_string)
+ goto failure;
+
+ if (shallow_update si-shallow_ref[cmd-index]) {
+ error(BUG: connectivity check has not been run on
ref %s,
+ cmd-ref_name);
+ checked_connectivity = 0;
+ reported_error = transaction failed due to internal
bug;
+ goto failure;
This code is also common to atomic and non-atomic cases and could be
factored out to a helper function, thus further reducing duplication.
The less code duplication between cases, the lower the cognitive load,
making the code easier to understand.
+ }
+ }
+
+ if (ref_transaction_commit(transaction, err)) {
+ rp_error(%s, err.buf);
+ reported_error = atomic transaction failed;
+ goto failure;
+ }
+
+ ref_transaction_free(transaction);
+ strbuf_release(err);
+ return;
+
+failure:
+ for (cmd = commands; cmd; cmd = cmd-next)
+ if (!cmd-error_string)
+ cmd-error_string = reported_error;
+ ref_transaction_free(transaction);
+ strbuf_release(err);
+
+ if (shallow_update !checked_connectivity)
+ error(BUG: run 'git fsck' for safety.\n
+ If there are errors, try to remove
+ the reported refs above);
This final conditional is common to both cases but does not even need
to be factored out to a helper function. It could/should have remained
in execute_commands() at its original position, following the call to
execute_commands_atomic() or execute_commands_non_atomic().
+}
+
static void execute_commands(struct command *commands,
const char *unpacker_error,
struct shallow_info *si)
{
- int checked_connectivity;
struct command *cmd;
unsigned char sha1[20];
struct iterate_data data;
@@ -1079,7 +1143,20 @@ static void execute_commands(struct command *commands,
free(head_name_to_free);
head_name = head_name_to_free = resolve_refdup(HEAD, 0, sha1, NULL);
- checked_connectivity = 1;
+ if (use_atomic) {
+ execute_commands_atomic(commands, si);
+ } else {
+ execute_commands_non_atomic(commands, si);
+ }
Style: Unnecessary braces.
More below.
+}
+
+static
Re: Saving space/network on common repos
Craig Silverstein wrote: btw, just FYI, the scheme you lay out here doesn't actually work as-is. The problem is the config file, which has an entry like: worktree = ../../../mysubmodule This depends on the config file living in ./git/modules/mysubmodule/config. But the proposed scheme moves the config file to mysubmodule/.git/config, and the relative path is broken. *puzzled* Can you give a transcript illustrating this happening? Submodules with .git directory within their worktree or under .git/modules/ are both supposed to work. And in either case, having refs/ and objects/ as symlinks should work fine. When git new-workdir creates a new workdir, it has its own new and separate config file, so I don't think that is the source of trouble. Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH w/signoff] pre-push.sample: Remove unwanted `IFS=' '`.
Junio C Hamano gits...@pobox.com writes: Jim Hill gjth...@gmail.com writes: I call it unwanted because the default works fine with the actual input and explicitly limiting whitespace this way breaks most command substitution. OK. I'd call that unnecessary, not unwanted, though. It becomes unwanted only when somebody cuts and pastes and changes what happens inside the body of the loop without thinking what IFS assignment is doing. Leaving it to the default is not wrong per-se, but I think it is better to justify this change as protecting cut-and-paste people, which is its primary benefit as far as I can see. Thanks for noticing. FYI, here is what I queued for today's integration cycle (you should be able to find it in 'pu' branch). -- 8 -- From: Jim Hill gjth...@gmail.com Date: Sun, 21 Dec 2014 11:26:00 -0800 Subject: [PATCH] pre-push.sample: remove unnecessary and misleading IFS=' ' The sample hook explicitly sets IFS to SP and nothing else so that the read used in the per-ref while loop that iterates over localref SP localsha1 SP remoteref SP remotesha records, where we know refs and sha1s will not have SPs, would split them correctly. While this is not wrong per-se, it is not necessary; because we know these fields do not contain HT or LF, either, we can simply leave IFS the default. This will also prevent those who cut and paste from this sample from getting bitten when they write things in the per-ref loop that need splitting with the default $IFS (e.g. use $(git rev-list ...) to produce one-record-per-line output). Signed-off-by: Jim Hill gjth...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- templates/hooks--pre-push.sample | 1 - 1 file changed, 1 deletion(-) diff --git a/templates/hooks--pre-push.sample b/templates/hooks--pre-push.sample index 69e3c67..6187dbf 100755 --- a/templates/hooks--pre-push.sample +++ b/templates/hooks--pre-push.sample @@ -24,7 +24,6 @@ url=$2 z40= -IFS=' ' while read local_ref local_sha remote_ref remote_sha do if [ $local_sha = $z40 ] -- 2.2.1-321-gd161b79 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] receive-pack.c: add protocol support to negotiate atomic-push
On 22.12.2014 14:52, Eric Sunshine wrote: On Fri, Dec 19, 2014 at 2:38 PM, Stefan Beller sbel...@google.com wrote: From: Ronnie Sahlberg sahlb...@google.com This adds support to the protocol between send-pack and receive-pack to * allow receive-pack to inform the client that it has atomic push capability * allow send-pack to request atomic push back. There is currently no setting in send-pack to actually request that atomic pushes are to be used yet. This only adds protocol capability not ability for the user to activate it. Signed-off-by: Ronnie Sahlberg sahlb...@google.com Signed-off-by: Stefan Beller sbel...@google.com --- diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt index 6d5424c..4f8a7bf 100644 --- a/Documentation/technical/protocol-capabilities.txt +++ b/Documentation/technical/protocol-capabilities.txt @@ -244,6 +245,14 @@ respond with the 'quiet' capability to suppress server-side progress reporting if the local progress reporting is also being suppressed (e.g., via `push -q`, or if stderr does not go to a tty). +atomic +-- + +If the server sends the 'atomic' capability it is capable of accepting +atomic pushes. If the pushing client requests this capability, the server +will update the refs in one atomic transaction. Either all refs are Not itself worth a re-send, but if you re-send for some other reason... one atomic still smacks of redundancy; an atomic sounds better. I did hear you saying 'one single atomic' being too redundant. And I agree that 'one' and 'single' makes the redundancy. However I have the impression 'an atomic' is too weak. Not everybody is a careful reader picking up the subtle notions. Not everybody is english native. Or concentrated. Look at it the other way: How could it be done? * All of the refs could be updated one at a time, not atomically, so foreach ref: open refs/heads/bla: write new sha1 * All of the refs could be updated at once, not atomically: open refs pack file: write new content * All of the refs could be updated, one at a time, atomically: foreach ref: get lock write content to lock rename into place * All of the refs at once, atomically. open packed refs file lock: write new content rename into place That said, atomicity and how many transactions there are, are orthogonal to each other. That's why I'd keep pointing out 'one atomic' transaction. Thanks for all the comments. I may be doing cleanup patches for you on top of what Junio queued. +updated or none. + allow-tip-sha1-in-want -- -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git update-ref --stdin : too many open files
On 22.12.2014 13:22, Junio C Hamano wrote: Loic Dachary l...@dachary.org writes: Hi, Steps to reproduce: $ git --version git version 1.9.1 $ wc -l /tmp/1 9090 /tmp/1 $ head /tmp/1 delete refs/pull/1/head create refs/heads/pull/1 86b715f346e52920ca7c9dfe65424eb9946ebd61 delete refs/pull/1/merge create refs/merges/1 c0633abdc5311354c9729374e0ba25c97a89f69e ... $ ulimit -n 1024 $ git update-ref --stdin /tmp/1 fatal: Unable to create /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too many open files $ head -20 /tmp/1 | git update-ref --stdin $ echo $? 0 The workaround is to increase ulimit -n git update-ref --stdin should probably close some files. Cheers Sounds like the recent ref update in a transaction issue to me. Stefan, want to take a look? I think we do need to keep the .lock files without renaming while in transaction, but we do not have to keep them open, so I suspect that a fix may be to split the commit function into two (one to close but not rename, the other to finalize by renaming) or something. Sounds reasonable. Though by closing the file we're giving up again a bit of safety. If we close the file everyone could tamper with the lock file. (Sure they are not supposed to touch it, but they could) Also the version of transaction series we have queued seem to lock these refs very late in the process, but as we discussed privately a few weeks ago, we would want to move the lock much earlier, when the first update is attempted. I'll look into that tomorrow. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XDL_FAST_HASH can be very slow
(sorry for the repost, I use gmail and it send html mails by default). On 22 December 2014 at 11:48, Thomas Rast t...@thomasrast.ch wrote: 1. hash function throughput 2. quality of the hash values 3. avoiding collision attacks XDL_FAST_HASH was strictly an attempt to improve throughput, and fairly successful at that (6942efc (xdiff: load full words in the inner loop of xdl_hash_record, 2012-04-06) quotes an 8% improvement on 'git log -p'). You are now addressing quality. I have no idea how you ran into this, but if you are reworking things already, I think it would be good to also randomize whatever hash you put in so as to give some measure of protection against collision attacks. I assume you mean DJB2 when you say DJB, and if so I will just note that it is a pretty terrible hash function for arbitrary data. (I understand it does better with raw text.) It does not pass either strict-avalanche-criteria[1], nor does it pass the bit-independence-criteria[2]. I have images which show how badly DJB2 fails these tests if anyone is interested. Murmur3 is better, in that it does pass SAC and BIC, but before you decide to use Murmur3 you should review https://131002.net/siphash/and related resources which demonstrate multi-collision attacks on Murmur3 which are independent of the seed chosen. The paper also introduces a new hash function with good performance properties, and claims that it has cyptographic strength. (I say claims because I am not qualified to judge if it is or not.) Eg: https://131002.net/siphash/murmur3collisions-20120827.tar.gz I think if you want performance and robustness against collision attacks Siphash is a good candidate, as is perhaps the AES derived hash used by the Go folks, but the performance of that algorithm is strongly dependent on the CPU supporting AES primitives. Anyway, the point is that simply adding a random seed to a hash function like DJB2 or Murmur3 is not sufficient to prevent collision attacks. Yves [1] A change in a single bit of the seed or the key should result in 50% of the output bits of the hash changing. [2] output bits j and k should change independently when any single input bit i is inverted, for all i, j and k. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Saving space/network on common repos
Craig Silverstein wrote:
btw, just FYI, the scheme you lay out here doesn't actually work
as-is. The problem is the config file, which has an entry like:
worktree = ../../../mysubmodule
This depends on the config file living in
./git/modules/mysubmodule/config. But the proposed scheme moves the
config file to mysubmodule/.git/config, and the relative path is
broken.
As was pointed out to me privately, the behavior is exactly as you
described and I had confused myself by looking at directory that
wasn't even made with git-new-workdir. Sorry for the nonsense.
Workdirs share a single config file because information associated to
branches set by git branch --set-upstream-to, git branch
--edit-description, git remote, and so on are stored in the config
file.
The 'git checkout --to' series in pu avoids this problem by ignoring
core.bare and core.worktree in worktrees created with 'git checkout --to'.
To try it:
git clone https://kernel.googlesource.com/pub/scm/git/git
cd git
git merge 'origin/pu^{/nd/multiple-work-trees}^2'
make
PATH=$(pwd)/bin-wrappers:$PATH
git checkout --to=../experiment next
This seems like good motivation to try to get that series in good
shape and release it soon.
Thanks again,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git update-ref --stdin : too many open files
Hi Stefan, Stefan Beller wrote: On 22.12.2014 13:22, Junio C Hamano wrote: Loic Dachary l...@dachary.org writes: fatal: Unable to create /home/gitmirror/repositories/Ceph/ceph/refs/heads/pull/1917.lock': Too many open files [...] Stefan, want to take a look? I think we do need to keep the .lock files without renaming while in transaction, but we do not have to keep them open, so I suspect that a fix may be to split the commit function into two (one to close but not rename, the other to finalize by renaming) or something. Makes sense. Sounds reasonable. Though by closing the file we're giving up again a bit of safety. If we close the file everyone could tamper with the lock file. (Sure they are not supposed to touch it, but they could) At least on Linux, keeping a file open doesn't offer any protection against someone else deleting it. (It also doesn't offer any protection against someone updating the ref directly. ;-) Opening the corresponding .lock file with O_EXCL is part of the contract for updating refs.) Thanks, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Saving space/network on common repos
} This seems like good motivation to try to get that series in good
shape and release it soon.
I was going to spend some time tomorrow (if I can find any :-) )
trying to fix up the contrib script to work with submodules, or at
least the kind that we use. Is that something that's worth the time
to do, or would we be better off just waiting for the work-tree stuff
to get released? If I do end up doing it, would you be interested in
a pull request (or however patches are submitted in the git world)?
craig
On Mon, Dec 22, 2014 at 7:12 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Craig Silverstein wrote:
btw, just FYI, the scheme you lay out here doesn't actually work
as-is. The problem is the config file, which has an entry like:
worktree = ../../../mysubmodule
This depends on the config file living in
./git/modules/mysubmodule/config. But the proposed scheme moves the
config file to mysubmodule/.git/config, and the relative path is
broken.
As was pointed out to me privately, the behavior is exactly as you
described and I had confused myself by looking at directory that
wasn't even made with git-new-workdir. Sorry for the nonsense.
Workdirs share a single config file because information associated to
branches set by git branch --set-upstream-to, git branch
--edit-description, git remote, and so on are stored in the config
file.
The 'git checkout --to' series in pu avoids this problem by ignoring
core.bare and core.worktree in worktrees created with 'git checkout --to'.
To try it:
git clone https://kernel.googlesource.com/pub/scm/git/git
cd git
git merge 'origin/pu^{/nd/multiple-work-trees}^2'
make
PATH=$(pwd)/bin-wrappers:$PATH
git checkout --to=../experiment next
This seems like good motivation to try to get that series in good
shape and release it soon.
Thanks again,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Question on for-each-ref and --contains
Hi. In order to make some analyses here I had to build certain report lists of our remote refs, based on various containing and merged rules. We have more than hundred such refs at a time usually. So my poor script which tries to make decisions with the help of rev-list for each ref on each arbitrary commit is not really fast. The git-branch --contains and --merged are not very handy too, because the output is not really flexible, and the --merged works on HEAD only. Do you think is a good idea to have the option --contains in for-each-ref too (and/or in rev-list directly)? If yes, then probably also the --(no-)merged options, but hopefully with the arbitrary commit this time? Thank you in advance, Blind. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
