Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 8/8] checkout: introduce checkout.overlayMode config
From: Junio C Hamano @ 2019-01-07 17:00 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: git, Nguyễn Thái Ngọc Duy, Elijah Newren
In-Reply-To: <20190106183225.GH25639@hank.intra.tgummerer.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

> Maybe it would be enough to say "... `git checkout` never removes
> files, that are not in the tree being checked out, from the index or
> the working tree"?  It is more technically correct, but dunno making
> the sentence harder to read is worth it.

Yeah, I share the same feeling.  Let's say the text in the posted
patch is good enough and move on.

Thanks.

^ permalink raw reply

* [PATCH] sha1-file: close fd of empty file in map_sha1_file_1()
From: René Scharfe @ 2019-01-07 16:48 UTC (permalink / raw)
  To: Git List; +Cc: Matthieu Moy, Junio C Hamano

map_sha1_file_1() checks if the file it is about to mmap() is empty and
errors out in that case and explains the situation in an error message.
It leaks the private handle to that empty file, though.

Have the function clean up after itself and close the file descriptor
before exiting early.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
Patch generated with --function-context for easier review.

 sha1-file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sha1-file.c b/sha1-file.c
index 5a272f70de..bfa7a2e121 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -942,31 +942,32 @@ static int quick_has_loose(struct repository *r,
 /*
  * Map the loose object at "path" if it is not NULL, or the path found by
  * searching for a loose object named "sha1".
  */
 static void *map_sha1_file_1(struct repository *r, const char *path,
 			     const unsigned char *sha1, unsigned long *size)
 {
 	void *map;
 	int fd;
 
 	if (path)
 		fd = git_open(path);
 	else
 		fd = open_sha1_file(r, sha1, &path);
 	map = NULL;
 	if (fd >= 0) {
 		struct stat st;
 
 		if (!fstat(fd, &st)) {
 			*size = xsize_t(st.st_size);
 			if (!*size) {
 				/* mmap() is forbidden on empty files */
 				error(_("object file %s is empty"), path);
+				close(fd);
 				return NULL;
 			}
 			map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
 		}
 		close(fd);
 	}
 	return map;
 }
-- 
2.20.1

^ permalink raw reply related

* Re: Feature request: --preserve-merges to add "exec git merge ..." instead of "pick ..."
From: Junio C Hamano @ 2019-01-07 16:42 UTC (permalink / raw)
  To: Andreas Hennings; +Cc: git
In-Reply-To: <CAH0Uv3Eu7bGSVaJKu6kDjp0BPRe0yROucDfJ8QHV3X_ZjNJT7g@mail.gmail.com>

Andreas Hennings <andreas@dqxtech.net> writes:

> This means we need a rebase operation where:
> - The commits of the acceptance branch itself are being replaced.
> - The commits of the feature branches remain as they are.
>
> A "git rebase --preserve-merges" almost does this, but not really.

This wished-for feature sounds to me more like the "first-parent"
mode that has been discussed a few times in the past but never
materialized.

The preserve-merges mode is getting abandoned by the original author
as unsalvageable.  Have you tried the rebase-merges mode?  That may
let you choose replaying only the merge commits on the acceptance
branch without touching the tips of the feature branches getting
merged.

^ permalink raw reply

* Recovering from a "detached from" HEAD
From: Alyssa Ross @ 2019-01-07 16:17 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

If I detach my head, then use `git subtree add` to generate a commit,
the output from `git status` changes from "detached at SHA" to
"detached from SHA". The sha doesn't change, but HEAD has updated.

I don't understand why `git status` doesn't tell me that I'm
"detached at" the sha of the new commit, but not only that, it seems to
be extremely difficult to get `git status` to reflect my actual HEAD.
`git checkout HEAD` doesn't change it, `git checkout NEWSHA` doesn't
work.

So my question is, what's going on here? Is this intentional behaviour,
or a bug? How should I get my working tree back to a normal state?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: Does "git push" open a pack for read before closing it?
From: git-mailinglist @ 2019-01-07 15:56 UTC (permalink / raw)
  To: git; +Cc: brian m. carlson
In-Reply-To: <20181222231215.GC26554@genre.crustytoothpaste.net>

On 22/12/2018 23:12, brian m. carlson wrote:
Thanks Brian, you helped me make some progress. I'm stuck again trying
to understand git behaviour though and wondering if there are better
ways of me seeing into git (source, debug o/p etc) than posting here.

As a reminder, I'm doing the following to create a bare repository on my
FUSE mounted decentralised storage:

  cd ~/SAFE/_public/tests/data1
  git init --bare blah
  cd ~/src/safe/sjs.git
  git remote remove origin
  git remote add origin ~/SAFE/_public/tests/data1/blah
  git push origin master

The bugs are in my implementation of FUSE on the SAFE storage.

I get additional output from git using the following (but it doesn't
help me):
 set -x; GIT_TRACE=2 GIT_CURL_VERBOSE=2 GIT_TRACE_PERFORMANCE=2 \
 GIT_TRACE_PACK_ACCESS=2 GIT_TRACE_PACKET=2 GIT_TRACE_PACKFILE=2 \
 GIT_TRACE_SETUP=2 GIT_TRACE_SHALLOW=2 git push origin master -v -v \
 2>&1 |tee ~/git-trace.log; set +x

Anyway, to add a little to your observations...

> What I expect is happening is that Git receives the objects and writes
> them to a temporary file (which you see in "objects/incoming") and then
> they're passed to either git unpack-objects or git index-pack, which
> then attempts to read it.
The git console output seems to confirm it is 'git index-pack' that
encounters the error, which is currently:

  Enumerating objects: 373, done.
  Counting objects: 100% (373/373), done.
  Delta compression using up to 8 threads
  Compressing objects: 100% (371/371), done.
  Writing objects: 100% (373/373), 192.43 KiB | 54.00 KiB/s, done.
  Total 373 (delta 255), reused 0 (delta 0)
  remote: fatal: premature end of pack file, 36 bytes missing
  remote: fatal: premature end of pack file, 65 bytes missing
  error: remote unpack failed: index-pack abnormal exit
  To /home/mrh/SAFE/_public/tests/data1/blah
   ! [remote rejected] master -> master (unpacker error)
  error: failed to push some refs to
'/home/mrh/SAFE/_public/tests/data/blah'

So I conclude I'm either not writing the file properly, or not reading
it back properly. I can continue looking into that of course, but
looking at the file requests I'm curious about what git is doing and how
to learn more about it as it looks odd.

I have quite a few questions, but will focus on just the point at which
it bails out. In summary, what I see is:

- The pack file is created and written with multiple calls, ending up
about 200k long.

- While still open for write, it is opened *four* times, so git has five
handles active on it. One write and four read.

- At this point I see the following FUSE read operation:

  read('/_public/tests/data1/blah/objects/incoming-quFPHB
        /pack/tmp_pack_E4ea92', 58, buf, 4096, 16384)

  58 is the file handle, 4096 the length of buf, and 16384 the position

- Presumably this is where git encounters a problem because it then
closes everything and cleans up the incoming directory.

It seems odd to me that it is starting to read the pack file at position
16384 rather than at 0 (or at 12 after the header). I can surmise it
might open it four times to speed access, but would expect to see it
read the beginning of the file (or at position 12) before trying to
interpret the content and bailing out.

So I'm wondering what git is doing there. Any comments on this, or a
pointer to the relevant git code so I can look myself would be great.

Thanks,

Mak

^ permalink raw reply

* Re: [PATCH 2/1] Revert "t/lib-git-daemon: record daemon log"
From: Junio C Hamano @ 2019-01-07 15:45 UTC (permalink / raw)
  To: Jeff King
  Cc: Thomas Gummerer, Torsten Bögershausen, Git Mailing List,
	szeder.dev, Jan Palus, Johannes Schindelin
In-Reply-To: <20190107082041.GA21362@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> Yep, this looks good to me. Thanks for being extra careful with the
> read/printf bits!
>
> Looks like Junio already queued a99653a9b6 (Revert "t/lib-git-daemon:
> record daemon log", 2018-12-28) on the tip of tg/t5570-drop-racy-test,
> but that's a pure revert. I think we can replace it with this.

Thanks, both.  Will do.

^ permalink raw reply

* Re: cvs2git failed in pass2 ...
From: paduarte @ 2019-01-07 15:06 UTC (permalink / raw)
  To: git
In-Reply-To: <1329837442995-7305031.post@n2.nabble.com>

Hi supadhyay

How did the complete command work?

cvs2git  --encoding=ascii --encoding=utf8 --encoding=utf16 --encoding=latin

or 

cvs2git --encoding=ascii --encoding=utf8 --encoding=utf16 --encoding=latin
*+ parameters of git*

?

Tks
:)



--
Sent from: http://git.661346.n2.nabble.com/

^ permalink raw reply

* Feature request: --preserve-merges to add "exec git merge ..." instead of "pick ..."
From: Andreas Hennings @ 2019-01-07 14:02 UTC (permalink / raw)
  To: git

Hello,
(this is my first message on this list, so hello everybody. I hope I
found the correct channel.)

I recently discovered the --preserve-merges option in git rebase,
I think it is nice, but it does not always do what I intend.

I am proposing a variation of this option which would behave differently.

## Use case

We have an acceptance branch that is frequently rebased on master.
At any given time, the acceptance branch should start with master, and
then merge in those feature branches that were selected for acceptance
testing.
Nobody is supposed to work on acceptance, it is only meant for
deployment on the acceptance server.

This means we need a rebase operation where:
- The commits of the acceptance branch itself are being replaced.
- The commits of the feature branches remain as they are.

A "git rebase --preserve-merges" almost does this, but not really.

## Proposal

Add a new option for rebase, similar to --preserve-merges, where:
Instead of "git pick MERGE_COMMIT", we get this line in the rebase
editor (if using the -i option):
exec git merge MERGE_COMMIT^2 -m"Merge FEATURE_BRANCH into acceptance."

In --interactive / -i mode, the developer can leave these lines as
they are, or replace them manually to merge a more recent version of
the feature branch.
E.g. replace this
exec git merge MERGE_COMMIT^2 -m"Merge FEATURE_BRANCH into acceptance."
with this
exec git merge FEATURE_BRANCH_RECENT

This would achieve the behavior as described in "Use case".
I have not thought about a name for this option yet, I first want to
see some general feedback.

Possibly related:
https://public-inbox.org/git/87y3jtqdyg.fsf@javad.com/
I think the author of this RFC has different expectations what a
rebase should do, so it is not really the same.


------------

I hope this was the correct way to propose this kind of feature. If
not, let me know where I can find more information.

Greetings,
Andreas Hennings
https://github.com/donquixote

^ permalink raw reply

* Re: [PATCH 1/3] object-store: factor out odb_loose_cache()
From: René Scharfe @ 2019-01-07 13:26 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107082702.GB21362@sigill.intra.peff.net>

Am 07.01.2019 um 09:27 schrieb Jeff King:
> On Sun, Jan 06, 2019 at 05:45:30PM +0100, René Scharfe wrote:
>> diff --git a/object-store.h b/object-store.h
>> index 60758efad8..7236c571c0 100644
>> --- a/object-store.h
>> +++ b/object-store.h
>> @@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir);
>>   */
>>  void odb_load_loose_cache(struct object_directory *odb, int subdir_nr);
>>  
>> +/*
>> + * Populate and return the loose object cache array corresponding to the
>> + * given object ID.
>> + */
>> +struct oid_array *odb_loose_cache(struct object_directory *odb,
>> +				  const struct object_id *oid);
>> +
> 
> I think the ugly-interfaced odb_load_loose_cache() can become "static"
> now, as the only outside caller (from sha1-name.c) has gone away.
> 
>> +struct oid_array *odb_loose_cache(struct object_directory *odb,
>> +				  const struct object_id *oid)
>> +{
>> +	int subdir_nr = oid->hash[0];
>> +	odb_load_loose_cache(odb, subdir_nr);
>> +	return &odb->loose_objects_cache;
>> +}
>> +
>>  void odb_load_loose_cache(struct object_directory *odb, int subdir_nr)
> 
> You'd need to re-order these definitions, of course (or alternatively,
> just fold the load function inline into odb_loose_cache()).

Yes, the functions are arranged so that odb_load_loose_cache() can be
inlined easily.  I meant to include a patch for that but then quibbled
about keeping the BUG check (which is probably optimized out) or not,
and dropped it for now to get the performance fix in more quickly..

René

^ permalink raw reply

* Re: [PATCH 02/10] repository.c: replace hold_locked_index() with repo_hold_locked_index()
From: Duy Nguyen @ 2019-01-07 12:55 UTC (permalink / raw)
  To: Martin Ågren; +Cc: Git Mailing List
In-Reply-To: <CAN0heSoRSgZVZ=NE6eof4dusEhfdz-eSiht06rGKb=GELvm86Q@mail.gmail.com>

On Sat, Jan 5, 2019 at 9:34 PM Martin Ågren <martin.agren@gmail.com> wrote:
>
> On Sat, 5 Jan 2019 at 07:07, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> >
> > 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_locked_index() to reduce changes in builtin/
>
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 7c7f98c72c..ddb3230d21 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -8,6 +8,7 @@
> >   * Clone a repository into a different directory that does not yet exist.
> >   */
> >
> > +#define USE_THE_INDEX_COMPATIBILITY_MACROS
>
> I think this should be in patch 10/10.

Eck. There has been a lot of fixups in this series. I guess I fixed up
the wrong commit.

> > -int hold_locked_index(struct lock_file *lk, int lock_flags)
> > -{
> > -       return hold_lock_file_for_update(lk, get_index_file(), lock_flags);
> > -}
>
> > +int repo_hold_locked_index(struct repository *repo,
> > +                          struct lock_file *lf,
> > +                          int flags)
> > +{
> > +       return hold_lock_file_for_update(lf, repo->index_file, flags);
> > +}
>
> `get_index_file()` BUGs if `the_repository->index_file` is NULL, but
> other than that, this looks like a faithful conversion. Do we want to
> keep that check here?

Better than segfault :)
-- 
Duy

^ permalink raw reply

* Re: [PATCH 1/3] object-store: factor out odb_loose_cache()
From: René Scharfe @ 2019-01-07 13:11 UTC (permalink / raw)
  To: Jeff King, Philip Oakley
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107123012.GA6032@sigill.intra.peff.net>

Am 07.01.2019 um 13:30 schrieb Jeff King:
> On Mon, Jan 07, 2019 at 11:27:06AM +0000, Philip Oakley wrote:
> 
>> On 06/01/2019 16:45, René Scharfe wrote:
>>> Add and use a function for loading the entries if a loose object
>>> subdirectory for a given object ID.
>>
>> The second part of the sentence 'a loose object subdirectory for a given
>> object ID' does not scan for me. Is there a word missing?
> 
> I think s/if/in/ ?

Good guess and close, but I think I meant s/if/of/.  Anyway, the idea is
that a caller provides an object ID and the function then reads the
corresponding object subdirectory.

René

^ permalink raw reply

* Re: [PATCH] helper/test-ref-store: fix "new-sha1" vs "old-sha1" typo
From: Duy Nguyen @ 2019-01-07 12:53 UTC (permalink / raw)
  To: Christian Couder
  Cc: Git Mailing List, Junio C Hamano,
	Ævar Arnfjörð Bjarmason, Christian Couder
In-Reply-To: <20190106154637.10815-1-chriscool@tuxfamily.org>

On Sun, Jan 6, 2019 at 10:46 PM Christian Couder
<christian.couder@gmail.com> wrote:
>
> It looks like it is a copy-paste error  made in 80f2a6097c
> (t/helper: add test-ref-store to test ref-store functions,
> 2017-03-26) to pass "old-sha1" instead of "new-sha1" to
> notnull() when we get the new sha1 argument from
> const char **argv.

Ack. Definitely copy-paste error.

>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  t/helper/test-ref-store.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> index e9e0541276..799fc00aa1 100644
> --- a/t/helper/test-ref-store.c
> +++ b/t/helper/test-ref-store.c
> @@ -233,7 +233,7 @@ static int cmd_update_ref(struct ref_store *refs, const char **argv)
>  {
>         const char *msg = notnull(*argv++, "msg");
>         const char *refname = notnull(*argv++, "refname");
> -       const char *new_sha1_buf = notnull(*argv++, "old-sha1");
> +       const char *new_sha1_buf = notnull(*argv++, "new-sha1");
>         const char *old_sha1_buf = notnull(*argv++, "old-sha1");
>         unsigned int flags = arg_flags(*argv++, "flags");
>         struct object_id old_oid;
> --
> 2.20.1.26.gc246996f60
>


-- 
Duy

^ permalink raw reply

* git-p4: default behavior for handling moves?
From: Luke Diamand @ 2019-01-07 12:50 UTC (permalink / raw)
  To: Git Users, Junio C Hamano; +Cc: Chen Bin, Merland Romain, Vitor Antunes

git-p4 can map a "git move" operation to a Perforce "move" operation.
But by default this is disabled. You then end up with a P4 commit
where the file is deleted, and a fresh file is created with the same
contents at the new location at revision #1.

Rename detection gets enabled either with the "-M" option, or with
some config variables, git-p4.detectCopies and git-p4.detectRenames.

I've been tripped up by this, and I actually know about it, and I know
other people have been as well.

Should we switch the default over so that it's enabled by default? I
can't think of any reason why you wouldn't want it enabled.

I think the rename code was first introduced around 2011 by Vitor.

Another option is to add a warning, but people just ignore warnings!

Thanks!
Luke

^ permalink raw reply

* Re: [PATCH 1/3] object-store: factor out odb_loose_cache()
From: Jeff King @ 2019-01-07 12:30 UTC (permalink / raw)
  To: Philip Oakley
  Cc: René Scharfe, git, Junio C Hamano,
	Ævar Arnfjörð Bjarmason
In-Reply-To: <0797a920-32f3-aaaf-9321-528f78d980ae@iee.org>

On Mon, Jan 07, 2019 at 11:27:06AM +0000, Philip Oakley wrote:

> On 06/01/2019 16:45, René Scharfe wrote:
> > Add and use a function for loading the entries if a loose object
> > subdirectory for a given object ID.
> 
> The second part of the sentence 'a loose object subdirectory for a given
> object ID' does not scan for me. Is there a word missing?

I think s/if/in/ ?

-Peff

^ permalink raw reply

* Introducing OneDev - an open source git server with interesting features
From: Robin Shen @ 2019-01-07 11:42 UTC (permalink / raw)
  To: git@vger.kernel.org

Dear git users, 

OneDev is an open source git server with interesting features such as language aware code search/navigation, issue workflow customization, free source/diff comment and discussion, etc.  

It is using MIT license and hope it can be useful to someone.  Learn more at https://onedev.io

Robin

^ permalink raw reply

* Re: [PATCH 1/3] object-store: factor out odb_loose_cache()
From: Philip Oakley @ 2019-01-07 11:27 UTC (permalink / raw)
  To: René Scharfe, git
  Cc: Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason
In-Reply-To: <b87e7e01-baa6-6fb2-7081-0042ecd3b6b7@web.de>

On 06/01/2019 16:45, René Scharfe wrote:
> Add and use a function for loading the entries if a loose object
> subdirectory for a given object ID.

The second part of the sentence 'a loose object subdirectory for a given 
object ID' does not scan for me. Is there a word missing?

>    It frees callers from deriving the
> fanout key; they can use the returned oid_array reference for lookups or
> forward range scans.
>
> Suggested-by: Jeff King<peff@peff.net>
> Signed-off-by: Rene Scharfe<l.s.r@web.de>

-- 

Philip


^ permalink raw reply

* Re: "git p4" fails when perforce login not needed
From: Luke Diamand @ 2019-01-07 10:10 UTC (permalink / raw)
  To: Peter Osterlund; +Cc: Git Users
In-Reply-To: <alpine.LFD.2.21.1901062337420.5908@fractal.localdomain>

On Sun, 6 Jan 2019 at 22:48, Peter Osterlund <peterosterlund2@gmail.com> wrote:
>
> Hi,
>
> When I use "git p4 sync" to update a git repository from a perforce depot,
> the operation fails with error message:
>
>      failure accessing depot: unknown error code info
>
> When I run "p4 login -s" from a shell it reports:
>
>      'login' not necessary, no password set for this user.
>
> The following patch fixes the problem for me:

That's my fault!

Your fix looks good, thanks!

Luke


>
> --- /usr/libexec/git-core/git-p4~        2018-12-15 14:51:07.000000000 +0100
> +++ /usr/libexec/git-core/git-p4      2019-01-06 23:23:06.934176387 +0100
> @@ -332,6 +332,8 @@
>               die_bad_access("p4 error: {0}".format(data))
>           else:
>               die_bad_access("unknown error")
> +    elif code == "info":
> +        return
>       else:
>           die_bad_access("unknown error code {0}".format(code))
>
>
> Not sure if this helps, but running "p4 -G login -s | hexdump" gives:
>
> 00000000  7b 73 04 00 00 00 63 6f  64 65 73 04 00 00 00 69  |{s....codes....i|
> 00000010  6e 66 6f 73 05 00 00 00  6c 65 76 65 6c 69 00 00  |nfos....leveli..|
> 00000020  00 00 73 04 00 00 00 64  61 74 61 73 35 00 00 00  |..s....datas5...|
> 00000030  27 6c 6f 67 69 6e 27 20  6e 6f 74 20 6e 65 63 65  |'login' not nece|
> 00000040  73 73 61 72 79 2c 20 6e  6f 20 70 61 73 73 77 6f  |ssary, no passwo|
> 00000050  72 64 20 73 65 74 20 66  6f 72 20 74 68 69 73 20  |rd set for this |
> 00000060  75 73 65 72 2e 30                                 |user.0|
> 00000066
>
> Best regards,
>
> --
> Peter Osterlund - peterosterlund2@gmail.com
> http://hem.bredband.net/petero2b

^ permalink raw reply

* Re: [PATCH v17 0/7] git bisect: convert from shell to C
From: TANUSHREE TUMANE @ 2019-01-07  9:15 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Tanushree Tumane via GitGitGadget, git, Junio C Hamano
In-Reply-To: <d64492dc-4643-823b-c804-3c152c4d9090@ramsayjones.plus.com>

Yes, you are right. I will revert this back.

On Fri, Jan 4, 2019 at 8:08 PM Ramsay Jones <ramsay@ramsayjones.plus.com> wrote:
>
>
>
> On 02/01/2019 15:38, Tanushree Tumane via GitGitGadget wrote:
> [snip]
> > base-commit: 7f4e64169352e03476b0ea64e7e2973669e491a2
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-101%2Ftanushree27%2Fgit-bisect_part2_fixup-v17
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-101/tanushree27/git-bisect_part2_fixup-v17
> > Pull-Request: https://github.com/gitgitgadget/git/pull/101
>
> I didn't look at the patches, only the range-diff below, and the
> only thing I noticed was ...
>
> >
> > Range-diff vs v16:
> >
> >  1:  f1e89ba517 ! 1:  338ebdc97a bisect--helper: `bisect_reset` shell function in C
> >      @@ -16,8 +16,9 @@
> >
> >           Mentored-by: Lars Schneider <larsxschneider@gmail.com>
> >           Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> >      +    Mentored by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> >           Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> >      -    Signed-off-by: Junio C Hamano <gitster@pobox.com>
> >      +    Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com>
> >
> >        diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> >        --- a/builtin/bisect--helper.c
> >      @@ -53,8 +54,10 @@
> >       +       struct strbuf branch = STRBUF_INIT;
> >       +
> >       +       if (!commit) {
> >      -+               if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
> >      -+                       return !printf(_("We are not bisecting.\n"));
> >      ++               if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {
> >      ++                       printf(_("We are not bisecting.\n"));
> >      ++                       return 0;
> >      ++               }
> >       +               strbuf_rtrim(&branch);
> >       +       } else {
> >       +               struct object_id oid;
> >      @@ -69,11 +72,11 @@
> >       +
> >       +               argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL);
> >       +               if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
> >      -+                       error(_("Could not check out original HEAD '%s'. Try "
> >      -+                               "'git bisect reset <commit>'."), branch.buf);
> >       +                       strbuf_release(&branch);
> >       +                       argv_array_clear(&argv);
> >      -+                       return -1;
> >      ++                       return error(_("could not check out original"
> >      ++                                      " HEAD '%s'. Try 'git bisect"
> >      ++                                      "reset <commit>'."), branch.buf);
>
> ... this 'branch.buf' will refer to the empty 'slopbuf', since
> the call to 'strbuf_release(&branch)' now precedes this call
> to error().
>
> ATB,
> Ramsay Jones
>

^ permalink raw reply

* Re: [PATCH v4 0/8] test-lib: add the '--stress' option to help reproduce occasional failures in flaky tests
From: Jeff King @ 2019-01-07  8:49 UTC (permalink / raw)
  To: SZEDER Gábor; +Cc: Junio C Hamano, git
In-Reply-To: <20190105010859.11031-1-szeder.dev@gmail.com>

On Sat, Jan 05, 2019 at 02:08:51AM +0100, SZEDER Gábor wrote:

> To recap: this patch series tries to make reproducing rare failures in
> flaky tests easier: it adds the '--stress' option to our test library
> to run the test script repeatedly in multiple parallel jobs, in the
> hope that the increased load creates enough variance in the timing of
> the test's commands that such a failure is eventually triggered.

Thanks, I looked over this version (glossing over the bits that hadn't
really changed since v2) and it all looks good to me.

-Peff

^ permalink raw reply

* [PATCH 11/11] prefer "hash mismatch" to "sha1 mismatch"
From: Jeff King @ 2019-01-07  8:40 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107083150.GC21362@sigill.intra.peff.net>

To future-proof ourselves against a change in the hash, let's use the
more generic "hash mismatch" to refer to integrity problems. Note that
we do advertise this exact string in git-fsck(1). However, the message
itself is marked for translation, meaning we do not expect it to be
machine-readable.

While we're touching that documentation, let's also update it for
grammar and clarity.

Signed-off-by: Jeff King <peff@peff.net>
---
I'm actually a little nervous that we _shouldn't_ have marked the
messages that fsck produces for translation (and nor should we change
them here, but then we're stuck with the word "sha1" forever).

I actually think fsck ought to have a machine-readable output format,
but of course that does not help any existing scripts.

 Documentation/git-fsck.txt | 6 +++---
 object.c                   | 4 ++--
 sha1-file.c                | 4 ++--
 t/t1450-fsck.sh            | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt
index ab9a93fb9b..55950d9eea 100644
--- a/Documentation/git-fsck.txt
+++ b/Documentation/git-fsck.txt
@@ -140,9 +140,9 @@ dangling <type> <object>::
 	The <type> object <object>, is present in the database but never
 	'directly' used. A dangling commit could be a root node.
 
-sha1 mismatch <object>::
-	The database has an object who's sha1 doesn't match the
-	database value.
+hash mismatch <object>::
+	The database has an object whose hash doesn't match the
+	object database value.
 	This indicates a serious data integrity problem.
 
 Environment Variables
diff --git a/object.c b/object.c
index a5c5cf830f..df72914bdc 100644
--- a/object.c
+++ b/object.c
@@ -263,7 +263,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
 	    (!obj && has_object_file(oid) &&
 	     oid_object_info(r, oid, NULL) == OBJ_BLOB)) {
 		if (check_object_signature(repl, NULL, 0, NULL) < 0) {
-			error(_("sha1 mismatch %s"), oid_to_hex(oid));
+			error(_("hash mismatch %s"), oid_to_hex(oid));
 			return NULL;
 		}
 		parse_blob_buffer(lookup_blob(r, oid), NULL, 0);
@@ -274,7 +274,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
 	if (buffer) {
 		if (check_object_signature(repl, buffer, size, type_name(type)) < 0) {
 			free(buffer);
-			error(_("sha1 mismatch %s"), oid_to_hex(repl));
+			error(_("hash mismatch %s"), oid_to_hex(repl));
 			return NULL;
 		}
 
diff --git a/sha1-file.c b/sha1-file.c
index 55a4782844..a5726c3e73 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -2223,7 +2223,7 @@ static int check_stream_oid(git_zstream *stream,
 
 	the_hash_algo->final_fn(real_oid.hash, &c);
 	if (!oideq(expected_oid, &real_oid)) {
-		error(_("sha1 mismatch for %s (expected %s)"), path,
+		error(_("hash mismatch for %s (expected %s)"), path,
 		      oid_to_hex(expected_oid));
 		return -1;
 	}
@@ -2275,7 +2275,7 @@ int read_loose_object(const char *path,
 		}
 		if (check_object_signature(expected_oid, *contents,
 					 *size, type_name(*type))) {
-			error(_("sha1 mismatch for %s (expected %s)"), path,
+			error(_("hash mismatch for %s (expected %s)"), path,
 			      oid_to_hex(expected_oid));
 			free(*contents);
 			goto out;
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 2e5e979336..c61f972141 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -406,7 +406,7 @@ test_expect_success 'rev-list --verify-objects with bad sha1' '
 
 	test_might_fail git rev-list --verify-objects refs/heads/bogus >/dev/null 2>out &&
 	cat out &&
-	test_i18ngrep -q "error: sha1 mismatch 63ffffffffffffffffffffffffffffffffffffff" out
+	test_i18ngrep -q "error: hash mismatch 63ffffffffffffffffffffffffffffffffffffff" out
 '
 
 test_expect_success 'force fsck to ignore double author' '
-- 
2.20.1.470.g640a3e2614

^ permalink raw reply related

* [PATCH 10/11] sha1-file: avoid "sha1 file" for generic use in messages
From: Jeff King @ 2019-01-07  8:39 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107083150.GC21362@sigill.intra.peff.net>

These error messages say "sha1 file", which is vague and not common in
user-facing documentation. Unlike the conversions from the previous
commit, these do not always refer to loose objects.

In finalize_object_file() we could be dealing with a packfile. Let's
just say "unable to write file" instead; since we include the filename,
the nature of the file is clear from the rest of the message.

In force_object_loose(), we're calling into read_object(), which could
actually be _any_ type of object. Just say "object".

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1-file.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sha1-file.c b/sha1-file.c
index 07cc9b548b..55a4782844 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1521,7 +1521,7 @@ int finalize_object_file(const char *tmpfile, const char *filename)
 	unlink_or_warn(tmpfile);
 	if (ret) {
 		if (ret != EEXIST) {
-			return error_errno(_("unable to write sha1 filename %s"), filename);
+			return error_errno(_("unable to write file %s"), filename);
 		}
 		/* FIXME!!! Collision check here ? */
 	}
@@ -1744,7 +1744,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
 		return 0;
 	buf = read_object(oid, &type, &len);
 	if (!buf)
-		return error(_("cannot read sha1_file for %s"), oid_to_hex(oid));
+		return error(_("cannot read object for %s"), oid_to_hex(oid));
 	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %"PRIuMAX , type_name(type), (uintmax_t)len) + 1;
 	ret = write_loose_object(oid, hdr, hdrlen, buf, len, mtime);
 	free(buf);
-- 
2.20.1.470.g640a3e2614


^ permalink raw reply related

* [PATCH 09/11] sha1-file: prefer "loose object file" to "sha1 file" in messages
From: Jeff King @ 2019-01-07  8:39 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107083150.GC21362@sigill.intra.peff.net>

When we're reporting an error for a loose object, let's use that term.
It's more consistent with other parts of Git, and it is future-proof
against changes to the hash function.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1-file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sha1-file.c b/sha1-file.c
index da6d78976f..07cc9b548b 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1552,9 +1552,9 @@ int hash_object_file(const void *buf, unsigned long len, const char *type,
 static void close_loose_object(int fd)
 {
 	if (fsync_object_files)
-		fsync_or_die(fd, "sha1 file");
+		fsync_or_die(fd, "loose object file");
 	if (close(fd) != 0)
-		die_errno(_("error when closing sha1 file"));
+		die_errno(_("error when closing loose object file"));
 }
 
 /* Size of directory component, including the ending '/' */
@@ -1645,7 +1645,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
 		ret = git_deflate(&stream, Z_FINISH);
 		the_hash_algo->update_fn(&c, in0, stream.next_in - in0);
 		if (write_buffer(fd, compressed, stream.next_out - compressed) < 0)
-			die(_("unable to write sha1 file"));
+			die(_("unable to write loose object file"));
 		stream.next_out = compressed;
 		stream.avail_out = sizeof(compressed);
 	} while (ret == Z_OK);
-- 
2.20.1.470.g640a3e2614


^ permalink raw reply related

* [PATCH 08/11] sha1-file: drop has_sha1_file()
From: Jeff King @ 2019-01-07  8:39 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107083150.GC21362@sigill.intra.peff.net>

There are no callers left of has_sha1_file() or its with_flags()
variant. Let's drop them, and convert has_object_file() from a wrapper
into the "real" function. Ironically, the sha1 variant was just copying
into an object_id internally, so the resulting code is actually shorter!

We can also drop the coccinelle rules for catching has_sha1_file()
callers. Since the function no longer exists, the compiler will do that
for us.

Signed-off-by: Jeff King <peff@peff.net>
---
 contrib/coccinelle/object_id.cocci | 32 ------------------------------
 object-store.h                     | 12 ++++-------
 sha1-file.c                        | 16 ++-------------
 3 files changed, 6 insertions(+), 54 deletions(-)

diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
index 73886ae583..6a7cf3e02d 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -147,35 +147,3 @@ expression E1, E2;
 - hashcmp(E1, E2) != 0
 + !hasheq(E1, E2)
   ...>}
-
-@@
-struct object_id OID;
-@@
-- has_sha1_file(OID.hash)
-+ has_object_file(&OID)
-
-@@
-identifier f != has_object_file;
-struct object_id *OIDPTR;
-@@
-  f(...) {<...
-- has_sha1_file(OIDPTR->hash)
-+ has_object_file(OIDPTR)
-  ...>}
-
-@@
-struct object_id OID;
-expression E;
-@@
-- has_sha1_file_with_flags(OID.hash, E)
-+ has_object_file_with_flags(&OID, E)
-
-@@
-identifier f != has_object_file_with_flags;
-struct object_id *OIDPTR;
-expression E;
-@@
-  f(...) {<...
-- has_sha1_file_with_flags(OIDPTR->hash, E)
-+ has_object_file_with_flags(OIDPTR, E)
-  ...>}
diff --git a/object-store.h b/object-store.h
index 6b1c408753..9e8ac6a1d4 100644
--- a/object-store.h
+++ b/object-store.h
@@ -209,20 +209,16 @@ int read_loose_object(const char *path,
 		      void **contents);
 
 /*
- * Convenience for sha1_object_info_extended() with a NULL struct
+ * Convenience for oid_object_info_extended() with a NULL struct
  * object_info. OBJECT_INFO_SKIP_CACHED is automatically set; pass
  * nonzero flags to also set other flags.
  */
-extern int has_sha1_file_with_flags(const unsigned char *sha1, int flags);
-static inline int has_sha1_file(const unsigned char *sha1)
+int has_object_file_with_flags(const struct object_id *oid, int flags);
+static inline int has_object_file(const struct object_id *oid)
 {
-	return has_sha1_file_with_flags(sha1, 0);
+	return has_object_file_with_flags(oid, 0);
 }
 
-/* Same as the above, except for struct object_id. */
-extern int has_object_file(const struct object_id *oid);
-extern int has_object_file_with_flags(const struct object_id *oid, int flags);
-
 /*
  * Return true iff an alternate object database has a loose object
  * with the specified name.  This function does not respect replace
diff --git a/sha1-file.c b/sha1-file.c
index 449456f2ad..da6d78976f 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1752,26 +1752,14 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
 	return ret;
 }
 
-int has_sha1_file_with_flags(const unsigned char *sha1, int flags)
+int has_object_file_with_flags(const struct object_id *oid, int flags)
 {
-	struct object_id oid;
 	if (!startup_info->have_repository)
 		return 0;
-	hashcpy(oid.hash, sha1);
-	return oid_object_info_extended(the_repository, &oid, NULL,
+	return oid_object_info_extended(the_repository, oid, NULL,
 					flags | OBJECT_INFO_SKIP_CACHED) >= 0;
 }
 
-int has_object_file(const struct object_id *oid)
-{
-	return has_sha1_file(oid->hash);
-}
-
-int has_object_file_with_flags(const struct object_id *oid, int flags)
-{
-	return has_sha1_file_with_flags(oid->hash, flags);
-}
-
 static void check_tree(const void *buf, size_t size)
 {
 	struct tree_desc desc;
-- 
2.20.1.470.g640a3e2614


^ permalink raw reply related

* [PATCH 07/11] convert has_sha1_file() callers to has_object_file()
From: Jeff King @ 2019-01-07  8:37 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107083150.GC21362@sigill.intra.peff.net>

The only remaining callers of has_sha1_file() actually have an object_id
already. They can use the "object" variant, rather than dereferencing
the hash themselves.

The code changes here were completely generated by the included
coccinelle patch.

Signed-off-by: Jeff King <peff@peff.net>
---
 apply.c                            |  2 +-
 builtin/fetch.c                    |  7 +++----
 builtin/index-pack.c               |  2 +-
 builtin/reflog.c                   |  2 +-
 builtin/show-ref.c                 |  2 +-
 bulk-checkin.c                     |  2 +-
 cache-tree.c                       |  4 ++--
 contrib/coccinelle/object_id.cocci | 32 ++++++++++++++++++++++++++++++
 http-walker.c                      |  4 ++--
 refs.c                             |  2 +-
 send-pack.c                        |  2 +-
 sha1-file.c                        |  2 +-
 12 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/apply.c b/apply.c
index 01793d6126..b8e257ead2 100644
--- a/apply.c
+++ b/apply.c
@@ -3183,7 +3183,7 @@ static int apply_binary(struct apply_state *state,
 		return 0; /* deletion patch */
 	}
 
-	if (has_sha1_file(oid.hash)) {
+	if (has_object_file(&oid)) {
 		/* We already have the postimage */
 		enum object_type type;
 		unsigned long size;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e0140327aa..57f35c6a0a 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -317,8 +317,7 @@ static void find_non_local_tags(const struct ref *refs,
 			    !has_object_file_with_flags(&ref->old_oid,
 							OBJECT_INFO_QUICK) &&
 			    !will_fetch(head, ref->old_oid.hash) &&
-			    !has_sha1_file_with_flags(item->oid.hash,
-						      OBJECT_INFO_QUICK) &&
+			    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 			    !will_fetch(head, item->oid.hash))
 				oidclr(&item->oid);
 			item = NULL;
@@ -332,7 +331,7 @@ static void find_non_local_tags(const struct ref *refs,
 		 * fetch.
 		 */
 		if (item &&
-		    !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
+		    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 		    !will_fetch(head, item->oid.hash))
 			oidclr(&item->oid);
 
@@ -353,7 +352,7 @@ static void find_non_local_tags(const struct ref *refs,
 	 * checked to see if it needs fetching.
 	 */
 	if (item &&
-	    !has_sha1_file_with_flags(item->oid.hash, OBJECT_INFO_QUICK) &&
+	    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 	    !will_fetch(head, item->oid.hash))
 		oidclr(&item->oid);
 
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ac1f4ea9a7..31046c7a0a 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -772,7 +772,7 @@ static void sha1_object(const void *data, struct object_entry *obj_entry,
 	if (startup_info->have_repository) {
 		read_lock();
 		collision_test_needed =
-			has_sha1_file_with_flags(oid->hash, OBJECT_INFO_QUICK);
+			has_object_file_with_flags(oid, OBJECT_INFO_QUICK);
 		read_unlock();
 	}
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 64a8df4f25..45e9e15006 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -94,7 +94,7 @@ static int tree_is_complete(const struct object_id *oid)
 	init_tree_desc(&desc, tree->buffer, tree->size);
 	complete = 1;
 	while (tree_entry(&desc, &entry)) {
-		if (!has_sha1_file(entry.oid->hash) ||
+		if (!has_object_file(entry.oid) ||
 		    (S_ISDIR(entry.mode) && !tree_is_complete(entry.oid))) {
 			tree->object.flags |= INCOMPLETE;
 			complete = 0;
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index ed888ffa48..6a706c02a6 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -23,7 +23,7 @@ static void show_one(const char *refname, const struct object_id *oid)
 	const char *hex;
 	struct object_id peeled;
 
-	if (!has_sha1_file(oid->hash))
+	if (!has_object_file(oid))
 		die("git show-ref: bad ref %s (%s)", refname,
 		    oid_to_hex(oid));
 
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 409ecb566b..39ee7d6107 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -67,7 +67,7 @@ static int already_written(struct bulk_checkin_state *state, struct object_id *o
 	int i;
 
 	/* The object may already exist in the repository */
-	if (has_sha1_file(oid->hash))
+	if (has_object_file(oid))
 		return 1;
 
 	/* Might want to keep the list sorted */
diff --git a/cache-tree.c b/cache-tree.c
index 190c6e5aa6..47f3464a1f 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -225,7 +225,7 @@ int cache_tree_fully_valid(struct cache_tree *it)
 	int i;
 	if (!it)
 		return 0;
-	if (it->entry_count < 0 || !has_sha1_file(it->oid.hash))
+	if (it->entry_count < 0 || !has_object_file(&it->oid))
 		return 0;
 	for (i = 0; i < it->subtree_nr; i++) {
 		if (!cache_tree_fully_valid(it->down[i]->cache_tree))
@@ -253,7 +253,7 @@ static int update_one(struct cache_tree *it,
 
 	*skip_count = 0;
 
-	if (0 <= it->entry_count && has_sha1_file(it->oid.hash))
+	if (0 <= it->entry_count && has_object_file(&it->oid))
 		return it->entry_count;
 
 	/*
diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
index 6a7cf3e02d..73886ae583 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -147,3 +147,35 @@ expression E1, E2;
 - hashcmp(E1, E2) != 0
 + !hasheq(E1, E2)
   ...>}
+
+@@
+struct object_id OID;
+@@
+- has_sha1_file(OID.hash)
++ has_object_file(&OID)
+
+@@
+identifier f != has_object_file;
+struct object_id *OIDPTR;
+@@
+  f(...) {<...
+- has_sha1_file(OIDPTR->hash)
++ has_object_file(OIDPTR)
+  ...>}
+
+@@
+struct object_id OID;
+expression E;
+@@
+- has_sha1_file_with_flags(OID.hash, E)
++ has_object_file_with_flags(&OID, E)
+
+@@
+identifier f != has_object_file_with_flags;
+struct object_id *OIDPTR;
+expression E;
+@@
+  f(...) {<...
+- has_sha1_file_with_flags(OIDPTR->hash, E)
++ has_object_file_with_flags(OIDPTR, E)
+  ...>}
diff --git a/http-walker.c b/http-walker.c
index 29b59e2fe0..8ae5d76c6a 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -131,7 +131,7 @@ static int fill_active_slot(struct walker *walker)
 	list_for_each_safe(pos, tmp, head) {
 		obj_req = list_entry(pos, struct object_request, node);
 		if (obj_req->state == WAITING) {
-			if (has_sha1_file(obj_req->oid.hash))
+			if (has_object_file(&obj_req->oid))
 				obj_req->state = COMPLETE;
 			else {
 				start_object_request(walker, obj_req);
@@ -489,7 +489,7 @@ static int fetch_object(struct walker *walker, unsigned char *sha1)
 	if (obj_req == NULL)
 		return error("Couldn't find request for %s in the queue", hex);
 
-	if (has_sha1_file(obj_req->oid.hash)) {
+	if (has_object_file(&obj_req->oid)) {
 		if (obj_req->req != NULL)
 			abort_http_object_request(obj_req->req);
 		abort_object_request(obj_req);
diff --git a/refs.c b/refs.c
index f9936355cd..142888a40a 100644
--- a/refs.c
+++ b/refs.c
@@ -188,7 +188,7 @@ int ref_resolves_to_object(const char *refname,
 {
 	if (flags & REF_ISBROKEN)
 		return 0;
-	if (!has_sha1_file(oid->hash)) {
+	if (!has_object_file(oid)) {
 		error(_("%s does not point to a valid object!"), refname);
 		return 0;
 	}
diff --git a/send-pack.c b/send-pack.c
index f692686770..c673d3ed06 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -40,7 +40,7 @@ int option_parse_push_signed(const struct option *opt,
 
 static void feed_object(const struct object_id *oid, FILE *fh, int negative)
 {
-	if (negative && !has_sha1_file(oid->hash))
+	if (negative && !has_object_file(oid))
 		return;
 
 	if (negative)
diff --git a/sha1-file.c b/sha1-file.c
index 589a666686..449456f2ad 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1372,7 +1372,7 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type,
 	struct cached_object *co;
 
 	hash_object_file(buf, len, type_name(type), oid);
-	if (has_sha1_file(oid->hash) || find_cached_object(oid))
+	if (has_object_file(oid) || find_cached_object(oid))
 		return 0;
 	ALLOC_GROW(cached_objects, cached_object_nr + 1, cached_object_alloc);
 	co = &cached_objects[cached_object_nr++];
-- 
2.20.1.470.g640a3e2614


^ permalink raw reply related

* [PATCH 06/11] sha1-file: convert pass-through functions to object_id
From: Jeff King @ 2019-01-07  8:37 UTC (permalink / raw)
  To: René Scharfe
  Cc: git, Junio C Hamano, Ævar Arnfjörð Bjarmason
In-Reply-To: <20190107083150.GC21362@sigill.intra.peff.net>

These two static functions, read_object() and quick_has_loose(), both
have to hashcpy() their bare-sha1 arguments into object_id structs to
pass them along. Since all of their callers actually have object_id
structs in the first place, we can eliminate the copying by adjusting
their input parameters.

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1-file.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/sha1-file.c b/sha1-file.c
index 4938258ed1..589a666686 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -922,16 +922,13 @@ static int open_loose_object(struct repository *r,
 }
 
 static int quick_has_loose(struct repository *r,
-			   const unsigned char *sha1)
+			   const struct object_id *oid)
 {
-	struct object_id oid;
 	struct object_directory *odb;
 
-	hashcpy(oid.hash, sha1);
-
 	prepare_alt_odb(r);
 	for (odb = r->objects->odb; odb; odb = odb->next) {
-		if (oid_array_lookup(odb_loose_cache(odb, &oid), &oid) >= 0)
+		if (oid_array_lookup(odb_loose_cache(odb, oid), oid) >= 0)
 			return 1;
 	}
 	return 0;
@@ -1191,7 +1188,7 @@ static int loose_object_info(struct repository *r,
 		const char *path;
 		struct stat st;
 		if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
-			return quick_has_loose(r, oid->hash) ? 0 : -1;
+			return quick_has_loose(r, oid) ? 0 : -1;
 		if (stat_loose_object(r, oid, &st, &path) < 0)
 			return -1;
 		if (oi->disk_sizep)
@@ -1355,19 +1352,16 @@ int oid_object_info(struct repository *r,
 	return type;
 }
 
-static void *read_object(const unsigned char *sha1, enum object_type *type,
+static void *read_object(const struct object_id *oid, enum object_type *type,
 			 unsigned long *size)
 {
-	struct object_id oid;
 	struct object_info oi = OBJECT_INFO_INIT;
 	void *content;
 	oi.typep = type;
 	oi.sizep = size;
 	oi.contentp = &content;
 
-	hashcpy(oid.hash, sha1);
-
-	if (oid_object_info_extended(the_repository, &oid, &oi, 0) < 0)
+	if (oid_object_info_extended(the_repository, oid, &oi, 0) < 0)
 		return NULL;
 	return content;
 }
@@ -1408,7 +1402,7 @@ void *read_object_file_extended(const struct object_id *oid,
 		lookup_replace_object(the_repository, oid) : oid;
 
 	errno = 0;
-	data = read_object(repl->hash, type, size);
+	data = read_object(repl, type, size);
 	if (data)
 		return data;
 
@@ -1748,7 +1742,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
 
 	if (has_loose_object(oid))
 		return 0;
-	buf = read_object(oid->hash, &type, &len);
+	buf = read_object(oid, &type, &len);
 	if (!buf)
 		return error(_("cannot read sha1_file for %s"), oid_to_hex(oid));
 	hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %"PRIuMAX , type_name(type), (uintmax_t)len) + 1;
-- 
2.20.1.470.g640a3e2614


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox