Re: git branch doesn't allow me to forcibly delete branch which was checked out in a now-deleted worktree dir

On Thu, Jul 7, 2016 at 8:44 PM, Erik Johnson wrote: > % git branch -D archive-extracted-xz > error: Cannot delete branch 'archive-extracted-xz' checked out at > '/home/erik/git/salt/archive-extracted-xz' This is from commit f292244 (branch -d: refuse deleting a branch which

[PATCH v3 1/4] test-lib.sh: introduce and use $_EMPTY_TREE

This is a special SHA1. Let's keep it at one place, easier to replace later when the hash change comes, easier to recognize. Start with an underscore to reduce the chances somebody may override it without realizing it's predefined. Signed-off-by: Nguyễn Thái Ngọc Duy ---

[PATCH v3 3/4] cache-tree.c: fix i-t-a entry skipping directory updates sometimes

Commit 3cf773e (cache-tree: fix writing cache-tree when CE_REMOVE is present - 2012-12-16) skips i-t-a entries when building trees objects from the index. Unfortunately it may skip too much. The code in question checks if an entry is an i-t-a one, then no tree entry will be written. But it does

[PATCH v3 0/4] cache-tree building fix in the presence of ita entries

v3 fixes 2/2 (which is 4/4 now), allowing cache-tree to generate an empty tree if the index contains nothing but i-t-a entries. Since empty tree SHA-1 is involved and we don't want to make it harder to move away from SHA-1 in future, 1/2 and 2/2 are added to keep SHA-1 for empty tree (and blob

[PATCH v3 2/4] test-lib.sh: introduce and use $_EMPTY_BLOB

Similar to $_EMPTY_TREE this makes it easier to recognize this special SHA-1 and change hash later. Signed-off-by: Nguyễn Thái Ngọc Duy --- t/t1011-read-tree-sparse-checkout.sh | 8 t/t1700-split-index.sh | 24

[PATCH v3 4/4] cache-tree: do not generate empty trees as a result of all i-t-a subentries

If a subdirectory contains nothing but i-t-a entries, we generate an empty tree object and add it to its parent tree. Which is wrong. Such a subdirectory should not be added. Note that this has a cascading effect. If subdir 'a/b/c' contains nothing but i-t-a entries, we ignore it. But then if

Re: gc and repack ignore .git/*HEAD when checking reachability

On Fri, Jul 08, 2016 at 07:50:53PM -0400, Theodore Ts'o wrote: > On Fri, Jul 08, 2016 at 01:30:06PM -0700, Junio C Hamano wrote: > > > > I can imagine I'd start hacking on a project that I rarely touch, give up > > resolving a "git pull" from an unconfigured place (hence, some stuff is > > only

Re: git branch doesn't allow me to forcibly delete branch which was checked out in a now-deleted worktree dir

On Fri, Jul 08, 2016 at 09:58:26PM -0700, Jacob Keller wrote: On Fri, Jul 8, 2016 at 7:22 PM, Erik Johnson wrote: On Fri, Jul 08, 2016 at 05:41:17PM -0700, Jacob Keller wrote: It is possible we should update "git branch -d" should perform a worktree prune first, since that

Re: git branch doesn't allow me to forcibly delete branch which was checked out in a now-deleted worktree dir

On Fri, Jul 8, 2016 at 7:22 PM, Erik Johnson wrote: > On Fri, Jul 08, 2016 at 05:41:17PM -0700, Jacob Keller wrote: >> It is possible we should update "git branch -d" should perform a >> worktree prune first, since that would enable it to determine that you >> deleted the

Git GUI Guesses Incorrect Language

Here is a screenshot of what I am talking about: http://i.imgur.com/9ZRkeYF.png This is the `git gui` program. Just before the first line of my code, it says "C++ source, ASCII text". That file is a JavaScript/ES6 file. The ".jsx" file extension signifies that it may also contain Facebook's

Re: git branch doesn't allow me to forcibly delete branch which was checked out in a now-deleted worktree dir

On Fri, Jul 08, 2016 at 05:41:17PM -0700, Jacob Keller wrote: On Thu, Jul 7, 2016 at 5:36 PM, Erik Johnson wrote: I'm not expecting _any_ git branch command to prune worktrees, but a branch _deletion_ shouldn't fail because git thinks the branch is checked out in a worktree

Re: Fast-forward able commit, otherwise fail

On Fri, Jul 8, 2016 at 3:19 PM, Junio C Hamano wrote: > David Lightle writes: >> In fact, I just noticed that GitLab has built in the functionality I'm >> looking for even, which is what they call "Merge commit with >> semi-linear history" but I'm asking

Re: git branch doesn't allow me to forcibly delete branch which was checked out in a now-deleted worktree dir

On Thu, Jul 7, 2016 at 5:36 PM, Erik Johnson wrote: > I'm not expecting _any_ git branch command to prune worktrees, but a > branch _deletion_ shouldn't fail because git thinks the branch is > checked out in a worktree that doesn't exist anymore. Even in the > scenario where

[PATCHv4 0/4] Push options

Thanks Junio, Jeff, Jonathan for discussion and feedback! I went over the emails again and we seem to agree that the initial design (in v3) was sane and the error messages and reporting for corner cases were to be dismissed as "it happens as often as 'BUG:' messages appear, so let's not care too

[PATCH 4/4] add a test for push options

The functions `mk_repo_pair` as well as `test_refs` are borrowed from t5543-atomic-push, with additional hooks installed. Signed-off-by: Stefan Beller --- t/t5545-push-options.sh | 101 1 file changed, 101 insertions(+)

[PATCH 1/4] push options: {pre,post}-receive hook learns about push options

The environment variable GIT_PUSH_OPTION_COUNT is set to the number of push options sent, and GIT_PUSH_OPTION_{0,1,..} is set to the transmitted option. The code is not executed as the push options are set to NULL, nor is the new capability advertised. There was some discussion back and forth

[PATCH 3/4] push: accept push options

This implements everything that is required on the client side to make use of push options from the porcelain push command. Signed-off-by: Stefan Beller --- Documentation/git-push.txt | 8 +++- builtin/push.c | 21 ++--- send-pack.c

[PATCH 2/4] receive-pack: implement advertising and receiving push options

The pre/post receive hook may be interested in more information from the user. This information can be transmitted when both client and server support the "push-options" capability, which when used is a phase directly after update commands ended by a flush pkt. Similar to the atomic option, the

Re: gc and repack ignore .git/*HEAD when checking reachability

On Fri, Jul 08, 2016 at 01:30:06PM -0700, Junio C Hamano wrote: > > I can imagine I'd start hacking on a project that I rarely touch, give up > resolving a "git pull" from an unconfigured place (hence, some stuff is > only reachable from FETCH_HEAD), and after 2*N days, come back > to the

What's cooking in git.git (Jul 2016, #03; Fri, 8)

Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. The first few batches for this

Re: [PATCH 3/4] push: accept push options

On Thu, Jul 7, 2016 at 1:52 PM, Junio C Hamano wrote: > Stefan Beller writes: > >> +-L:: >> +--push-option:: >> + Transmit the given string to the server, which passes them to >> + the pre-receive as well as the post-receive hook. Only C strings >>

Re: [PATCH/RFC] archive: allow archiving of reachable sha1

Nicolas Cornu writes: > Remotely specify a tree-ish by a sha1 is now valid even if > uploadarchive.allowunreachable is false only if this sha1 is reachable > from a branch or a tag reference. We consider those last one to be > public. > > Signed-off-by: Nicolas Cornu

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 8, 2016 at 3:46 PM, Jeff King wrote: > Sorry, I meant converting die() into: > > rp_error(...); > die(...); > > possibly via an rp_die() helper. The existing rp_error() cases would > remain untouched. Oh I see! That makes a lot of sense. -- To unsubscribe from

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 08, 2016 at 03:43:27PM -0700, Stefan Beller wrote: > On Fri, Jul 8, 2016 at 3:35 PM, Jeff King wrote: > > > > Yes. I haven't been following the intermediate discussion and patches, > > but I don't see anything wrong with the general design above. I think > > you do

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 8, 2016 at 3:35 PM, Jeff King wrote: > > Yes. I haven't been following the intermediate discussion and patches, > but I don't see anything wrong with the general design above. I think > you do need to use rp_error() to get the die message to the client for > non-ssh

[PATCH/RFC] archive: allow archiving of reachable sha1

Remotely specify a tree-ish by a sha1 is now valid even if uploadarchive.allowunreachable is false only if this sha1 is reachable from a branch or a tag reference. We consider those last one to be public. Signed-off-by: Nicolas Cornu --- Do you think this patch is too much

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 08, 2016 at 03:29:09PM -0700, Stefan Beller wrote: > > In the malicious case, the client says "I'll send you 10 push option > > with an upper bound of 1024K", and then sends gigabytes anyway. Either > > way the server has to react to what is sent, not what is promised. > > Well that

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 8, 2016 at 3:21 PM, Jeff King wrote: > On Fri, Jul 08, 2016 at 03:17:13PM -0700, Stefan Beller wrote: > >> >If people are seeing these in >> > routine use, then the limits are set too low, and this should happen >> > roughly as often as a BUG assertion, and IMHO should

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 08, 2016 at 03:17:13PM -0700, Stefan Beller wrote: > >If people are seeing these in > > routine use, then the limits are set too low, and this should happen > > roughly as often as a BUG assertion, and IMHO should be treated roughly > > the same: don't bother with translation, and

Re: Fast-forward able commit, otherwise fail

David Lightle writes: > In your above scenario with Alice & Bob, wouldn't that same issue come > up in _any_ rebase workflow (--ff-only)? From what I've read this is > a typical consequence of requiring fast-forward merges; to be > effective, the rebase step must be more

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 8, 2016 at 2:46 PM, Jeff King wrote: > On Fri, Jul 08, 2016 at 11:57:20AM -0700, Stefan Beller wrote: > >> >> Sorry to butt into the conversation late, but: I am not yet convinced. >> >> >> >> Is the idea that if the push options were very large, this would save >> >>

Re: [PATCH] am: ignore return value of write_file()

On Fri, Jul 08, 2016 at 08:44:28PM +0200, René Scharfe wrote: > > The question is whether it makes sense for write_file() to die(). It is a > > library function and not every caller can be happy with that function to > > exit the program when some file could not be written, without a chance to >

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 08, 2016 at 11:57:20AM -0700, Stefan Beller wrote: > >> Sorry to butt into the conversation late, but: I am not yet convinced. > >> > >> Is the idea that if the push options were very large, this would save > >> the client from the cost of sending them? > > > > Not really. I have no

Re: [PATCH 2/2] avoid using sha1_to_hex output as printf format

On Fri, Jul 08, 2016 at 10:09:28AM -0700, Junio C Hamano wrote: > Junio C Hamano writes: > > > As to the hunk to commit.c that was dropped in this round, the only > > caller of print_commit_list() is bisect.c, and it passes "%s\n" to > > format_cur and format_last, it seems,

Re: [PATCH v2] am: counteract gender bias

Johannes Schindelin writes: > Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), i.e. for > almost 11 years already, we used a male form to describe "the other > tree". This actually is older than that commit by 9 commits. I may have said it already,

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

Junio C Hamano writes: > I've tentatively queued the following (it's the same as "Here is > another approach." I sent earlier, but with a real log message) on > 'pu'. And here is yet another approach, which probably is even less intrusive. Whatever alternative we would end

Re: gc and repack ignore .git/*HEAD when checking reachability

On Fri, Jul 8, 2016 at 12:25 PM, Theodore Ts'o wrote: > On Fri, Jul 08, 2016 at 10:14:33AM -0700, Junio C Hamano wrote: >> >> It cannot be "anything directly under .git that has all-caps name >> that ends with _HEAD". The ones we write we know are going to be >> removed at some

Re: gc and repack ignore .git/*HEAD when checking reachability

On Fri, Jul 08, 2016 at 10:14:33AM -0700, Junio C Hamano wrote: > Josh Triplett writes: > > > That sounds reasonable. And if they *do* end up taking any time to > > traverse, it's because they weren't reachable from other anchoring > > points, so taking the extra time to

Re: Fast-forward able commit, otherwise fail

I've been trying to think of just what to say in response, so it has taken me some time as well as I've been experimenting with our workflow to try some alternate approaches while we still can. In your above scenario with Alice & Bob, wouldn't that same issue come up in _any_ rebase workflow

Re: gc and repack ignore .git/*HEAD when checking reachability

On Fri, Jul 08, 2016 at 10:14:33AM -0700, Junio C Hamano wrote: > > It cannot be "anything directly under .git that has all-caps name > that ends with _HEAD". The ones we write we know are going to be > removed at some point in time (e.g. "git reset", "git bisect reset", > "git merge --abort",

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

Torsten Bögershausen writes: > Did that experiment made it to a branch somewhere ? I've tentatively queued the following (it's the same as "Here is another approach." I sent earlier, but with a real log message) on 'pu'. -- >8 -- Subject: [PATCH] merge: avoid "safer crlf" during

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

On Fri, Jul 8, 2016 at 11:39 AM, Junio C Hamano wrote: > Jonathan Nieder writes: > >> Hi, >> >> Junio C Hamano wrote: >>> Stefan Beller writes: >> > More importantly, if we plan to make this configurable and not make > the limit

Re: [PATCH 0/8] write_file cleanups

Am 08.07.2016 um 11:04 schrieb Jeff King: Here it is. There actually weren't that many spots to clean up, as quite a few of them have a "twist" where they want to do something clever, like open the file and feed the descriptor to a sub-function, or open with funny things like O_EXCL. But still,

Re: [PATCH] am: ignore return value of write_file()

Hi Dscho, Am 08.07.2016 um 08:33 schrieb Johannes Schindelin: On Thu, 7 Jul 2016, René Scharfe wrote: write_file() either returns 0 or dies, so there is no point in checking its return value. The question is whether it makes sense for write_file() to die(). It is a library function and not

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

Jonathan Nieder writes: > Hi, > > Junio C Hamano wrote: >> Stefan Beller writes: > More importantly, if we plan to make this configurable and not make the limit a hardwired constant of the wire protocol, it may be better to advertise

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

Junio C Hamano writes: > Not yet. As I called it "experiment", it was merely to demonstrate > that there are less intrusive ways to kill the "safer crlf" we may > want to consider first before passing an extra blob object name > around. Here is another approach, that

Re: [PATCH 2/4] receive-pack: implement advertising and receiving push options

Hi, Junio C Hamano wrote: > Stefan Beller writes: >>> More importantly, if we plan to make this configurable and not make >>> the limit a hardwired constant of the wire protocol, it may be >>> better to advertise push-options capability with the limit, e.g. >>>

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

Torsten Bögershausen writes: >> And then with this further on top: >> >> diff --git a/merge-recursive.c b/merge-recursive.c >> index b880ae5..628c8ed 100644 >> --- a/merge-recursive.c >> +++ b/merge-recursive.c >> @@ -202,6 +202,9 @@ static int add_cacheinfo(unsigned int mode,

Git 2.8.1 - bug in patience diff algorithm when used with --ignore-space-at-eol?

When diffing with --patience and --ignore-space-at-eol, a change that adds or removes just one character a the end of a line isn't picked up. Other diff algorithms don't suffer from this and patience doesn't if I don't put --ignore-space-at-eol. See the following output for a proof: $ git diff

Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries

Duy Nguyen writes: > I thought about that too, then did a grep which showed empty sha1 tree > was used elsewhere. And thought of sending a patch to define > $EMPTY_SHA1 in test-lib-functions.sh or somewhere so we don't hard > code it everywhere, but I didn't. But yeah ls-tree

Re: gc and repack ignore .git/*HEAD when checking reachability

Josh Triplett writes: > That sounds reasonable. And if they *do* end up taking any time to > traverse, it's because they weren't reachable from other anchoring > points, so taking the extra time to traverse them seems fine. The only thing that is hard is to clearly

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

On 07/08/2016 06:36 PM, Junio C Hamano wrote: Torsten Bögershausen writes: I dunno. I really do not like that extra sha1 argument added all over the callchain by this patch. Or did you mean other calls to add_cacheinfo()? I didn't mean too much - the whole call chain

Re: [PATCH 2/2] avoid using sha1_to_hex output as printf format

Junio C Hamano writes: > As to the hunk to commit.c that was dropped in this round, the only > caller of print_commit_list() is bisect.c, and it passes "%s\n" to > format_cur and format_last, it seems, so that suggests a more > obvious direction for cleaning things up, I would

Re: [PATCH 2/2] avoid using sha1_to_hex output as printf format

Jeff King writes: > On Fri, Jul 08, 2016 at 05:25:26AM -0400, Jeff King wrote: > >> diff --git a/commit.c b/commit.c >> index 3f4f371..9603379 100644 >> --- a/commit.c >> +++ b/commit.c >> @@ -1623,7 +1623,7 @@ void print_commit_list(struct commit_list *list, >> { >> for ( ;

Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries

On Fri, Jul 8, 2016 at 5:53 PM, Junio C Hamano wrote: > Duy Nguyen writes: > >> Yeah that's better. So the squash patch is something like this > > Rather... > >> diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh >> index a19f06b..80880b7 100755 >>

Re: [PATCH v13 11/20] index-helper: use watchman to avoid refreshing index with lstat()

BTW just for me to have some perspective, roughly how many directories and files are there in the worktree of this repo? -- Duy -- 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

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

Torsten Bögershausen writes: >> I dunno. I really do not like that extra sha1 argument added all >> over the callchain by this patch. >> >> Or did you mean other calls to add_cacheinfo()? > > I didn't mean too much - the whole call chain touches code where I > am not able to

Re: [PATCH v13 11/20] index-helper: use watchman to avoid refreshing index with lstat()

On Wed, Jul 06, 2016 at 04:54:00PM +, Ben Peart wrote: > Duy Nguyen gmail.com> writes: > > > > > First step would be enabling that because besides directory > > traversing, this code does a lot of string processing that's hopefully > > eliminated by untracked cache extension. I cut

Re: [PATCH] config.c: fix potential number truncation in git_parse_signed()

On Wed, Jul 6, 2016 at 9:33 PM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> clang -Wabsolute-value on IA-32 architecture complains that "absolute >> value function 'labs' given an argument of type 'intmax_t' (aka 'long >> long') but has

Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries

Duy Nguyen writes: > Yeah that's better. So the squash patch is something like this Rather... > diff --git a/t/t2203-add-intent.sh b/t/t2203-add-intent.sh > index a19f06b..80880b7 100755 > --- a/t/t2203-add-intent.sh > +++ b/t/t2203-add-intent.sh > @@ -107,7 +107,9 @@

Re: [PATCH v2 2/2] cache-tree: do not generate empty trees as a result of all i-t-a subentries

On Thu, Jul 07, 2016 at 11:52:58AM -0700, Junio C Hamano wrote: > Duy Nguyen writes: > > > I'll deal with that separately. Let's focus on cache-tree only this > > time. So how about this on top? > > I was hoping that you would limit the scope of the test to check if >

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

On 07/07/16 20:43, Junio C Hamano wrote: > Torsten Bögershausen writes: > >> This is the call stack: >> >> merge-recursive.c/add_cacheinfo(unsigned int mode, const unsigned char *sha1, >>const char *path, int stage, int refresh, int options) >> { >>

Dear Beneficiary,

Dear Beneficiary, I'm happy to inform you that your email address has been selected among those that have compensation payment of ($2.5 Million USD), The INTERNATIONAL MONETARY FUND contacted us for the release of your money a couple of hours ago due to your allocated security code. We are

Integer-overflow in read-cache.c can cause buffer-overflow (bug requires a crafted/corrupted index)

Hi, I had a procastination episode today and wrote https://glandium.org/blog/?p=3659 . It started with me looking what git was doing with the size of the index when writing the index header, leading me to find a rather improbable integer overflow in

Re: [PATCH] pack-objects: Use reachability bitmap index when generating non-stdout pack too

Peff first of all thanks for feedback, On Thu, Jul 07, 2016 at 04:52:23PM -0400, Jeff King wrote: > On Thu, Jul 07, 2016 at 10:09:17PM +0300, Kirill Smelkov wrote: > > > Starting from 6b8fda2d (pack-objects: use bitmaps when packing objects) > > if a repository has bitmap index, pack-objects can

Re: [PATCH 2/2] avoid using sha1_to_hex output as printf format

On Fri, Jul 08, 2016 at 05:25:26AM -0400, Jeff King wrote: > diff --git a/commit.c b/commit.c > index 3f4f371..9603379 100644 > --- a/commit.c > +++ b/commit.c > @@ -1623,7 +1623,7 @@ void print_commit_list(struct commit_list *list, > { > for ( ; list; list = list->next) { >

Re: [PATCH] notes-merge: use O_EXCL to avoid overwriting existing files

On Thu, Jul 07, 2016 at 10:08:30PM +0200, René Scharfe wrote: > diff --git a/notes-merge.c b/notes-merge.c > index b7814c9..2b29fc4 100644 > --- a/notes-merge.c > +++ b/notes-merge.c > @@ -298,12 +298,8 @@ static void write_buf_to_worktree(const unsigned char > *obj, > char *path =

[PATCH 2/2] avoid using sha1_to_hex output as printf format

We know that it should not contain any percent-signs, but it's a good habit not to feed non-literals to printf formatters. Signed-off-by: Jeff King --- builtin/worktree.c | 2 +- commit.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git

[PATCH 1/2] walker: let walker_say take arbitrary formats

We take a printf-style format and a single "char *" parameter, and the format must therefore have at most one "%s" in it. Besides being error-prone (and tickling -Wformat-nonliteral), this is unnecessarily restrictive. We can just provide the usual varargs interface. Signed-off-by: Jeff King

Re: [PATCH 7/8] write_file: add format attribute

On Fri, Jul 08, 2016 at 05:12:41AM -0400, Jeff King wrote: > I had also hoped it would help with confusing write_file() > and write_file_buf(), since the former's "..." can make it > match the signature of the latter. But given that the buffer > for write_file_buf() is generally not a string

[PATCH 9/8] branch: use write_file_buf instead of write_file

If we already have a strbuf, then using write_file_buf is a little nicer to read (no wondering whether "%s" will eat your NULs), and it's more efficient (no extra formatting step). We don't care about the newline magic of write_file(), as we have our own multi-line content. Signed-off-by: Jeff

[PATCH 8/8] use write_file_buf where applicable

There are several places where we open a file, write some content from a strbuf, and close it. These can be simplified with write_file_buf(). As a bonus, many of these did not catch write problems at close() time. Signed-off-by: Jeff King --- builtin/am.c| 7 +--

[PATCH 7/8] write_file: add format attribute

This gives us compile-time checking of our format strings, which is a good thing. I had also hoped it would help with confusing write_file() and write_file_buf(), since the former's "..." can make it match the signature of the latter. But given that the buffer for write_file_buf() is generally

[PATCH 6/8] write_file: add pointer+len variant

There are many callsites which could use write_file, but for which it is a little awkward because they have a strbuf or other pointer/len combo. Specifically: 1. write_file() takes a format string, so we have to use "%s" or "%.*s", which are ugly. 2. Using any form of "%s" does not handle

[PATCH 5/8] write_file: use xopen

This simplifies the code a tiny bit, and provides consistent error messages with other users of xopen(). While we're here, let's also switch to using O_WRONLY. We know we're only going to open/write/close the file, so there's no point in asking for O_RDWR. Signed-off-by: Jeff King

[PATCH 3/8] branch: use non-gentle write_file for branch description

We use write_file_gently() to do this job currently. However, if we see an error, we simply complain via error_errno() and then end up exiting with an error code. By switching to the non-gentle form, the function will die for us, with a better error. It is more specific about which syscall caused

[PATCH 4/8] write_file: drop "gently" form

There are no callers left of write_file_gently(). Let's drop it, as it doesn't seem likely for new callers to be added (since its inception, the only callers who wanted the gentle form generally just died immediately themselves, and have since been converted). While we're there, let's also drop

[PATCH 2/8] am: ignore return value of write_file()

From: René Scharfe write_file() either returns 0 or dies, so there is no point in checking its return value. The callers of the wrappers write_state_text(), write_state_count() and write_state_bool() consequently already ignore their return values. Stop pretending we care and

[PATCH 1/8] config: fix bogus fd check when setting up default config

Since 9830534 (config --global --edit: create a template file if needed, 2014-07-25), an edit of the global config file will try to open() it with O_EXCL, and wants to handle three cases: 1. We succeeded; the user has no config file, and we should fill in the default template. 2. We got

[PATCH 0/8] write_file cleanups

On Fri, Jul 08, 2016 at 02:56:50AM -0400, Jeff King wrote: > > This is more an illustration of unnecessarily duplicated code, isn't it? > > There are *tons* of instances in Git's code where writing to a file is > > implemented separately (and differently). > > > > It would make tons of sense to

Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

On 07/07/16 20:43, Junio C Hamano wrote: Torsten Bögershausen writes: This is the callstack: merge-recursive.c/add_cacheinfo(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh, int options) { struct cache_entry *ce;

Re: [PATCH v2] am: counteract gender bias

Hello Johannes, On 8 July 2016 at 09:17, Johannes Schindelin wrote: > Since d1c5f2a (Add git-am, applymbox replacement., 2005-10-07), i.e. for > almost 11 years already, we used a male form to describe "the other > tree". > > While most likely unintended, this gave

Re: [PATCH] am: counteract gender bias

Hi Junio, On Thu, 7 Jul 2016, Junio C Hamano wrote: > We should just use "theirs" to be consistent from the beginning, as > you suggested. There is no need to churn the codebase for political > correctness to first use "hers" that everybody knows will *not* be > the final form. I will change

Re: [PATCH] am: ignore return value of write_file()

On Fri, Jul 08, 2016 at 08:37:35AM +0200, Johannes Schindelin wrote: > > We have two forms of write_file(): one that dies, and one > > that returns an error. However, the latter has only a single > > caller, which immediately dies anyway (after producing a > > message that is not really any more

Re: gc and repack ignore .git/*HEAD when checking reachability

On Thu, Jul 07, 2016 at 09:34:02PM -0700, Junio C Hamano wrote: > Josh Triplett writes: > > This could result in data loss, if a user expected that having an object > > referenced from those places would protect it from pruning. > > Yeah, luckily, nobody expects such. I

Re: [PATCH] am: ignore return value of write_file()

Hi Peff, On Thu, 7 Jul 2016, Jeff King wrote: > We have two forms of write_file(): one that dies, and one > that returns an error. However, the latter has only a single > caller, which immediately dies anyway (after producing a > message that is not really any more informative than >

Re: [PATCH] am: ignore return value of write_file()

Hi René, On Thu, 7 Jul 2016, René Scharfe wrote: > write_file() either returns 0 or dies, so there is no point in checking > its return value. The question is whether it makes sense for write_file() to die(). It is a library function and not every caller can be happy with that function to exit

Re: [PATCH] Add very verbose porcelain output to status

Hi, On Thu, 7 Jul 2016, Jeff Hostetler wrote: > Tools interacting with Git repositories may need to know the complete > state of the working directory. For efficiency, it would be good to have > a single command to obtain this information. > > We already have a `--porcelain` mode intended for