Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

Am 04.11.2015 um 21:14 schrieb Stefan Beller: On Wed, Nov 4, 2015 at 10:14 AM, Junio C Hamano wrote: Imagine that we are running two things A and B at the same time. We ask poll(2) and it says both A and B have some data ready to be read, and we try to read from A.

Re: [PATCH v6 25/25] refs: break out ref conflict checks

On Tue, 2015-11-03 at 08:40 +0100, Michael Haggerty wrote: > + * extras and skip must be sorted lists of reference names. Either one > + * can be NULL, signifying the empty list. > + */ My version had: "skip can be NULL; extras cannot." The first thing that function does is:

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

Johannes Sixt writes: > I think that a scenario where A and B are communicating is rather > far-fetched. We are talking about parallelizing independent tasks. I > would not worry. I wouldn't worry too much if this were merely a hack that is only applicable to submodules, but I do

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

On Wed, Nov 4, 2015 at 12:42 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> Another approach would be to test if we can set to non blocking and if >> that is not possible, do not buffer it, but redirect the subcommand >> directly to stderr of the

[PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die

Discussion turned out a warning is not enough, but we need to die in case of blocking output as we can lockup the child processes. Junio wrote: > Imagine that we are running two things A and B at the same time. We > ask poll(2) and it says both A and B have some data ready to be > read, and we

[PATCH 2/2] run-parallel: Run sequential if nonblocking I/O is unavailable

Windows doesn't have O_NONBLOCK nor F_GETFL defined, so we need cannot run in parallel there. Instead the children will output directly to our stderr and we run one child at a time. Bonus: We are setting process.err = -1; which we previously expected the get_next_task callback to do. It is easy

[PATCH 0/2] Missing O_NONBLOCK under Windows (was: git.git as of tonight)

The first patch is a general fixup as per discussion, the second patch will make Git compile in Windows again (hopefully, not tested) The number of #ifdefs seems acceptable to me, opinions on that? This has been developed on top of d075d2604c0f9 (Merge branch 'rs/daemon-plug-child-leak' into

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

On Wed, Nov 4, 2015 at 1:19 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> So more like: >> >> if (platform_capable_non_blocking_IO()) >> set_nonblocking_or_die(>children[i].process.err); >> else >>

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

On Wed, Nov 04, 2015 at 01:01:53PM -0800, Junio C Hamano wrote: > But the symptom does not have to be as severe as a total deadlock to > be problematic. If we block B (and other tasks) by not reading from > them quickly because we are blocked on reading from A, which may > take forever (in

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

Stefan Beller writes: > Another approach would be to test if we can set to non blocking and if > that is not possible, do not buffer it, but redirect the subcommand > directly to stderr of the calling process. > > if (set_nonblocking(pp->children[i].process.err) < 0) { >

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

Stefan Beller writes: > So more like: > > if (platform_capable_non_blocking_IO()) > set_nonblocking_or_die(>children[i].process.err); > else > pp->children[i].process.err = 2; /* ugly intermixed output is > possible*/ When I mentioned #if..#endif, I

Re: When a file was locked by some program, git will work stupidly

On 2 November 2015 at 15:33, Randall S. Becker wrote: > > On November-01-15 11:57 PM dayong xie wrote: >>To be specific >>In my Unity project, there is a native plugin, and plugin's extension is >>.dll, >and this plugin is under git version control, when Unity is

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

Jeff King writes: > So I'm not sure I see why we need to be non-blocking at all here, if we > are correctly hitting poll() and doing a single read on anybody who > claims to be ready (rather than trying to soak up all of their available > data), then we should never block, and we

File owner/group and git

In unpack-trees.c, in verify_uptodate_1, we check ie_match_stat. This returns OWNER_CHANGED if a file has changed ownership since the index was updated. Do we actually care about that particular case? Or really anything other than DATA_CHANGED? (We noticed this due to a bug in our watchman

Re: File owner/group and git

David Turner writes: > In unpack-trees.c, in verify_uptodate_1, we check ie_match_stat. This > returns OWNER_CHANGED if a file has changed ownership since the index > was updated. Do we actually care about that particular case? Or really > anything other than

Re: [PATCH v6 25/25] refs: break out ref conflict checks

On 11/04/2015 10:01 PM, David Turner wrote: > On Tue, 2015-11-03 at 08:40 +0100, Michael Haggerty wrote: >> + * extras and skip must be sorted lists of reference names. Either one >> + * can be NULL, signifying the empty list. >> + */ > > My version had: > > "skip can be NULL; extras cannot." >

Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die

Torsten Bögershausen writes: > (Jumping into an old discussion, I may be off topic) I think this is exactly the latest "I wonder" from Peff, to which I said "well, perhaps we didn't need nonblock after all from the beginning". > And this work regardless if the fd blocking or

[PATCH v3 2/4] upload-pack: strip refs before calling ref_is_hidden()

Make hideRefs handling in upload-pack consistent with the behavior described in the documentation by stripping refs before comparing them with prefixes in hideRefs. Signed-off-by: Lukas Fleischer --- upload-pack.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)

[PATCH v3 4/4] t5509: add basic tests for hideRefs

Test whether regular and full hideRefs patterns work as expected when namespaces are used. Helped-by: Eric Sunshine Signed-off-by: Lukas Fleischer --- t/t5509-fetch-push-namespaces.sh | 41 1 file changed, 41

[PATCH v3 3/4] Add support for matching full refs in hideRefs

In addition to matching stripped refs, one can now add hideRefs patterns that the full (unstripped) ref is matched against. To distinguish between stripped and full matches, those new patterns must be prefixed with a circumflex (^). This commit also removes support for the undocumented and

[PATCH v3 0/4] Improve hideRefs when used with namespaces

This is a first set of patches improving documentation and behavior of the transfer.hideRefs feature as discussed in [1]. In particular, hideRefs is changed to generally match stripped refs by default and match full refs when prefixed with "^". The documentation is updated accordingly. Basic tests

Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die

On 11/04/2015 11:43 PM, Stefan Beller wrote: Discussion turned out a warning is not enough, but we need to die in case of blocking output as we can lockup the child processes. Junio wrote: Imagine that we are running two things A and B at the same time. We ask poll(2) and it says both A and B

[PATCH v3 1/4] Document the semantics of hideRefs with namespaces

Right now, there is no clear definition of how transfer.hideRefs should behave when a namespace is set. Explain that hideRefs prefixes match stripped names in that case. This is how hideRefs patterns are currently handled in receive-pack. Signed-off-by: Lukas Fleischer ---

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

On Wed, Nov 04, 2015 at 06:05:17PM -0800, Junio C Hamano wrote: > I've always assumed that the original reason why we wanted to set > the fd to nonblock was because poll(2) only tells us there is > something to read (even a single byte), and the xread_nonblock() > call strbuf_read_once() makes

Re: About global --progress option

Edmundo Carmona Antoranz writes: > Which would be the correct development path? > > - Two-step process: First step, implement global --[no-]progress at > the git level and also support the option from the builtins that > laready have it. Let it live like that for some

Re: About global --progress option

On Thu, Nov 5, 2015 at 12:11 AM, Junio C Hamano wrote: > Besides, I am not convinced that you are bringing in a net positive > value by allowing "git --no-progress clone". You would need to > think what to do when you get two conflicting options (e.g. should > "git

About global --progress option

Hi, everybody! Coming back from my patch about the --[no-]progress option for checkout (it's already in next, J), I'd like to build into the idea of the single --[no-]progress option for git as a whole. So, in order to know what/how things should be developed, let's start with a tiny survey

Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die

Junio C Hamano writes: > Torsten Bögershausen writes: > >> (Jumping into an old discussion, I may be off topic) > > I think this is exactly the latest "I wonder" from Peff, to which I > said "well, perhaps we didn't need nonblock after all from the >

[PATCH v4 3/3] Move all the SHA1 implementations into one directory

From: Atousa Pahlevan Duprat The various SHA1 implementations were spread around in 3 directories. This makes it easier to understand what implementations are available at a glance. Signed-off-by: Atousa Pahlevan Duprat --- Makefile | 10

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

Jeff King writes: > POSIX implies it is the case in the definition of read[2] in two ways: > > 1. The O_NONBLOCK behavior for pipes is mentioned only when dealing > with empty pipes. > > 2. Later, it says: > >The value returned may be less than nbyte if the number

[PATCH v4 1/3] Provide another level of abstraction for the SHA1 utilities.

From: Atousa Pahlevan Duprat The git source uses git_SHA1_Update() and friends to call into the code that computes the hashes. This is can then be mapped directly to an implementation that computes the hash, such as platform_SHA1_Update(); or as we will do in a subsequent

Re: [PATCH] test: accept death by SIGPIPE as a valid failure mode

On Fri, Oct 30, 2015 at 02:22:14PM -0700, Junio C Hamano wrote: > On a local host, the object/history transport code often talks over > pipe with the other side. The other side may notice some (expected) > failure, send the error message either to our process or to the > standard error and hung

[PATCH v4 2/3] Limit the size of the data block passed to SHA1_Update()

From: Atousa Pahlevan Duprat Use the previous commit's abstraction mechanism for SHA1 to support a chunked implementation of SHA1_Update() which limits the amount of data in the chunk passed to SHA1_Update(). This is enabled by using the SHA1_MAX_BLOCK_SIZE envvar to specify

Re: [PATCH 1/2] run-parallel: rename set_nonblocking to set_nonblocking_or_die

On Wed, Nov 04, 2015 at 10:19:43PM -0800, Junio C Hamano wrote: > Having read your message, I notice these disturbing passages in my > copy of manual pages. > > poll(2) has > > BUGS >See the discussion of spurious readiness notifications under >the BUGS section of

Re: About global --progress option

On Thu, Nov 05, 2015 at 12:47:22AM -0600, Edmundo Carmona Antoranz wrote: > On the technical side, I think the global --progress option (and > removing the option from the builtins) would not add complexity but > the opposite because setting the flag would be done at the "main git" > level and

Re: [PATCH v2 4/4] t5509: add basic tests for hideRefs

On Wed, Nov 04, 2015 at 02:36:54PM -0500, Eric Sunshine wrote: > > +test_expect_success 'try to update a hidden ref' ' > > + test_config -C pushee transfer.hideRefs refs/heads/master && > > + test_must_fail git -C original push pushee-namespaced master > > In above tests, you use -c

Re: [PATCH] Limit the size of the data block passed to SHA1_Update()

On Tuesday, November 3, 2015, wrote: > [PATCH] Limit the size of the data block passed to SHA1_Update() As an aid for reviewers, please include the version number of the patch, such as [PATCH vN] where "N" is the revision. format-patch's -v option can help automate this. >

Re: [PATCH 2/2] http: use credential API to handle proxy authentication

Knut Franke writes: > @@ -337,6 +342,24 @@ static void var_override(const char **var, char *value) > > static void init_curl_proxy_auth(CURL *result) > { > + if (proxy_auth.username) { > + struct strbuf s = STRBUF_INIT; Having this variable

Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

On Wed, Nov 4, 2015 at 1:35 PM, Junio C Hamano wrote: > Doug Kelly writes: > >> I think the patches I sent (a bit prematurely) address the >> remaining comments... I did find there was a relevant test in >> t5304 already, so I added a new test in the same

Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

On Wed, Nov 04, 2015 at 11:35:52AM -0800, Junio C Hamano wrote: > Doug Kelly writes: > > > I think the patches I sent (a bit prematurely) address the > > remaining comments... I did find there was a relevant test in > > t5304 already, so I added a new test in the same

Re: O_NONBLOCK under Windows (was: git.git as of tonight)

On 11/04/2015 12:00 AM, Stefan Beller wrote: > On Tue, Nov 3, 2015 at 1:03 PM, Johannes Sixt wrote: > The #ifdef assumes that Windows never will have O_NONBLOCK/F_GETFL) Does the following make more sense ? #if defined (O_NONBLOCK) && defined (F_GETFL) Or may be: #ifndef

Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

On Wed, Nov 04, 2015 at 01:56:38PM -0600, Doug Kelly wrote: > > I did wonder if we want to say anything about .bitmap files, though. > > If there is one without matching .idx and .pack, shouldn't we report > > just like we report .idx without .pack (or vice versa)? > > I think you're right --

Re: O_NONBLOCK under Windows (was: git.git as of tonight)

On Wed, Nov 4, 2015 at 11:59 AM, Torsten Bögershausen wrote: > On 11/04/2015 12:00 AM, Stefan Beller wrote: >> On Tue, Nov 3, 2015 at 1:03 PM, Johannes Sixt wrote: >> > The #ifdef assumes that Windows never will have O_NONBLOCK/F_GETFL) > > Does the following make

Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

On Wed, Nov 4, 2015 at 2:02 PM, Jeff King wrote: > On Wed, Nov 04, 2015 at 01:56:38PM -0600, Doug Kelly wrote: > >> > I did wonder if we want to say anything about .bitmap files, though. >> > If there is one without matching .idx and .pack, shouldn't we report >> > just like we

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

On Wed, Nov 4, 2015 at 10:14 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> The warning message is cluttering the output itself, >> so just report it once. >> >> Signed-off-by: Stefan Beller >> --- >> run-command.c | 20

Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

On Wed, Nov 04, 2015 at 02:08:21PM -0600, Doug Kelly wrote: > Currently there's no mtime-guarding logic (I dug up that conversation > earlier, though, but after I'd done the respin on this series)... OK, > in that case, I'll create a separate patch that tests/cleans up > .bitmap, but doesn't

Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

Doug Kelly writes: > I think the patches I sent (a bit prematurely) address the > remaining comments... I did find there was a relevant test in > t5304 already, so I added a new test in the same section (and > cleaned up some of the garbage it wasn't removing before). I'm >

Re: [PATCH v2 4/4] t5509: add basic tests for hideRefs

On Tuesday, November 3, 2015, Lukas Fleischer wrote: > Test whether regular and full hideRefs patterns work as expected when > namespaces are used. > > Helped-by: Eric Sunshine > Signed-off-by: Lukas Fleischer > --- > diff --git

Re: [PATCH 1/2] http: allow selection of proxy authentication method

Knut Franke writes: > diff --git a/http.c b/http.c > index 7da76ed..a786802 100644 > --- a/http.c > +++ b/http.c > @@ -305,6 +326,40 @@ static void init_curl_http_auth(CURL *result) > #endif > } > > +/* assumes *var is free-able */ This is not just "assumes",

Re: [PATCH v2 3/4] Add support for matching full refs in hideRefs

Junio C Hamano writes: > The semantics of "hideRefs" used to be that "refs can be hidden from > ls-remote" (i.e. prevents fetching, avoids contaminating "--mirror") > and "refs can be hidden from 'push'", but the objects being known > tips of histories that are complete, they

Re: [PATCH 2/2] http: use credential API to handle proxy authentication

On Wednesday, November 4, 2015, Knut Franke wrote: > Currently, the only way to pass proxy credentials to curl is by including them > in the proxy URL. Usually, this means they will end up on disk unencrypted, > one > way or another (by inclusion in ~/.gitconfig,

Re: Bug: stash save -u removes (some) ignored files

On 4 November 2015 at 03:23, Atousa Duprat wrote: >> felipe@felipe:testgit% git stash save -u > > This does the following: > $ GIT_TRACE=1 git stash save -u > [...] > 21:59:10.606094 git.c:348 trace: built-in: git 'clean' > '--force' '--quiet' '-d' > > git-clean

Re: [PATCH] Limit the size of the data block passed to SHA1_Update()

Junio C Hamano writes: >> ifdef BLK_SHA1 >> SHA1_HEADER = "block-sha1/sha1.h" >> LIB_OBJS += block-sha1/sha1.o >> else >> ifdef PPC_SHA1 >> SHA1_HEADER = "ppc/sha1.h" >> LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o >> else >> ifdef APPLE_COMMON_CRYPTO

Re: drop connectivity check for local clones

[I'm cc-ing the list, since I think this answer is of general interest.] On Wed, Nov 04, 2015 at 11:00:34AM +0100, Matej Buday wrote: > I have a question somewhat regarding this old commit of yours: > https://github.com/git/git/commit/125a05fd0b45416558923b753f6418c24208d443 > > Let me preface

Re: [PATCHv3 00/11] Expose the submodule parallelism to the user

Stefan Beller writes: > Where does it apply? > --- > This series applies on top of d075d2604c0f92045caa8d5bb6ab86cf4921a4ae (Merge > branch 'rs/daemon-plug-child-leak' into sb/submodule-parallel-update) and > replaces > the previous patches in sb/submodule-parallel-update >

[PATCH v4 0/2]

Changes in the fourth iteration: * update Documentation/technical/api-remote.txt to reflect the addition to struct remote * update documentation of http.proxy in Documentation/config.txt to mention the possibility of having git fill in missing proxy password * fix decl-after-stmt *

[PATCH 1/2] http: allow selection of proxy authentication method

CURLAUTH_ANY does not work with proxies which answer unauthenticated requests with a 307 redirect to an error page instead of a 407 listing supported authentication methods. Therefore, allow the authentication method to be set using the environment variable GIT_HTTP_PROXY_AUTHMETHOD or

[PATCH 2/2] http: use credential API to handle proxy authentication

Currently, the only way to pass proxy credentials to curl is by including them in the proxy URL. Usually, this means they will end up on disk unencrypted, one way or another (by inclusion in ~/.gitconfig, shell profile or history). Since proxy authentication often uses a domain user, credentials

Re: [PATCHv3 00/11] Expose the submodule parallelism to the user

On Wed, Nov 4, 2015 at 9:54 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> Where does it apply? >> --- >> This series applies on top of d075d2604c0f92045caa8d5bb6ab86cf4921a4ae (Merge >> branch 'rs/daemon-plug-child-leak' into

Re: [PATCHv3 02/11] run-command: report failure for degraded output just once

Stefan Beller writes: > The warning message is cluttering the output itself, > so just report it once. > > Signed-off-by: Stefan Beller > --- > run-command.c | 20 ++-- > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git

Re: [PATCHv3 00/11] Expose the submodule parallelism to the user

Stefan Beller writes: > So you are saying that reading the Windows cleanup patch > before the s/deinit/clear/ Patch by Rene makes it way easier to understand? No. The run-parallel API added in parallel-fetch that needs to be fixed up (because the topic is in 'next', my bad