Re: [PATCH 5/6] merge-recursive: copy $GITHEAD strings

Jeff King writes: > If $GITHEAD_1234abcd is set in the environment, we use its value as a > "better branch name" in generating conflict markers. However, we pick > these better names early in the process, and the return value from > getenv() is not guaranteed to stay valid. > > Let's make a copy

Re: [PATCH 4/6] init: make a copy of $GIT_DIR string

Jeff King writes: > We pass the result of getenv("GIT_DIR") to init_db() and assume that the > string remains valid. But that's not guaranteed across calls to setenv() > or even getenv(), although it often works in practice. Let's make a copy > of the string so that we follow the rules. > > Note

Re: [PATCH 2/6] commit: copy saved getenv() result

Jeff King writes: > We save the result of $GIT_INDEX_FILE so that we can restore it after > setting it to a new value and running add--interactive. However, the > pointer returned by getenv() is not guaranteed to be valid after calling > setenv(). This _usually_ works fine, but can fail if libc n

Re: [PATCH 1/6] get_super_prefix(): copy getenv() result

Jeff King writes: > The return value of getenv() is not guaranteed to remain valid across > multiple calls (nor across calls to setenv()). Since this function > caches the result for the length of the program, we must make a copy to > ensure that it is still valid when we need it. > > Reported-by

Re: [PATCH v2] diff: ensure correct lifetime of external_diff_cmd

Kim Gybels writes: > According to getenv(3)'s notes: > > The implementation of getenv() is not required to be reentrant. The > string pointed to by the return value of getenv() may be statically > allocated, and can be modified by a subsequent call to getenv(), > putenv(3), seten

Re: [PATCH] remote: check config validity before creating rewrite struct

Jeff King writes: > This was a minor cleanup I mentioned in: > > https://public-inbox.org/git/20181122173109.gi28...@sigill.intra.peff.net/ > > but didn't get picked up. Yeah, that does sound vaguely familiar ;-) Thanks, will queue. > > remote.c | 4 ++-- > 1 file changed, 2 insertions(+),

[PATCH v2 09/11] merge-recursive.c: remove implicit dependency on the_repository

Signed-off-by: Nguyễn Thái Ngọc Duy --- merge-recursive.c | 45 - 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 28f44c73ec..a596d95739 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @

[PATCH v2 07/11] sha1-name.c: remove implicit dependency on the_index

This kills the_index dependency in get_oid_with_context() but for get_oid() and friends, they still assume the_repository (which also means the_index). Unfortunately the widespread use of get_oid() will make it hard to make the conversion now. We probably will add repo_get_oid() at some point and

[PATCH v2 08/11] merge-recursive.c: remove implicit dependency on the_index

Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/am.c | 2 +- builtin/checkout.c| 2 +- builtin/merge-recursive.c | 2 +- builtin/merge.c | 2 +- merge-recursive.c | 148 -- merge-recursive.h | 6 +- seque

git@vger.kernel.org

Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/commit.c | 2 +- builtin/describe.c | 2 +- builtin/diff.c | 2 +- builtin/rebase.c | 5 ++--- cache.h| 6 -- read-cache.c | 14 -- repository.h | 6 ++ wt-status.c| 2 +- 8 files ch

[PATCH v2 10/11] read-cache.c: remove the_* from index_has_changes()

Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/am.c | 6 +++--- cache.h | 6 +++--- merge-recursive.c | 2 +- read-cache.c | 12 +--- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 611712dc95..a9ffc92eaa 100644 --

[PATCH v2 11/11] cache.h: flip NO_THE_INDEX_COMPATIBILITY_MACROS switch

>From now on, by default index compat macros are off because they could hide the_index dependency. Only those in builtin can use it (and even so should be avoided if possible). Signed-off-by: Nguyễn Thái Ngọc Duy --- attr.c | 1 - builtin/add.c

[PATCH v2 05/11] read-cache.c: kill read_index()

read_index() shares the same problem as hold_locked_index(): it assumes $GIT_DIR/index. Move all call sites to repo_read_index() instead. read_index_preload() and read_index_unmerged() are also killed as a consequence. Signed-off-by: Nguyễn Thái Ngọc Duy --- apply.c | 2 +- blame.c

[PATCH v2 02/11] notes-utils.c: remove the_repository references

Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/am.c | 2 +- builtin/commit.c | 2 +- builtin/notes.c | 21 + notes-merge.c| 4 ++-- notes-utils.c| 17 ++--- notes-utils.h| 11 --- sequencer.c | 7 --- sequencer.h | 3 ++-

[PATCH v2 04/11] checkout: avoid the_index when possible

Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 08b0ac48f3..1b672a9fd9 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -284,7 +284,7 @@ static int checkout_path

[PATCH v2 03/11] repository.c: replace hold_locked_index() with repo_hold_locked_index()

hold_locked_index() assumes the index path at $GIT_DIR/index. This is not good for places that take an arbitrary index_state instead of the_index, which is basically everywhere except builtin/. Replace it with repo_hold_locked_index(). hold_locked_index() remains as a wrapper around repo_hold_lock

[PATCH v2 01/11] grep: use grep_opt->repo instead of explict repo argument

This command is probably the first one that operates on a repository other than the_repository, in f9ee2fcdfa (grep: recurse in-process using 'struct repository' - 2017-08-02). An explicit 'struct repository *' was added in that commit to pass around the repository that we're supposed to grep from.

[PATCH v2 00/11] Remove the_index, the final part

v2 fixes Martin's comment in 2/10 and also includes a new patch about grep_opt->repo that was sent and reviewed during rc time [1]. It's kind of the same topic so I include it here instead of resending it separately. [1] https://public-inbox.org/git/20181118163851.32178-1-pclo...@gmail.com/ Range

Re: [PATCH v4 4/6] revision: implement sparse algorithm

"Derrick Stolee via GitGitGadget" writes: > From: Derrick Stolee > > When enumerating objects to place in a pack-file during 'git > pack-objects --revs', we discover the "frontier" of commits > that we care about and the boundary with commit we find > uninteresting. From that point, we walk tree

Re: [PATCH v4 2/6] list-objects: consume sparse tree walk

"Derrick Stolee via GitGitGadget" writes: > From: Derrick Stolee > > When creating a pack-file using 'git pack-objects --revs' we provide > a list of interesting and uninteresting commits. For example, a push > operation would make the local topic branch be interesting and the > known remote ref

Re: [PATCH v4 3/6] pack-objects: add --sparse option

"Derrick Stolee via GitGitGadget" writes: > From: Derrick Stolee > > Add a '--sparse' option flag to the pack-objects builtin. This > allows the user to specify that they want to use the new logic > for walking trees. This logic currently does not differ from the > existing output, but will in a

[WIP 7/4] upload-pack: refactor reading of pack-objects out

Subsequent patches will change how the output of pack-objects is processed, so extract that processing into its own function. Currently, at most 1 character can be buffered (in the "buffered" local variable). One of those patches will require a larger buffer, so replace that "buffered" local varia

[WIP 5/4] Documentation: order protocol v2 sections

The current C Git implementation expects Git servers to follow a specific order of sections when transmitting protocol v2 responses, but this is not explicit in the documentation. Make the order explicit. Signed-off-by: Jonathan Tan --- Documentation/technical/protocol-v2.txt | 18 --

[PATCH 3/4] {fetch,upload}-pack: sideband v2 fetch response

Currently, a response to a fetch request has sideband support only while the packfile is being sent, meaning that the server cannot send notices until the start of the packfile. Extend sideband support in protocol v2 fetch responses to the whole response. upload-pack will advertise it if the uploa

[PATCH 4/4] tests: define GIT_TEST_SIDEBAND_ALL

Define a GIT_TEST_SIDEBAND_ALL environment variable meant to be used from tests. When set to true, this overrides uploadpack.allowsidebandall to true, allowing the entire test suite to be run as if this configuration is in place for all repositories. As of this patch, all tests pass whether GIT_TE

[WIP 8/4] upload-pack: send part of packfile response as uri

This is a partial implementation of upload-pack sending part of its packfile response as URIs. The client is not fully implemented - it knows to ignore the "packfile-uris" section, but because it does not actually fetch those URIs, the returned packfile is incomplete. A test is included to show th

[PATCH 0/4] Sideband the whole fetch v2 response

This is on ms/packet-err-check. This is a follow-up of my earlier work to allow partial CDN offloading of fetch v2 response packfiles, which had the issue of having to buffer (or suspend) progress and keepalive messages because we didn't have sideband until the packfile section started (as I wrote

[WIP 6/4] Documentation: add Packfile URIs design doc

Signed-off-by: Jonathan Tan --- Documentation/technical/packfile-uri.txt | 83 Documentation/technical/protocol-v2.txt | 6 +- 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 Documentation/technical/packfile-uri.txt diff --git a/Documentation/techn

[PATCH 1/4] pkt-line: introduce struct packet_writer

It will be convenient for a future patch if writing options (specifically, whether the written data is to be multiplexed) could be controlled from a single place, so create struct packet_writer to serve as that place, and modify upload-pack to use it. Currently, it only stores the output fd, but a

[PATCH 2/4] sideband: reverse its dependency on pkt-line

A subsequent patch will teach struct packet_reader a new field that, if set, instructs it to interpret read data as multiplexed. This will create a dependency from pkt-line to sideband. To avoid a circular dependency, split recv_sideband() into 2 parts: the reading loop (left in recv_sideband()) a

[PATCH 5/6] merge-recursive: copy $GITHEAD strings

If $GITHEAD_1234abcd is set in the environment, we use its value as a "better branch name" in generating conflict markers. However, we pick these better names early in the process, and the return value from getenv() is not guaranteed to stay valid. Let's make a copy of the returned string. And to

[PATCH 6/6] builtin_diff(): read $GIT_DIFF_OPTS closer to use

The value returned by getenv() is not guaranteed to remain valid across other environment function calls. But in between our call and using the value, we run fill_textconv(), which may do quite a bit of work, including spawning sub-processes. We can make this safer by calling getenv() right before

[PATCH 4/6] init: make a copy of $GIT_DIR string

We pass the result of getenv("GIT_DIR") to init_db() and assume that the string remains valid. But that's not guaranteed across calls to setenv() or even getenv(), although it often works in practice. Let's make a copy of the string so that we follow the rules. Note that we need to mark it with UN

[PATCH 2/6] commit: copy saved getenv() result

We save the result of $GIT_INDEX_FILE so that we can restore it after setting it to a new value and running add--interactive. However, the pointer returned by getenv() is not guaranteed to be valid after calling setenv(). This _usually_ works fine, but can fail if libc needs to reallocate the envir

[PATCH 3/6] config: make a copy of $GIT_CONFIG string

cmd_config() points our source filename pointer at the return value of getenv(), but that value may be invalidated by further calls to environment functions. Let's copy it to make sure it remains valid. We don't need to bother freeing it, as it remains part of the whole-process global state until

[PATCH 1/6] get_super_prefix(): copy getenv() result

The return value of getenv() is not guaranteed to remain valid across multiple calls (nor across calls to setenv()). Since this function caches the result for the length of the program, we must make a copy to ensure that it is still valid when we need it. Reported-by: Yngve N. Pettersen Signed-of

[PATCH 0/6] getenv() timing fixes

Similar to the recent: https://public-inbox.org/git/20190109221007.21624-1-kgyb...@infogroep.be/ there are some other places where we do not follow the POSIX rule that getenv()'s return value may be invalidated by other calls to getenv() or setenv(). For the most part we haven't noticed becaus

Re: [PATCH v4 1/6] revision: add mark_tree_uninteresting_sparse

On 1/11/2019 3:25 PM, Junio C Hamano wrote: Junio C Hamano writes: So, I assumed that the implementation was wrong, but it is the other way around. You do mean to pick only already uninteresting trees out of "set" and mark its reachables. One thing that would make me worried is what help the

[PATCH] userdiff: Add a builtin pattern for dts files

The Linux kernel receives many patches to the devicetree files each release. The hunk header for those patches typically show nothing, making it difficult to figure out what node is being modified without applying the patch or opening the file and seeking to the context. Let's add a builtin 'dts' p

[PATCH v2] diff: ensure correct lifetime of external_diff_cmd

According to getenv(3)'s notes: The implementation of getenv() is not required to be reentrant. The string pointed to by the return value of getenv() may be statically allocated, and can be modified by a subsequent call to getenv(), putenv(3), setenv(3), or unsetenv(3). Since str

Re: [PATCH v4 1/6] revision: add mark_tree_uninteresting_sparse

Junio C Hamano writes: >> In preparation for a new algorithm that walks fewer trees when >> creating a pack from a set of revisions, create a method that >> takes an oidset of tree oids and marks reachable objects as >> UNINTERESTING. >> ... >> There is one new assumption in this approach: we are

Re: [PATCH v4 1/6] revision: add mark_tree_uninteresting_sparse

"Derrick Stolee via GitGitGadget" writes: > From: Derrick Stolee > > In preparation for a new algorithm that walks fewer trees when > creating a pack from a set of revisions, create a method that > takes an oidset of tree oids and marks reachable objects as > UNINTERESTING. > > The current imple

[PATCH] remote: check config validity before creating rewrite struct

When parsing url.foo.insteadOf, we call make_rewrite() and only then check to make sure the value is a string (and return an error if it isn't). This isn't quite a leak, because the struct we allocate is part of a global array, but it does leave a funny half-finished struct. In practice, it doesn'

Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings

On Fri, Jan 11, 2019 at 10:03:01AM -0800, Junio C Hamano wrote: > SZEDER Gábor writes: > > > On Thu, Jan 10, 2019 at 01:22:00PM -0800, Junio C Hamano wrote: > >> Ævar Arnfjörð Bjarmason writes: > >> ... > >> > I.e. is this another case where we're blindly fixing bugs but should > >> > just re-i

Re: [PATCH v4 0/8] Reimplement rebase --merge via interactive machinery

Hi Junio, A small update... On Mon, Jan 7, 2019 at 12:39 PM Elijah Newren wrote: > On Mon, Jan 7, 2019 at 12:11 PM Junio C Hamano wrote: > > Junio C Hamano writes: > > > > > Elijah Newren writes: > > > > > >> On Tue, Dec 11, 2018 at 8:11 AM Elijah Newren wrote: > > >>> > > >>> This series co

Re: [PATCH 1/5] compat/obstack: fix -Wcast-function-type warnings

SZEDER Gábor writes: > On Thu, Jan 10, 2019 at 01:22:00PM -0800, Junio C Hamano wrote: >> Ævar Arnfjörð Bjarmason writes: >> ... >> > I.e. is this another case where we're blindly fixing bugs but should >> > just re-import upstream's code instead? >> >> Good point. I am inclined to queue the

Re: [PATCH v5 1/5] t5323: test cases for git-pack-redundant

Jiang Xin writes: > Junio C Hamano 于2019年1月11日周五 上午5:11写道： >> >> Jiang Xin writes: >> >> > From: Jiang Xin >> > +create_commits() >> > +{ >> >> Style (see Documentation/CodingGuidelines). > > OK, parenthese after function name. >> >> > +create_pack_1() >> > +{ >> > + P1=$(cd .git/objects/p

Re: [PATCH] log: add %S option (like --source) to log --format

Issac Trotts writes: > Sounds good. Btw, did you queue it yet? I didn't see it at the mirror: > https://github.com/git/git/commits/master. No patch goes to 'master' directly. Once we see that a patch is in reviewable shape during the mailing list discussion, we wait for a few more days and un

Re: [PATCH] log: add %S option (like --source) to log --format

Issac Trotts writes: > On Tue, Jan 8, 2019 at 2:21 PM Junio C Hamano wrote: >> >> issac.tro...@gmail.com writes: >> >> > From: Issac Trotts >> >> Heh, I'll edit this line to match S-o-b: below. > > Thanks. Otherwise I have to set up git send-email again on my work laptop. This in-body From: li

Re: [PATCH 1/1] gc/repack: release packs when needed

Jeff King writes: > On Thu, Jan 10, 2019 at 01:01:36PM -0800, Junio C Hamano wrote: > >> > diff --git a/builtin/gc.c b/builtin/gc.c >> > index 871a56f1c5..df90fd7f51 100644 >> > --- a/builtin/gc.c >> > +++ b/builtin/gc.c >> > @@ -659,8 +659,10 @@ int cmd_gc(int argc, const char **argv, const char

Re: [PATCH 1/1] gc/repack: release packs when needed

On Thu, Jan 10, 2019 at 01:01:36PM -0800, Junio C Hamano wrote: > > diff --git a/builtin/gc.c b/builtin/gc.c > > index 871a56f1c5..df90fd7f51 100644 > > --- a/builtin/gc.c > > +++ b/builtin/gc.c > > @@ -659,8 +659,10 @@ int cmd_gc(int argc, const char **argv, const char > > *prefix) > > > >

Re: Git rebase --exec cannot run git commands affecting other repos

On Thu, Jan 10, 2019 at 09:48:42PM +, Samir Benmendil wrote: > > If the user wants to work in a different repository, the > > environments that tells Git about the original repository can be > > unset to do so, which is a very much deliberately designed > > behaviour, primarily to help those w

Re: Make "git log --count" work like "git rev-list"

On Thu, Jan 10, 2019 at 02:19:51PM -0800, Junio C Hamano wrote: > > I know "--follow" is a bit hacky in general, but I think there are other > > cases where log_tree_diff() may decide not to show a commit (maybe > > without --root, though I guess that's the default these days). > > > > I dunno. Ce

Re: [PATCH 3/5] match-trees: use hashcpy to splice trees

On Fri, Jan 11, 2019 at 09:51:06AM -0500, Jeff King wrote: > I think rewrite_here needs to be a direct pointer into the buffer, since > we plan to modify it. > > I think rewrite_with is correct to be an object_id. It's either the oid > passed in from the caller, or the subtree we generated (in wh

Re: [PATCH 3/5] match-trees: use hashcpy to splice trees

On Thu, Jan 10, 2019 at 11:55:51PM +, brian m. carlson wrote: > > I think the only reason they are "struct object_id" is because that's > > what tree_entry_extract() returns. Should we be providing another > > function to allow more byte-oriented access? > > The reason is we recursively call

Re: [PATCH 0/5] tree-walk object_id refactor

On Fri, Jan 11, 2019 at 12:17:50AM +, brian m. carlson wrote: > > I think this is really the only safe and sane solution. We resisted it > > because of the cost of the extra copies (especially the > > update_tree_entry() one). But I don't know that anybody actually > > measured it. Do you have

Re: [PATCH 0/7] Turn git add-i into built-in

On 20-Dec-18 1:34 PM, Johannes Schindelin wrote: From: "Slavica Đukić via GitGitGadget" This is the first version of a patch series to start porting git-add--interactive from Perl to C. Daniel Ferreira's patch series used as a head start: https://public-inbox.org/git/1494907234-28903-1-git-se

t0021-conversion: flaky filter process test

Hi Lars, I see occasional failures in the test 'required process filter should filter data' in 't0021-conversion.sh', added in edcc85814c (convert: add filter..process option, 2016-10-16); most often in the macOS Travis CI build jobs, but it's an OS-independent timing issue. That's an enormous te

Re: git rebase: retain original head?

Hi Markus, On Wed, 9 Jan 2019, Markus Wiederkehr wrote: > On Wed, Jan 9, 2019 at 3:05 PM Johannes Schindelin > wrote: > > > > Having said that, it is an unintended regression in the built-in rebase. > > Markus, could you come up with a minimal test case, preferably in the form > > of a patch to