Re: [PATCH v2 1/1] zlib.c: use size_t for size
On 15.10.18 06:22, Junio C Hamano wrote: > Ramsay Jones writes: > >>> >>> For the record, I am not opposed to including the comment _and_ using >>> xsize_t() to do the cast, giving us an assertion that the comment is >>> correct. >> >> Heh, I haven't found any enthusiasm tonight. Let's see if there >> are any more comments/opinions. > > OK, in the meantime, I've replaced the thread-starter partch I had > in 'pu' with your v3. > If that helps to move things forward: I'm fully fine with v3.
Re: [PATCH 4/4] merge-recursive: Avoid showing conflicts with merge branch before HEAD
Elijah Newren writes:
> The correct order usually comes naturally and for free, but with renames
> we often have data in the form {rename_branch, other_branch}, and
> working relative to the rename first (e.g. for rename/add) is more
> convenient elsewhere in the code. Address the slight impedance
> mismatch by having some functions re-call themselves with flipped
> arguments when the branch order is reversed.
I've never noticed or felt disturbed myself, but thanks for this
level of attention to the detail.
> @@ -228,7 +228,26 @@ static inline void setup_rename_conflict_info(enum
> rename_type rename_type,
> struct stage_data *src_entry1,
> struct stage_data *src_entry2)
> {
> - struct rename_conflict_info *ci = xcalloc(1, sizeof(struct
> rename_conflict_info));
> + struct rename_conflict_info *ci;
> +
> + /*
> + * When we have two renames involved, it's easiest to get the
> + * correct things into stage 2 and 3, and to make sure that the
> + * content merge puts HEAD before the other branch if we just
> + * ensure that branch1 == o->branch1. So, simply flip arguments
> + * around if we don't have that.
> + */
> + if (dst_entry2 && branch1 != o->branch1) {
> + setup_rename_conflict_info(rename_type,
> +pair2, pair1,
> +branch2,branch1,
> +dst_entry2, dst_entry1,
> +o,
> +src_entry2, src_entry1);
> + return;
> + }
;-)
Re: [PATCH 3/4] merge-recursive: improve auto-merging messages with path collisions
Elijah Newren writes: > Each individual file involved in a rename could have also been modified > on both sides of history, meaning it may need to have content merges. > If two such files are renamed into the same location, then on top of the > two natural auto-merging messages we also have to two-way merge the > result, giving us messages that look like > > Auto-merging somefile.c (was somecase.c) > Auto-merging somefile.c (was somefolder.c) > Auto-merging somefile.c > > However, despite the fact that I was the one who put the "(was %s)" > portions into the messages (and just a few months ago), I was still > initially confused when running into a rename/rename(2to1) case and > wondered if somefile.c had been merged three times. Update this to > instead be: > > Auto-merging version of somefile.c from somecase.c > Auto-merging version of somefile.c from someportfolio.c > Auto-merging somefile.c > > This is an admittedly long set of messages for a single path, but you > only get all three messages when dealing with the rare case of a > rename/rename(2to1) conflict where both sides of both original files > were also modified, in conflicting ways. Yeah, that does look mouthful, but definitely is more understandable.
Re: [PATCH 2/4] merge-recursive: increase marker length with depth of recursion
Elijah Newren writes: > When using merge.conflictstyle=diff3 to have the "base version" be shown > in conflicts, there is the possibility that the base version itself has > conflicts in it. This comes about when there are more than one merge > base, and the merging of those merge bases produces a conflict. > Since this process applies recursively, it is possible to have conflict > markers nested at an arbitrary depth; to be able to differentiate the > conflict markers from different nestings, we make them all of different > lengths. I know it is possible that the common ancestor part that is enclosed by the outermost makers can have arbitrary conflicts, and they can be even recursive conflicts. But I fail to see why it is a problem. Perhaps that is because I am not particularly good at resolving merge conflicts, but as long as the common part of the outermost merge is identifyable, would that really matter? What I would do while looking at common ancestor part with conflicts (not even a recursive one) is just to ignore it, so... Not that I strongly oppose to incrementing the marker length at every level. I do not think it would hurt, but I just do not see how it would help.
Re: [PATCH] object_id.cocci: match only expressions of type 'struct object_id'
Junio C Hamano writes: > SZEDER Gábor writes: > >> Most of our semantic patches in 'contrib/coccinelle/object_id.cocci' >> turn calls of SHA1-specific functions into calls of their >> corresponding object_id counterparts, e.g. sha1_to_hex() to >> oid_to_hex(). These semantic patches look something like this: >> >> @@ >> expression E1; >> @@ >> - sha1_to_hex(E1.hash) >> + oid_to_hex() >> >> and match the access to the 'hash' field in any data type, not only in >> 'struct object_id', and, consquently, can produce wrong >> transformations. > > Thanks, will queue. I ended up taking this as part of Brian's "the-hash-algo" series.
Re: [PATCH v2 1/1] zlib.c: use size_t for size
Ramsay Jones writes: >> >> For the record, I am not opposed to including the comment _and_ using >> xsize_t() to do the cast, giving us an assertion that the comment is >> correct. > > Heh, I haven't found any enthusiasm tonight. Let's see if there > are any more comments/opinions. OK, in the meantime, I've replaced the thread-starter partch I had in 'pu' with your v3.
Re: [RFC/PATCH] headers: normalize the spelling of some header guards
Jeff King writes: > I'm in favor of normalizing even the ones that aren't illegal, though > I'm OK either way on the vcs-svn bits if they're going away anyway. I'm in favour of normalizing even the ones that aren't illegal, too.
Re: [PATCH v12 0/8] filter: support for excluding all trees and blobs
Junio C Hamano writes: > Matthew DeVore writes: > >> Here is a re-roll-up since I haven't received any additional corrections for >> almost a week. > > Sorry, but doesn't this topic already sit in 'next'? If so, please make > these small clean-ups as incremental patches. Here is what I'd queue for now, with forged s-o-by from you ;-). Thanks. -- >8 -- From: Matthew DeVore Date: Fri, 12 Oct 2018 13:01:41 -0700 Subject: [PATCH] filter-trees: code clean-up of tests A few trivial updates to test to match the current best practices. - avoid "grep -q" that strips potentially useful output from running tests under "-v". - use test_write_lines to prepare multi-line expected output file - reserve use of test_must_fail to "git" commands. Signed-off-by: Matthew DeVore Signed-off-by: Junio C Hamano --- t/t5317-pack-objects-filter-objects.sh | 2 +- t/t5616-partial-clone.sh | 2 +- t/t6112-rev-list-filters-objects.sh| 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t5317-pack-objects-filter-objects.sh b/t/t5317-pack-objects-filter-objects.sh index 510d3537f6..d9dccf4d4d 100755 --- a/t/t5317-pack-objects-filter-objects.sh +++ b/t/t5317-pack-objects-filter-objects.sh @@ -69,7 +69,7 @@ test_expect_success 'get an error for missing tree object' ' test_must_fail git -C r5 pack-objects --rev --stdout 2>bad_tree <<-EOF && HEAD EOF - grep -q "bad tree object" bad_tree + grep "bad tree object" bad_tree ' test_expect_success 'setup for tests of tree:0' ' diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 53fbf7db88..392caa08fd 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -192,7 +192,7 @@ test_expect_success 'use fsck before and after manually fetching a missing subtr xargs -n1 git -C dst cat-file -t >fetched_types && sort -u fetched_types >unique_types.observed && - printf "blob\ncommit\ntree\n" >unique_types.expected && + test_write_lines blob commit tree >unique_types.expected && test_cmp unique_types.expected unique_types.observed ' diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh index 2cbb81d3bb..d24f9d5b5a 100755 --- a/t/t6112-rev-list-filters-objects.sh +++ b/t/t6112-rev-list-filters-objects.sh @@ -38,8 +38,8 @@ test_expect_success 'specify blob explicitly prevents filtering' ' awk -f print_2.awk) && git -C r1 rev-list --objects --filter=blob:none HEAD $file_3 >observed && - grep -q "$file_3" observed && - test_must_fail grep -q "$file_4" observed + grep "$file_3" observed && + ! grep "$file_4" observed ' test_expect_success 'verify emitted+omitted == all' ' @@ -241,7 +241,7 @@ test_expect_success 'verify tree:0 includes trees in "filtered" output' ' xargs -n1 git -C r3 cat-file -t >unsorted_filtered_types && sort -u unsorted_filtered_types >filtered_types && - printf "blob\ntree\n" >expected && + test_write_lines blob tree >expected && test_cmp expected filtered_types ' -- 2.19.1-328-g5a0cc8aca7
Re: [PATCH v3] gpg-interface.c: detect and reject multiple signatures on commits
Michał Górny writes:
> GnuPG supports creating signatures consisting of multiple signature
> packets. If such a signature is verified, it outputs all the status
> messages for each signature separately. However, git currently does not
> account for such scenario and gets terribly confused over getting
> multiple *SIG statuses.
>
> For example, if a malicious party alters a signed commit and appends
> a new untrusted signature, git is going to ignore the original bad
> signature and report untrusted commit instead. However, %GK and %GS
> format strings may still expand to the data corresponding
> to the original signature, potentially tricking the scripts into
> trusting the malicious commit.
>
> Given that the use of multiple signatures is quite rare, git does not
> support creating them without jumping through a few hoops, and finally
> supporting them properly would require extensive API improvement, it
> seems reasonable to just reject them at the moment.
>
> Signed-off-by: Michał Górny
> ---
> gpg-interface.c | 94 +++-
> t/t7510-signed-commit.sh | 26 +++
> 2 files changed, 91 insertions(+), 29 deletions(-)
>
> Changes in v3: reworked the whole loop to iterate over lines rather
> than scanning the whole buffer, as requested. Now it also catches
> duplicate instances of the same status.
>
> diff --git a/gpg-interface.c b/gpg-interface.c
> index db17d65f8..480aab4ee 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -75,48 +75,84 @@ void signature_check_clear(struct signature_check *sigc)
> FREE_AND_NULL(sigc->key);
> }
>
> +/* An exclusive status -- only one of them can appear in output */
> +#define GPG_STATUS_EXCLUSIVE (1<<0)
> +
> static struct {
> char result;
> const char *check;
> + unsigned int flags;
> } sigcheck_gpg_status[] = {
> - { 'G', "\n[GNUPG:] GOODSIG " },
> - { 'B', "\n[GNUPG:] BADSIG " },
> - { 'U', "\n[GNUPG:] TRUST_NEVER" },
> - { 'U', "\n[GNUPG:] TRUST_UNDEFINED" },
> - { 'E', "\n[GNUPG:] ERRSIG "},
> - { 'X', "\n[GNUPG:] EXPSIG "},
> - { 'Y', "\n[GNUPG:] EXPKEYSIG "},
> - { 'R', "\n[GNUPG:] REVKEYSIG "},
> + { 'G', "GOODSIG ", GPG_STATUS_EXCLUSIVE },
> + { 'B', "BADSIG ", GPG_STATUS_EXCLUSIVE },
> + { 'U', "TRUST_NEVER", 0 },
> + { 'U', "TRUST_UNDEFINED", 0 },
> + { 'E', "ERRSIG ", GPG_STATUS_EXCLUSIVE },
> + { 'X', "EXPSIG ", GPG_STATUS_EXCLUSIVE },
> + { 'Y', "EXPKEYSIG ", GPG_STATUS_EXCLUSIVE },
> + { 'R', "REVKEYSIG ", GPG_STATUS_EXCLUSIVE },
> };
>
> static void parse_gpg_output(struct signature_check *sigc)
> {
> const char *buf = sigc->gpg_status;
> + const char *line, *next;
> int i;
> -
> - /* Iterate over all search strings */
> - for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
> - const char *found, *next;
> -
> - if (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1,
> )) {
> - found = strstr(buf, sigcheck_gpg_status[i].check);
> - if (!found)
> - continue;
> - found += strlen(sigcheck_gpg_status[i].check);
> - }
> - sigc->result = sigcheck_gpg_status[i].result;
> - /* The trust messages are not followed by key/signer
> information */
> - if (sigc->result != 'U') {
> - next = strchrnul(found, ' ');
> - sigc->key = xmemdupz(found, next - found);
> - /* The ERRSIG message is not followed by signer
> information */
> - if (*next && sigc-> result != 'E') {
> - found = next + 1;
> - next = strchrnul(found, '\n');
> - sigc->signer = xmemdupz(found, next - found);
> + int had_exclusive_status = 0;
> +
> + /* Iterate over all lines */
> + for (line = buf; *line; line = strchrnul(line+1, '\n')) {
> + while (*line == '\n')
> + line++;
> + /* Skip lines that don't start with GNUPG status */
> + if (strncmp(line, "[GNUPG:] ", 9))
> + continue;
> + line += 9;
You do not want to count to 9 yourself. Instead
if (!skip_prefix(line, "[GNUPG:] ", ))
continue;
> + /* Iterate over all search strings */
> + for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
> + if (!strncmp(line, sigcheck_gpg_status[i].check,
> + strlen(sigcheck_gpg_status[i].check))) {
> + line += strlen(sigcheck_gpg_status[i].check);
Likewise.
> + if (sigcheck_gpg_status[i].flags &
> GPG_STATUS_EXCLUSIVE)
> + had_exclusive_status++;
"has" is fine, but I think existing code elsewhere we use "seen"
Re: [PATCH v3] gpg-interface.c: detect and reject multiple signatures on commits
Thanks, will take a look.
Re: [PATCH v12 0/8] filter: support for excluding all trees and blobs
Matthew DeVore writes: > Here is a re-roll-up since I haven't received any additional corrections for > almost a week. Sorry, but doesn't this topic already in 'next'? If so, please make these small clean-ups as incremental patches. Thansk.
Re: [PATCH] object_id.cocci: match only expressions of type 'struct object_id'
SZEDER Gábor writes: > Most of our semantic patches in 'contrib/coccinelle/object_id.cocci' > turn calls of SHA1-specific functions into calls of their > corresponding object_id counterparts, e.g. sha1_to_hex() to > oid_to_hex(). These semantic patches look something like this: > > @@ > expression E1; > @@ > - sha1_to_hex(E1.hash) > + oid_to_hex() > > and match the access to the 'hash' field in any data type, not only in > 'struct object_id', and, consquently, can produce wrong > transformations. Thanks, will queue.
Re: [RFC PATCH 3/3] list-objects-filter: teach tree:# how to handle >0
Matthew DeVore writes:
> The long-term goal at the end of this is to allow a partial clone to
> eagerly fetch an entire directory of files by fetching a tree and
> specifying =1. This, for instance, would make a build operation
> fast and convenient
This would reduce round-trip, as you do not have to fetch the tree
to see what its contents are before issuing another set of fetches
for them. Those who are building virtual filesystem that let you
mount a specific tree object, perhaps via fuse, may find it useful,
too, even though I suspect that may not be your primary focus.
> diff --git a/Documentation/rev-list-options.txt
> b/Documentation/rev-list-options.txt
> index c2c1c40e6..c78985c41 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -734,8 +734,12 @@ specification contained in .
> +
> The form '--filter=tree:' omits all blobs and trees whose depth
> from the root tree is >= (minimum depth if an object is located
> -at multiple depths in the commits traversed). Currently, only =0
> -is supported, which omits all blobs and trees.
> +at multiple depths in the commits traversed). =0 will not include
> +any trees or blobs unless included explicitly in . =1
Here, refers to the objects directly requested on the
command line (or --stdin)? Triggering this question from me is a
sign that this description may want to say a bit more to avoid the
same question from the real readers. Perhaps replace "included
explicitly in " with "explicitly requested by listing them
on the command line or feeding them with `--stdin`", or something
like that?
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index e8da2e858..9dc61d6e6 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -50,12 +50,12 @@ static int gently_parse_list_objects_filter(
> }
>
> } else if (skip_prefix(arg, "tree:", )) {
> - unsigned long depth;
> - if (!git_parse_ulong(v0, ) || depth != 0) {
> + if (!git_parse_ulong(v0,
> + _options->tree_depth_limit_value)) {
> if (errbuf) {
> strbuf_addstr(
> errbuf,
> - _("only 'tree:0' is supported"));
> + _("expected 'tree:'"));
We do not accept "tree:-1", even though "-1" is an int. Is it too
obvious to worry about? I do not think we want to say tree:
even if we do want to make it clear we reject "tree:-1"
I am wondering if "expected 'tree:'" would work better.
> diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
> index af64e5c66..c1ae70cd8 100644
> --- a/list-objects-filter-options.h
> +++ b/list-objects-filter-options.h
> @@ -44,6 +44,7 @@ struct list_objects_filter_options {
> struct object_id *sparse_oid_value;
> char *sparse_path_value;
> unsigned long blob_limit_value;
> + unsigned long tree_depth_limit_value;
> };
This change gets it right by adding "depth" in the field name and it
is not a comment on this patch, but someday not in too distant
future we should rename the "blob_limit_value" to make it clear that
it is filtering by number of bytes, as other filtering criteria on
blobs that can be expressed in ulong are quite possible.
> -static enum list_objects_filter_result filter_trees_none(
> +static enum list_objects_filter_result filter_trees_depth(
> enum list_objects_filter_situation filter_situation,
> struct object *obj,
> const char *pathname,
> const char *filename,
> void *filter_data_)
> {
> - struct filter_trees_none_data *filter_data = filter_data_;
> + struct filter_trees_depth_data *filter_data = filter_data_;
> +
> + int too_deep = filter_data->current_depth >= filter_data->max_depth;
Does max mean "maximum allowed", or "this and anything larger are
rejected". The latter sound wrong, but I offhand do not know if
your current_depth counts from 0 or 1, so there may not be
off-by-one.
- dir.c::within_depth() that is used by pathspec matching that in turn
is used by "grep --max-depth=1" does "if (depth > max_depth)", which
sounds more in line with the usual convention, I would think.
- pack-objects has max_delta_cache_size, which also is used as
"maximum allowed", not "this is already too big". So is its
max_depth.
There may be other examples. One existing violator I noticed was
the "reject blobs that is this size or larger" in this file; it is
called "max_bytes", but it is apparently not "maximum allowed",
which we probably would want to fix.
> + /*
> + * Note that we do not use _MARK_SEEN in order to allow re-traversal in
> + * case we encounter a tree or blob again at a shallower depth.
> + */
Hmph. Earlier tree:0 support never even read the actual tree, so
this was not a problem.
[PATCH v2 04/13] cache: make hashcmp and hasheq work with larger hashes
In 183a638b7d ("hashcmp: assert constant hash size", 2018-08-23), we
modified hashcmp to assert that the hash size was always 20 to help it
optimize and inline calls to memcmp. In a future series, we replaced
many calls to hashcmp and oidcmp with calls to hasheq and oideq to
improve inlining further.
However, we want to support hash algorithms other than SHA-1, namely
SHA-256. When doing so, we must handle the case where these values are
32 bytes long as well as 20. Adjust hashcmp to handle two cases:
20-byte matches, and maximum-size matches. Therefore, when we include
SHA-256, we'll automatically handle it properly, while at the same time
teaching the compiler that there are only two possible options to
consider. This will allow the compiler to write the most efficient
possible code.
Copy similar code into hasheq and perform an identical transformation.
At least with GCC 8.2.0, making hasheq defer to hashcmp when there are
two branches prevents the compiler from inlining the comparison, while
the code in this patch is inlined properly. Add a comment to avoid an
accidental performance regression from well-intentioned refactoring.
Signed-off-by: brian m. carlson
---
cache.h | 22 --
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/cache.h b/cache.h
index a13d14ce0a..0b88c3a344 100644
--- a/cache.h
+++ b/cache.h
@@ -1024,16 +1024,12 @@ extern const struct object_id null_oid;
static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
{
/*
-* This is a temporary optimization hack. By asserting the size here,
-* we let the compiler know that it's always going to be 20, which lets
-* it turn this fixed-size memcmp into a few inline instructions.
-*
-* This will need to be extended or ripped out when we learn about
-* hashes of different sizes.
+* Teach the compiler that there are only two possibilities of hash size
+* here, so that it can optimize for this case as much as possible.
*/
- if (the_hash_algo->rawsz != 20)
- BUG("hash size not yet supported by hashcmp");
- return memcmp(sha1, sha2, the_hash_algo->rawsz);
+ if (the_hash_algo->rawsz == GIT_MAX_RAWSZ)
+ return memcmp(sha1, sha2, GIT_MAX_RAWSZ);
+ return memcmp(sha1, sha2, GIT_SHA1_RAWSZ);
}
static inline int oidcmp(const struct object_id *oid1, const struct object_id
*oid2)
@@ -1043,7 +1039,13 @@ static inline int oidcmp(const struct object_id *oid1,
const struct object_id *o
static inline int hasheq(const unsigned char *sha1, const unsigned char *sha2)
{
- return !hashcmp(sha1, sha2);
+ /*
+* We write this here instead of deferring to hashcmp so that the
+* compiler can properly inline it and avoid calling memcmp.
+*/
+ if (the_hash_algo->rawsz == GIT_MAX_RAWSZ)
+ return !memcmp(sha1, sha2, GIT_MAX_RAWSZ);
+ return !memcmp(sha1, sha2, GIT_SHA1_RAWSZ);
}
static inline int oideq(const struct object_id *oid1, const struct object_id
*oid2)
[PATCH v2 06/13] t: make the sha1 test-tool helper generic
Since we're going to have multiple hash algorithms to test, it makes
sense to share as much of the test code as possible. Convert the sha1
helper for the test-tool to be generic and move it out into its own
module. This will allow us to share most of this code with our NewHash
implementation.
Signed-off-by: brian m. carlson
---
Makefile | 1 +
t/helper/{test-sha1.c => test-hash.c} | 19 +-
t/helper/test-sha1.c | 52 +--
t/helper/test-tool.h | 2 ++
4 files changed, 14 insertions(+), 60 deletions(-)
copy t/helper/{test-sha1.c => test-hash.c} (66%)
diff --git a/Makefile b/Makefile
index 5c8307b7c4..324967410d 100644
--- a/Makefile
+++ b/Makefile
@@ -714,6 +714,7 @@ TEST_BUILTINS_OBJS += test-dump-split-index.o
TEST_BUILTINS_OBJS += test-dump-untracked-cache.o
TEST_BUILTINS_OBJS += test-example-decorate.o
TEST_BUILTINS_OBJS += test-genrandom.o
+TEST_BUILTINS_OBJS += test-hash.o
TEST_BUILTINS_OBJS += test-hashmap.o
TEST_BUILTINS_OBJS += test-index-version.o
TEST_BUILTINS_OBJS += test-json-writer.o
diff --git a/t/helper/test-sha1.c b/t/helper/test-hash.c
similarity index 66%
copy from t/helper/test-sha1.c
copy to t/helper/test-hash.c
index 1ba0675c75..9992de2212 100644
--- a/t/helper/test-sha1.c
+++ b/t/helper/test-hash.c
@@ -1,13 +1,14 @@
#include "test-tool.h"
#include "cache.h"
-int cmd__sha1(int ac, const char **av)
+int cmd_hash_impl(int ac, const char **av, int algo)
{
- git_SHA_CTX ctx;
- unsigned char sha1[20];
+ git_hash_ctx ctx;
+ unsigned char hash[GIT_MAX_HEXSZ];
unsigned bufsz = 8192;
int binary = 0;
char *buffer;
+ const struct git_hash_algo *algop = _algos[algo];
if (ac == 2) {
if (!strcmp(av[1], "-b"))
@@ -26,7 +27,7 @@ int cmd__sha1(int ac, const char **av)
die("OOPS");
}
- git_SHA1_Init();
+ algop->init_fn();
while (1) {
ssize_t sz, this_sz;
@@ -38,20 +39,20 @@ int cmd__sha1(int ac, const char **av)
if (sz == 0)
break;
if (sz < 0)
- die_errno("test-sha1");
+ die_errno("test-hash");
this_sz += sz;
cp += sz;
room -= sz;
}
if (this_sz == 0)
break;
- git_SHA1_Update(, buffer, this_sz);
+ algop->update_fn(, buffer, this_sz);
}
- git_SHA1_Final(sha1, );
+ algop->final_fn(hash, );
if (binary)
- fwrite(sha1, 1, 20, stdout);
+ fwrite(hash, 1, algop->rawsz, stdout);
else
- puts(sha1_to_hex(sha1));
+ puts(hash_to_hex_algo(hash, algo));
exit(0);
}
diff --git a/t/helper/test-sha1.c b/t/helper/test-sha1.c
index 1ba0675c75..d860c387c3 100644
--- a/t/helper/test-sha1.c
+++ b/t/helper/test-sha1.c
@@ -3,55 +3,5 @@
int cmd__sha1(int ac, const char **av)
{
- git_SHA_CTX ctx;
- unsigned char sha1[20];
- unsigned bufsz = 8192;
- int binary = 0;
- char *buffer;
-
- if (ac == 2) {
- if (!strcmp(av[1], "-b"))
- binary = 1;
- else
- bufsz = strtoul(av[1], NULL, 10) * 1024 * 1024;
- }
-
- if (!bufsz)
- bufsz = 8192;
-
- while ((buffer = malloc(bufsz)) == NULL) {
- fprintf(stderr, "bufsz %u is too big, halving...\n", bufsz);
- bufsz /= 2;
- if (bufsz < 1024)
- die("OOPS");
- }
-
- git_SHA1_Init();
-
- while (1) {
- ssize_t sz, this_sz;
- char *cp = buffer;
- unsigned room = bufsz;
- this_sz = 0;
- while (room) {
- sz = xread(0, cp, room);
- if (sz == 0)
- break;
- if (sz < 0)
- die_errno("test-sha1");
- this_sz += sz;
- cp += sz;
- room -= sz;
- }
- if (this_sz == 0)
- break;
- git_SHA1_Update(, buffer, this_sz);
- }
- git_SHA1_Final(sha1, );
-
- if (binary)
- fwrite(sha1, 1, 20, stdout);
- else
- puts(sha1_to_hex(sha1));
- exit(0);
+ return cmd_hash_impl(ac, av, GIT_HASH_SHA1);
}
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index e4890566da..29ac7b0b0d 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -50,4 +50,6 @@ int cmd__windows_named_pipe(int argc, const char **argv);
#endif
int cmd__write_cache(int argc, const char **argv);
[PATCH v2 07/13] sha1-file: add a constant for hash block size
There is one place we need the hash algorithm block size: the HMAC code
for push certs. Expose this constant in struct git_hash_algo and expose
values for SHA-1 and for the largest value of any hash.
Signed-off-by: brian m. carlson
---
cache.h | 4
hash.h | 3 +++
sha1-file.c | 2 ++
3 files changed, 9 insertions(+)
diff --git a/cache.h b/cache.h
index 0b88c3a344..48e736b0d5 100644
--- a/cache.h
+++ b/cache.h
@@ -45,10 +45,14 @@ unsigned long git_deflate_bound(git_zstream *, unsigned
long);
/* The length in bytes and in hex digits of an object name (SHA-1 value). */
#define GIT_SHA1_RAWSZ 20
#define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
+/* The block size of SHA-1. */
+#define GIT_SHA1_BLKSZ 64
/* The length in byte and in hex digits of the largest possible hash value. */
#define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ
#define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ
+/* The largest possible block size for any supported hash. */
+#define GIT_MAX_BLKSZ GIT_SHA1_BLKSZ
struct object_id {
unsigned char hash[GIT_MAX_RAWSZ];
diff --git a/hash.h b/hash.h
index 90f4344619..46dff69eb3 100644
--- a/hash.h
+++ b/hash.h
@@ -81,6 +81,9 @@ struct git_hash_algo {
/* The length of the hash in hex characters. */
size_t hexsz;
+ /* The block size of the hash. */
+ size_t blksz;
+
/* The hash initialization function. */
git_hash_init_fn init_fn;
diff --git a/sha1-file.c b/sha1-file.c
index 3a75d515eb..9aadaf0c8d 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -90,6 +90,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
0x,
0,
0,
+ 0,
git_hash_unknown_init,
git_hash_unknown_update,
git_hash_unknown_final,
@@ -102,6 +103,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
0x73686131,
GIT_SHA1_RAWSZ,
GIT_SHA1_HEXSZ,
+ GIT_SHA1_BLKSZ,
git_hash_sha1_init,
git_hash_sha1_update,
git_hash_sha1_final,
[PATCH v2 13/13] commit-graph: specify OID version for SHA-256
Since the commit-graph code wants to serialize the hash algorithm into
the data store, specify a version number for each supported algorithm.
Note that we don't use the values of the constants themselves, as they
are internal and could change in the future.
Signed-off-by: brian m. carlson
---
commit-graph.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/commit-graph.c b/commit-graph.c
index 7a28fbb03f..e587c21bb6 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -45,7 +45,14 @@ char *get_commit_graph_filename(const char *obj_dir)
static uint8_t oid_version(void)
{
- return 1;
+ switch (hash_algo_by_ptr(the_hash_algo)) {
+ case GIT_HASH_SHA1:
+ return 1;
+ case GIT_HASH_SHA256:
+ return 2;
+ default:
+ BUG("unknown hash algorithm");
+ }
}
static struct commit_graph *alloc_commit_graph(void)
[PATCH v2 02/13] sha1-file: provide functions to look up hash algorithms
There are several ways we might refer to a hash algorithm: by name, such
as in the config file; by format ID, such as in a pack; or internally,
by a pointer to the hash_algos array. Provide functions to look up hash
algorithms based on these various forms and return the internal constant
used for them. If conversion to another form is necessary, this
internal constant can be used to look up the proper data in the
hash_algos array.
Signed-off-by: brian m. carlson
---
hash.h | 13 +
sha1-file.c | 21 +
2 files changed, 34 insertions(+)
diff --git a/hash.h b/hash.h
index 7c8238bc2e..90f4344619 100644
--- a/hash.h
+++ b/hash.h
@@ -98,4 +98,17 @@ struct git_hash_algo {
};
extern const struct git_hash_algo hash_algos[GIT_HASH_NALGOS];
+/*
+ * Return a GIT_HASH_* constant based on the name. Returns GIT_HASH_UNKNOWN if
+ * the name doesn't match a known algorithm.
+ */
+int hash_algo_by_name(const char *name);
+/* Identical, except based on the format ID. */
+int hash_algo_by_id(uint32_t format_id);
+/* Identical, except for a pointer to struct git_hash_algo. */
+inline int hash_algo_by_ptr(const struct git_hash_algo *p)
+{
+ return p - hash_algos;
+}
+
#endif
diff --git a/sha1-file.c b/sha1-file.c
index e29825f259..3a75d515eb 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -122,6 +122,27 @@ const char *empty_blob_oid_hex(void)
return oid_to_hex_r(buf, the_hash_algo->empty_blob);
}
+int hash_algo_by_name(const char *name)
+{
+ int i;
+ if (!name)
+ return GIT_HASH_UNKNOWN;
+ for (i = 1; i < GIT_HASH_NALGOS; i++)
+ if (!strcmp(name, hash_algos[i].name))
+ return i;
+ return GIT_HASH_UNKNOWN;
+}
+
+int hash_algo_by_id(uint32_t format_id)
+{
+ int i;
+ for (i = 1; i < GIT_HASH_NALGOS; i++)
+ if (format_id == hash_algos[i].format_id)
+ return i;
+ return GIT_HASH_UNKNOWN;
+}
+
+
/*
* This is meant to hold a *small* number of objects that you would
* want read_sha1_file() to be able to return, but yet you do not want
[PATCH v2 11/13] sha256: add an SHA-256 implementation using libgcrypt
Generally, one gets better performance out of cryptographic routines
written in assembly than C, and this is also true for SHA-256. In
addition, most Linux distributions cannot distribute Git linked against
OpenSSL for licensing reasons.
Most systems with GnuPG will also have libgcrypt, since it is a
dependency of GnuPG. libgcrypt is also faster than the SHA1DC
implementation for messages of a few KiB and larger. It is licensed
under the LGPL 2.1, which is compatible with the GPL.
Add an implementation of SHA-256 that uses libgcrypt.
Signed-off-by: brian m. carlson
---
Makefile| 13 +++--
hash.h | 4
sha256/gcrypt.h | 30 ++
3 files changed, 45 insertions(+), 2 deletions(-)
create mode 100644 sha256/gcrypt.h
diff --git a/Makefile b/Makefile
index 76d378c7ba..3d91555a81 100644
--- a/Makefile
+++ b/Makefile
@@ -179,6 +179,10 @@ all::
# in one call to the platform's SHA1_Update(). e.g. APPLE_COMMON_CRYPTO
# wants 'SHA1_MAX_BLOCK_SIZE=1024L*1024L*1024L' defined.
#
+# Define BLK_SHA256 to use the built-in SHA-256 routines.
+#
+# Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt.
+#
# Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin).
#
# Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).
@@ -1634,8 +1638,13 @@ endif
endif
endif
-LIB_OBJS += sha256/block/sha256.o
-BASIC_CFLAGS += -DSHA256_BLK
+ifdef GCRYPT_SHA256
+ BASIC_CFLAGS += -DSHA256_GCRYPT
+ EXTLIBS += -lgcrypt
+else
+ LIB_OBJS += sha256/block/sha256.o
+ BASIC_CFLAGS += -DSHA256_BLK
+endif
ifdef SHA1_MAX_BLOCK_SIZE
LIB_OBJS += compat/sha1-chunked.o
diff --git a/hash.h b/hash.h
index 88d18896d7..9df562f2f6 100644
--- a/hash.h
+++ b/hash.h
@@ -15,7 +15,11 @@
#include "block-sha1/sha1.h"
#endif
+#if defined(SHA256_GCRYPT)
+#include "sha256/gcrypt.h"
+#else
#include "sha256/block/sha256.h"
+#endif
#ifndef platform_SHA_CTX
/*
diff --git a/sha256/gcrypt.h b/sha256/gcrypt.h
new file mode 100644
index 00..09bd8bb200
--- /dev/null
+++ b/sha256/gcrypt.h
@@ -0,0 +1,30 @@
+#ifndef SHA256_GCRYPT_H
+#define SHA256_GCRYPT_H
+
+#include
+
+#define SHA256_DIGEST_SIZE 32
+
+typedef gcry_md_hd_t gcrypt_SHA256_CTX;
+
+inline void gcrypt_SHA256_Init(gcrypt_SHA256_CTX *ctx)
+{
+ gcry_md_open(ctx, GCRY_MD_SHA256, 0);
+}
+
+inline void gcrypt_SHA256_Update(gcrypt_SHA256_CTX *ctx, const void *data,
size_t len)
+{
+ gcry_md_write(*ctx, data, len);
+}
+
+inline void gcrypt_SHA256_Final(unsigned char *digest, gcrypt_SHA256_CTX *ctx)
+{
+ memcpy(digest, gcry_md_read(*ctx, GCRY_MD_SHA256), SHA256_DIGEST_SIZE);
+}
+
+#define platform_SHA256_CTX gcrypt_SHA256_CTX
+#define platform_SHA256_Init gcrypt_SHA256_Init
+#define platform_SHA256_Update gcrypt_SHA256_Update
+#define platform_SHA256_Final gcrypt_SHA256_Final
+
+#endif
[PATCH v2 12/13] hash: add an SHA-256 implementation using OpenSSL
We already have OpenSSL routines available for SHA-1, so add routines for SHA-256 as well. Signed-off-by: brian m. carlson --- Makefile | 7 +++ hash.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/Makefile b/Makefile index 3d91555a81..3164e2aeee 100644 --- a/Makefile +++ b/Makefile @@ -183,6 +183,8 @@ all:: # # Define GCRYPT_SHA256 to use the SHA-256 routines in libgcrypt. # +# Define OPENSSL_SHA256 to use the SHA-256 routines in OpenSSL. +# # Define NEEDS_CRYPTO_WITH_SSL if you need -lcrypto when using -lssl (Darwin). # # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin). @@ -1638,6 +1640,10 @@ endif endif endif +ifdef OPENSSL_SHA256 + EXTLIBS += $(LIB_4_CRYPTO) + BASIC_CFLAGS += -DSHA256_OPENSSL +else ifdef GCRYPT_SHA256 BASIC_CFLAGS += -DSHA256_GCRYPT EXTLIBS += -lgcrypt @@ -1645,6 +1651,7 @@ else LIB_OBJS += sha256/block/sha256.o BASIC_CFLAGS += -DSHA256_BLK endif +endif ifdef SHA1_MAX_BLOCK_SIZE LIB_OBJS += compat/sha1-chunked.o diff --git a/hash.h b/hash.h index 9df562f2f6..9df06d56b4 100644 --- a/hash.h +++ b/hash.h @@ -17,6 +17,8 @@ #if defined(SHA256_GCRYPT) #include "sha256/gcrypt.h" +#elif defined(SHA256_OPENSSL) +#include #else #include "sha256/block/sha256.h" #endif
[PATCH v2 10/13] Add a base implementation of SHA-256 support
SHA-1 is weak and we need to transition to a new hash function. For
some time, we have referred to this new function as NewHash. Recently,
we decided to pick SHA-256 as NewHash.
Add a basic implementation of SHA-256 based off libtomcrypt, which is in
the public domain. Optimize it and restructure it to meet our coding
standards. Place it in a directory called "sha256" where it and any
future implementations can live so as to avoid a proliferation of
implementation directories.
Wire up SHA-256 in the list of hash algorithms, and add a test that the
algorithm works correctly.
Note that with this patch, it is still not possible to switch to using
SHA-256 in Git. Additional patches are needed to prepare the code to
handle a larger hash algorithm and further test fixes are needed.
Signed-off-by: brian m. carlson
---
Makefile | 4 +
cache.h| 12 ++-
hash.h | 19 -
sha1-file.c| 45 +++
sha256/block/sha256.c | 180 +
sha256/block/sha256.h | 26 ++
t/helper/test-sha256.c | 7 ++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0014-hash.sh| 25 ++
10 files changed, 316 insertions(+), 4 deletions(-)
create mode 100644 sha256/block/sha256.c
create mode 100644 sha256/block/sha256.h
create mode 100644 t/helper/test-sha256.c
diff --git a/Makefile b/Makefile
index 1c43bf9aa8..76d378c7ba 100644
--- a/Makefile
+++ b/Makefile
@@ -739,6 +739,7 @@ TEST_BUILTINS_OBJS += test-run-command.o
TEST_BUILTINS_OBJS += test-scrap-cache-tree.o
TEST_BUILTINS_OBJS += test-sha1.o
TEST_BUILTINS_OBJS += test-sha1-array.o
+TEST_BUILTINS_OBJS += test-sha256.o
TEST_BUILTINS_OBJS += test-sigchain.o
TEST_BUILTINS_OBJS += test-strcmp-offset.o
TEST_BUILTINS_OBJS += test-string-list.o
@@ -1633,6 +1634,9 @@ endif
endif
endif
+LIB_OBJS += sha256/block/sha256.o
+BASIC_CFLAGS += -DSHA256_BLK
+
ifdef SHA1_MAX_BLOCK_SIZE
LIB_OBJS += compat/sha1-chunked.o
BASIC_CFLAGS += -DSHA1_MAX_BLOCK_SIZE="$(SHA1_MAX_BLOCK_SIZE)"
diff --git a/cache.h b/cache.h
index 48e736b0d5..81ece0360e 100644
--- a/cache.h
+++ b/cache.h
@@ -48,11 +48,17 @@ unsigned long git_deflate_bound(git_zstream *, unsigned
long);
/* The block size of SHA-1. */
#define GIT_SHA1_BLKSZ 64
+/* The length in bytes and in hex digits of an object name (SHA-256 value). */
+#define GIT_SHA256_RAWSZ 32
+#define GIT_SHA256_HEXSZ (2 * GIT_SHA256_RAWSZ)
+/* The block size of SHA-256. */
+#define GIT_SHA256_BLKSZ 64
+
/* The length in byte and in hex digits of the largest possible hash value. */
-#define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ
-#define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ
+#define GIT_MAX_RAWSZ GIT_SHA256_RAWSZ
+#define GIT_MAX_HEXSZ GIT_SHA256_HEXSZ
/* The largest possible block size for any supported hash. */
-#define GIT_MAX_BLKSZ GIT_SHA1_BLKSZ
+#define GIT_MAX_BLKSZ GIT_SHA256_BLKSZ
struct object_id {
unsigned char hash[GIT_MAX_RAWSZ];
diff --git a/hash.h b/hash.h
index 46dff69eb3..88d18896d7 100644
--- a/hash.h
+++ b/hash.h
@@ -15,6 +15,8 @@
#include "block-sha1/sha1.h"
#endif
+#include "sha256/block/sha256.h"
+
#ifndef platform_SHA_CTX
/*
* platform's underlying implementation of SHA-1; could be OpenSSL,
@@ -34,6 +36,18 @@
#define git_SHA1_Updateplatform_SHA1_Update
#define git_SHA1_Final platform_SHA1_Final
+#ifndef platform_SHA256_CTX
+#define platform_SHA256_CTXSHA256_CTX
+#define platform_SHA256_Init SHA256_Init
+#define platform_SHA256_Update SHA256_Update
+#define platform_SHA256_Final SHA256_Final
+#endif
+
+#define git_SHA256_CTX platform_SHA256_CTX
+#define git_SHA256_Initplatform_SHA256_Init
+#define git_SHA256_Update platform_SHA256_Update
+#define git_SHA256_Final platform_SHA256_Final
+
#ifdef SHA1_MAX_BLOCK_SIZE
#include "compat/sha1-chunked.h"
#undef git_SHA1_Update
@@ -52,12 +66,15 @@
#define GIT_HASH_UNKNOWN 0
/* SHA-1 */
#define GIT_HASH_SHA1 1
+/* SHA-256 */
+#define GIT_HASH_SHA256 2
/* Number of algorithms supported (including unknown). */
-#define GIT_HASH_NALGOS (GIT_HASH_SHA1 + 1)
+#define GIT_HASH_NALGOS (GIT_HASH_SHA256 + 1)
/* A suitably aligned type for stack allocations of hash contexts. */
union git_hash_ctx {
git_SHA_CTX sha1;
+ git_SHA256_CTX sha256;
};
typedef union git_hash_ctx git_hash_ctx;
diff --git a/sha1-file.c b/sha1-file.c
index 9aadaf0c8d..66ba3dadb9 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -40,10 +40,20 @@
#define EMPTY_TREE_SHA1_BIN_LITERAL \
"\x4b\x82\x5d\xc6\x42\xcb\x6e\xb9\xa0\x60" \
"\xe5\x4b\xf8\xd6\x92\x88\xfb\xee\x49\x04"
+#define EMPTY_TREE_SHA256_BIN_LITERAL \
+ "\x6e\xf1\x9b\x41\x22\x5c\x53\x69\xf1\xc1" \
+ "\x04\xd4\x5d\x8d\x85\xef\xa9\xb0\x57\xb5" \
+ "\x3b\x14\xb4\xb9\xb9\x39\xdd\x74\xde\xcc" \
+ "\x53\x21"
#define EMPTY_BLOB_SHA1_BIN_LITERAL \
[PATCH v2 09/13] commit-graph: convert to using the_hash_algo
Instead of using hard-coded constants for object sizes, use
the_hash_algo to look them up. In addition, use a function call to look
up the object ID version and produce the correct value.
Signed-off-by: brian m. carlson
---
commit-graph.c | 33 +
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 7f4519ec3b..7a28fbb03f 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -20,16 +20,11 @@
#define GRAPH_CHUNKID_DATA 0x43444154 /* "CDAT" */
#define GRAPH_CHUNKID_LARGEEDGES 0x45444745 /* "EDGE" */
-#define GRAPH_DATA_WIDTH 36
+#define GRAPH_DATA_WIDTH (the_hash_algo->rawsz + 16)
#define GRAPH_VERSION_1 0x1
#define GRAPH_VERSION GRAPH_VERSION_1
-#define GRAPH_OID_VERSION_SHA1 1
-#define GRAPH_OID_LEN_SHA1 GIT_SHA1_RAWSZ
-#define GRAPH_OID_VERSION GRAPH_OID_VERSION_SHA1
-#define GRAPH_OID_LEN GRAPH_OID_LEN_SHA1
-
#define GRAPH_OCTOPUS_EDGES_NEEDED 0x8000
#define GRAPH_PARENT_MISSING 0x7fff
#define GRAPH_EDGE_LAST_MASK 0x7fff
@@ -41,13 +36,18 @@
#define GRAPH_FANOUT_SIZE (4 * 256)
#define GRAPH_CHUNKLOOKUP_WIDTH 12
#define GRAPH_MIN_SIZE (GRAPH_HEADER_SIZE + 4 * GRAPH_CHUNKLOOKUP_WIDTH \
- + GRAPH_FANOUT_SIZE + GRAPH_OID_LEN)
+ + GRAPH_FANOUT_SIZE + the_hash_algo->rawsz)
char *get_commit_graph_filename(const char *obj_dir)
{
return xstrfmt("%s/info/commit-graph", obj_dir);
}
+static uint8_t oid_version(void)
+{
+ return 1;
+}
+
static struct commit_graph *alloc_commit_graph(void)
{
struct commit_graph *g = xcalloc(1, sizeof(*g));
@@ -100,15 +100,15 @@ struct commit_graph *load_commit_graph_one(const char
*graph_file)
}
hash_version = *(unsigned char*)(data + 5);
- if (hash_version != GRAPH_OID_VERSION) {
+ if (hash_version != oid_version()) {
error(_("hash version %X does not match version %X"),
- hash_version, GRAPH_OID_VERSION);
+ hash_version, oid_version());
goto cleanup_fail;
}
graph = alloc_commit_graph();
- graph->hash_len = GRAPH_OID_LEN;
+ graph->hash_len = the_hash_algo->rawsz;
graph->num_chunks = *(unsigned char*)(data + 6);
graph->graph_fd = fd;
graph->data = graph_map;
@@ -124,7 +124,7 @@ struct commit_graph *load_commit_graph_one(const char
*graph_file)
chunk_lookup += GRAPH_CHUNKLOOKUP_WIDTH;
- if (chunk_offset > graph_size - GIT_MAX_RAWSZ) {
+ if (chunk_offset > graph_size - the_hash_algo->rawsz) {
error(_("improper chunk offset %08x%08x"),
(uint32_t)(chunk_offset >> 32),
(uint32_t)chunk_offset);
goto cleanup_fail;
@@ -711,6 +711,7 @@ void write_commit_graph(const char *obj_dir,
int num_chunks;
int num_extra_edges;
struct commit_list *parent;
+ const unsigned hashsz = the_hash_algo->rawsz;
oids.nr = 0;
oids.alloc = approximate_object_count() / 4;
@@ -831,7 +832,7 @@ void write_commit_graph(const char *obj_dir,
hashwrite_be32(f, GRAPH_SIGNATURE);
hashwrite_u8(f, GRAPH_VERSION);
- hashwrite_u8(f, GRAPH_OID_VERSION);
+ hashwrite_u8(f, oid_version());
hashwrite_u8(f, num_chunks);
hashwrite_u8(f, 0); /* unused padding byte */
@@ -846,8 +847,8 @@ void write_commit_graph(const char *obj_dir,
chunk_offsets[0] = 8 + (num_chunks + 1) * GRAPH_CHUNKLOOKUP_WIDTH;
chunk_offsets[1] = chunk_offsets[0] + GRAPH_FANOUT_SIZE;
- chunk_offsets[2] = chunk_offsets[1] + GRAPH_OID_LEN * commits.nr;
- chunk_offsets[3] = chunk_offsets[2] + (GRAPH_OID_LEN + 16) * commits.nr;
+ chunk_offsets[2] = chunk_offsets[1] + hashsz * commits.nr;
+ chunk_offsets[3] = chunk_offsets[2] + (hashsz + 16) * commits.nr;
chunk_offsets[4] = chunk_offsets[3] + 4 * num_extra_edges;
for (i = 0; i <= num_chunks; i++) {
@@ -860,8 +861,8 @@ void write_commit_graph(const char *obj_dir,
}
write_graph_chunk_fanout(f, commits.list, commits.nr);
- write_graph_chunk_oids(f, GRAPH_OID_LEN, commits.list, commits.nr);
- write_graph_chunk_data(f, GRAPH_OID_LEN, commits.list, commits.nr);
+ write_graph_chunk_oids(f, hashsz, commits.list, commits.nr);
+ write_graph_chunk_data(f, hashsz, commits.list, commits.nr);
write_graph_chunk_large_edges(f, commits.list, commits.nr);
close_commit_graph();
[PATCH v2 05/13] t: add basic tests for our SHA-1 implementation
We have in the past had some unfortunate endianness issues with some
SHA-1 implementations we ship, especially on big-endian machines. Add
an explicit test using the test helper to catch these issues and point
them out prominently. This test can also be used as a staging ground
for people testing additional algorithms to verify that their
implementations are working as expected.
Signed-off-by: brian m. carlson
---
t/t0014-hash.sh | 29 +
1 file changed, 29 insertions(+)
create mode 100755 t/t0014-hash.sh
diff --git a/t/t0014-hash.sh b/t/t0014-hash.sh
new file mode 100755
index 00..8e763c2c3d
--- /dev/null
+++ b/t/t0014-hash.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='test basic hash implementation'
+. ./test-lib.sh
+
+
+test_expect_success 'test basic SHA-1 hash values' '
+ test-tool sha1 actual &&
+ grep da39a3ee5e6b4b0d3255bfef95601890afd80709 actual &&
+ printf "a" | test-tool sha1 >actual &&
+ grep 86f7e437faa5a7fce15d1ddcb9eaeaea377667b8 actual &&
+ printf "abc" | test-tool sha1 >actual &&
+ grep a9993e364706816aba3e25717850c26c9cd0d89d actual &&
+ printf "message digest" | test-tool sha1 >actual &&
+ grep c12252ceda8be8994d5fa0290a47231c1d16aae3 actual &&
+ printf "abcdefghijklmnopqrstuvwxyz" | test-tool sha1 >actual &&
+ grep 32d10c7b8cf96570ca04ce37f2a19d84240d3a89 actual &&
+ perl -E "for (1..10) { print q{aa}; }" | \
+ test-tool sha1 >actual &&
+ grep 34aa973cd4c4daa4f61eeb2bdbad27316534016f actual &&
+ printf "blob 0\0" | test-tool sha1 >actual &&
+ grep e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 actual &&
+ printf "blob 3\0abc" | test-tool sha1 >actual &&
+ grep f2ba8f84ab5c1bce84a7b441cb1959cfc7093b7f actual &&
+ printf "tree 0\0" | test-tool sha1 >actual &&
+ grep 4b825dc642cb6eb9a060e54bf8d69288fbee4904 actual
+'
+
+test_done
[PATCH v2 01/13] sha1-file: rename algorithm to "sha1"
The transition plan anticipates us using a syntax such as "^{sha1}" for
disambiguation. Since this is a syntax some people will be typing a
lot, it makes sense to provide a short, easy-to-type syntax. Omitting
the dash doesn't create any ambiguity, but it does make it shorter and
easier to type, especially for touch typists. In addition, the
transition plan already uses "sha1" in this context.
Rename the name of SHA-1 implementation to "sha1".
Note that this change creates no backwards compatibility concerns, since
we haven't yet used this field in any serialized data formats.
Signed-off-by: brian m. carlson
---
sha1-file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sha1-file.c b/sha1-file.c
index a4367b8f04..e29825f259 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -97,7 +97,7 @@ const struct git_hash_algo hash_algos[GIT_HASH_NALGOS] = {
NULL,
},
{
- "sha-1",
+ "sha1",
/* "sha1", big-endian */
0x73686131,
GIT_SHA1_RAWSZ,
[PATCH v2 03/13] hex: introduce functions to print arbitrary hashes
Currently, we have functions that turn an arbitrary SHA-1 value or an
object ID into hex format, either using a static buffer or with a
user-provided buffer. Add variants of these functions that can handle
an arbitrary hash algorithm, specified by constant. Update the
documentation as well.
While we're at it, remove the "extern" declaration from this family of
functions, since it's not needed and our style now recommends against
it.
We use the variant taking the algorithm structure pointer as the
internal variant, since taking an algorithm pointer is the easiest way
to handle all of the variants in use.
Note that we maintain these functions because there are hashes which
must change based on the hash algorithm in use but are not object IDs
(such as pack checksums).
Signed-off-by: brian m. carlson
---
cache.h | 15 +--
hex.c | 37 +
2 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/cache.h b/cache.h
index d508f3d4f8..a13d14ce0a 100644
--- a/cache.h
+++ b/cache.h
@@ -1361,9 +1361,9 @@ extern int get_oid_hex(const char *hex, struct object_id
*sha1);
extern int hex_to_bytes(unsigned char *binary, const char *hex, size_t len);
/*
- * Convert a binary sha1 to its hex equivalent. The `_r` variant is reentrant,
+ * Convert a binary hash to its hex equivalent. The `_r` variant is reentrant,
* and writes the NUL-terminated output to the buffer `out`, which must be at
- * least `GIT_SHA1_HEXSZ + 1` bytes, and returns a pointer to out for
+ * least `GIT_MAX_HEXSZ + 1` bytes, and returns a pointer to out for
* convenience.
*
* The non-`_r` variant returns a static buffer, but uses a ring of 4
@@ -1371,10 +1371,13 @@ extern int hex_to_bytes(unsigned char *binary, const
char *hex, size_t len);
*
* printf("%s -> %s", sha1_to_hex(one), sha1_to_hex(two));
*/
-extern char *sha1_to_hex_r(char *out, const unsigned char *sha1);
-extern char *oid_to_hex_r(char *out, const struct object_id *oid);
-extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer
result! */
-extern char *oid_to_hex(const struct object_id *oid); /* same static buffer
as sha1_to_hex */
+char *hash_to_hex_algo_r(char *buffer, const unsigned char *hash, int algo);
+char *sha1_to_hex_r(char *out, const unsigned char *sha1);
+char *oid_to_hex_r(char *out, const struct object_id *oid);
+char *hash_to_hex_algo(const unsigned char *hash, int algo); /* static
buffer result! */
+char *sha1_to_hex(const unsigned char *sha1); /* same static
buffer */
+char *hash_to_hex(const unsigned char *hash); /* same static
buffer */
+char *oid_to_hex(const struct object_id *oid); /* same static
buffer */
/*
* Parse a 40-character hexadecimal object ID starting from hex, updating the
diff --git a/hex.c b/hex.c
index 10af1a29e8..080597ad3f 100644
--- a/hex.c
+++ b/hex.c
@@ -73,14 +73,15 @@ int parse_oid_hex(const char *hex, struct object_id *oid,
const char **end)
return ret;
}
-char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
+static inline char *hash_to_hex_algop_r(char *buffer, const unsigned char
*hash,
+ const struct git_hash_algo *algop)
{
static const char hex[] = "0123456789abcdef";
char *buf = buffer;
int i;
- for (i = 0; i < the_hash_algo->rawsz; i++) {
- unsigned int val = *sha1++;
+ for (i = 0; i < algop->rawsz; i++) {
+ unsigned int val = *hash++;
*buf++ = hex[val >> 4];
*buf++ = hex[val & 0xf];
}
@@ -89,20 +90,40 @@ char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
return buffer;
}
-char *oid_to_hex_r(char *buffer, const struct object_id *oid)
+char *hash_to_hex_algo_r(char *buffer, const unsigned char *hash, int algo)
{
- return sha1_to_hex_r(buffer, oid->hash);
+ return hash_to_hex_algop_r(buffer, hash, _algos[algo]);
}
-char *sha1_to_hex(const unsigned char *sha1)
+char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
+{
+ return hash_to_hex_algo_r(buffer, sha1, GIT_HASH_SHA1);
+}
+
+char *oid_to_hex_r(char *buffer, const struct object_id *oid)
+{
+ return hash_to_hex_algop_r(buffer, oid->hash, the_hash_algo);
+}
+
+char *hash_to_hex_algo(const unsigned char *hash, int algo)
{
static int bufno;
static char hexbuffer[4][GIT_MAX_HEXSZ + 1];
bufno = (bufno + 1) % ARRAY_SIZE(hexbuffer);
- return sha1_to_hex_r(hexbuffer[bufno], sha1);
+ return hash_to_hex_algo_r(hexbuffer[bufno], hash, algo);
+}
+
+char *sha1_to_hex(const unsigned char *sha1)
+{
+ return hash_to_hex_algo(sha1, GIT_HASH_SHA1);
+}
+
+char *hash_to_hex(const unsigned char *hash)
+{
+ return hash_to_hex_algo(hash, hash_algo_by_ptr(the_hash_algo));
}
char *oid_to_hex(const struct object_id *oid)
{
- return sha1_to_hex(oid->hash);
+ return
[PATCH v2 08/13] t/helper: add a test helper to compute hash speed
Add a utility (which is less for the testsuite and more for developers)
that can compute hash speeds for whatever hash algorithms are
implemented. This allows developers to test their personal systems to
determine the performance characteristics of various algorithms.
Signed-off-by: brian m. carlson
---
Makefile | 1 +
t/helper/test-hash-speed.c | 61 ++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
4 files changed, 64 insertions(+)
create mode 100644 t/helper/test-hash-speed.c
diff --git a/Makefile b/Makefile
index 324967410d..1c43bf9aa8 100644
--- a/Makefile
+++ b/Makefile
@@ -716,6 +716,7 @@ TEST_BUILTINS_OBJS += test-example-decorate.o
TEST_BUILTINS_OBJS += test-genrandom.o
TEST_BUILTINS_OBJS += test-hash.o
TEST_BUILTINS_OBJS += test-hashmap.o
+TEST_BUILTINS_OBJS += test-hash-speed.o
TEST_BUILTINS_OBJS += test-index-version.o
TEST_BUILTINS_OBJS += test-json-writer.o
TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
diff --git a/t/helper/test-hash-speed.c b/t/helper/test-hash-speed.c
new file mode 100644
index 00..432233c7f0
--- /dev/null
+++ b/t/helper/test-hash-speed.c
@@ -0,0 +1,61 @@
+#include "test-tool.h"
+#include "cache.h"
+
+#define NUM_SECONDS 3
+
+static inline void compute_hash(const struct git_hash_algo *algo, git_hash_ctx
*ctx, uint8_t *final, const void *p, size_t len)
+{
+ algo->init_fn(ctx);
+ algo->update_fn(ctx, p, len);
+ algo->final_fn(final, ctx);
+}
+
+int cmd__hash_speed(int ac, const char **av)
+{
+ git_hash_ctx ctx;
+ unsigned char hash[GIT_MAX_RAWSZ];
+ clock_t initial, start, end;
+ unsigned bufsizes[] = { 64, 256, 1024, 8192, 16384 };
+ int i;
+ void *p;
+ const struct git_hash_algo *algo = NULL;
+
+ if (ac == 2) {
+ for (i = 1; i < GIT_HASH_NALGOS; i++) {
+ if (!strcmp(av[1], hash_algos[i].name)) {
+ algo = _algos[i];
+ break;
+ }
+ }
+ }
+ if (!algo)
+ die("usage: test-tool hash-speed algo_name");
+
+ /* Use this as an offset to make overflow less likely. */
+ initial = clock();
+
+ printf("algo: %s\n", algo->name);
+
+ for (i = 0; i < ARRAY_SIZE(bufsizes); i++) {
+ unsigned long j, kb;
+ double kb_per_sec;
+ p = xcalloc(1, bufsizes[i]);
+ start = end = clock() - initial;
+ for (j = 0; ((end - start) / CLOCKS_PER_SEC) < NUM_SECONDS;
j++) {
+ compute_hash(algo, , hash, p, bufsizes[i]);
+
+ /*
+* Only check elapsed time every 128 iterations to avoid
+* dominating the runtime with system calls.
+*/
+ if (!(j & 127))
+ end = clock() - initial;
+ }
+ kb = j * bufsizes[i];
+ kb_per_sec = kb / (1024 * ((double)end - start) /
CLOCKS_PER_SEC);
+ printf("size %u: %lu iters; %lu KiB; %0.2f KiB/s\n",
bufsizes[i], j, kb, kb_per_sec);
+ free(p);
+ }
+
+ exit(0);
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index 6b5836dc1b..e009c8186d 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -20,6 +20,7 @@ static struct test_cmd cmds[] = {
{ "example-decorate", cmd__example_decorate },
{ "genrandom", cmd__genrandom },
{ "hashmap", cmd__hashmap },
+ { "hash-speed", cmd__hash_speed },
{ "index-version", cmd__index_version },
{ "json-writer", cmd__json_writer },
{ "lazy-init-name-hash", cmd__lazy_init_name_hash },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 29ac7b0b0d..19a7e8332a 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -16,6 +16,7 @@ int cmd__dump_untracked_cache(int argc, const char **argv);
int cmd__example_decorate(int argc, const char **argv);
int cmd__genrandom(int argc, const char **argv);
int cmd__hashmap(int argc, const char **argv);
+int cmd__hash_speed(int argc, const char **argv);
int cmd__index_version(int argc, const char **argv);
int cmd__json_writer(int argc, const char **argv);
int cmd__lazy_init_name_hash(int argc, const char **argv);
[PATCH v2 00/13] Base SHA-256 implementation
This series provides an actual SHA-256 implementation and wires it up,
along with some housekeeping patches to make it suitable for testing.
New in this version is a patch which reverts the change to limit hashcmp
to 20 bytes. I've taken care to permit the compiler to inline as much
as possible for efficiency, but it would be helpful to see what the
performance impact of these changes is on real-life workflows, or with
MSVC and other non-GCC and non-clang compilers. The resulting change is
uglier and more duplicative than I wanted, but oh, well.
I didn't attempt to pull in the full complement of code from libtomcrypt
to try to show the changes made, since that would have involved
importing a significant quantity of code in order to make things work.
I realize the increase to GIT_MAX_HEXSZ will result in an increase in
memory usage, but we're going to need to do it at some point, and the
sooner the code is in the codebase, the sooner people can play around
with it and test it.
This piece should be the final piece of preparatory work required for a
fully functioning SHA-256-only Git. Additional work should be able to
come into the testsuite and codebase without needing to build on work in
any series after this one.
v1, which was RFC, can be seen at
https://public-inbox.org/git/20180829005857.980820-1-sand...@crustytoothpaste.net/.
Changes from v1:
* Add a hash_to_hex function mirroring sha1_to_hex, but for
the_hash_algo.
* Strip commit message explanation about why we chose SHA-256.
* Rebase on master
* Strip leading whitespace from commit message.
* Improve commit-graph patch to cover new code added since v1.
* Be more honest about the scope of work involved in porting the SHA-256
implementation out of libtomcrypt.
* Revert change to limit hashcmp to 20 bytes.
brian m. carlson (13):
sha1-file: rename algorithm to "sha1"
sha1-file: provide functions to look up hash algorithms
hex: introduce functions to print arbitrary hashes
cache: make hashcmp and hasheq work with larger hashes
t: add basic tests for our SHA-1 implementation
t: make the sha1 test-tool helper generic
sha1-file: add a constant for hash block size
t/helper: add a test helper to compute hash speed
commit-graph: convert to using the_hash_algo
Add a base implementation of SHA-256 support
sha256: add an SHA-256 implementation using libgcrypt
hash: add an SHA-256 implementation using OpenSSL
commit-graph: specify OID version for SHA-256
Makefile | 22
cache.h | 51 +---
commit-graph.c| 40 +++---
hash.h| 41 +-
hex.c | 37 --
sha1-file.c | 70 +-
sha256/block/sha256.c | 180 ++
sha256/block/sha256.h | 26
sha256/gcrypt.h | 30 +
t/helper/test-hash-speed.c| 61 +
t/helper/{test-sha1.c => test-hash.c} | 19 +--
t/helper/test-sha1.c | 52 +---
t/helper/test-sha256.c| 7 +
t/helper/test-tool.c | 2 +
t/helper/test-tool.h | 4 +
t/t0014-hash.sh | 54
16 files changed, 592 insertions(+), 104 deletions(-)
create mode 100644 sha256/block/sha256.c
create mode 100644 sha256/block/sha256.h
create mode 100644 sha256/gcrypt.h
create mode 100644 t/helper/test-hash-speed.c
copy t/helper/{test-sha1.c => test-hash.c} (66%)
create mode 100644 t/helper/test-sha256.c
create mode 100755 t/t0014-hash.sh
Range-diff against v1:
1: 0a96c59452 = 1: 804ec2fd27 sha1-file: rename algorithm to "sha1"
2: 65f9feba41 = 2: 5196e00b26 sha1-file: provide functions to look up hash
algorithms
3: b6d960121d ! 3: 5873510d0a hex: introduce functions to print arbitrary
hashes
@@ -13,9 +13,12 @@
it.
We use the variant taking the algorithm structure pointer as the
-internal variant, since in the future we'll want to replace sha1_to_hex
-with a hash_to_hex that handles the_hash_algo, and taking an algorithm
-pointer is the easiest way to handle all of the variants in use.
+internal variant, since taking an algorithm pointer is the easiest way
+to handle all of the variants in use.
+
+Note that we maintain these functions because there are hashes which
+must change based on the hash algorithm in use but are not object IDs
+(such as pack checksums).
Signed-off-by: brian m. carlson
@@ -47,6 +50,7 @@
+char *oid_to_hex_r(char *out, const struct object_id *oid);
+char *hash_to_hex_algo(const unsigned char *hash, int algo); /*
static buffer result! */
+char *sha1_to_hex(const unsigned char *sha1); /* same
static buffer */
++char *hash_to_hex(const
Re: [PATCH v2 1/1] zlib.c: use size_t for size
On 15/10/18 01:01, Jeff King wrote:
> On Sun, Oct 14, 2018 at 04:03:48PM +0100, Ramsay Jones wrote:
>
>>> So I kind of wonder if a comment would be better than xsize_t here.
>>> Something like:
>>>
>>> if (avail > len) {
>>> /*
>>> * This can never truncate because we know that len is smaller
>>> * than avail, which is already a size_t.
>>> */
>>> avail = (size_t)len;
>>> }
>>
>> Heh, you are, of course, correct! (that will learn me[1]). :-D
>>
>> Hmm, let's see if I can muster the enthusiasm to do all that
>> testing again!
>
> For the record, I am not opposed to including the comment _and_ using
> xsize_t() to do the cast, giving us an assertion that the comment is
> correct.
Heh, I haven't found any enthusiasm tonight. Let's see if there
are any more comments/opinions.
Thanks.
ATB,
Ramsay Jones
Re: [RFC/PATCH] headers: normalize the spelling of some header guards
On 15/10/18 00:59, Jeff King wrote: > On Sun, Oct 14, 2018 at 09:13:09PM +0100, Ramsay Jones wrote: > >> This patch is marked RFC because I am not aware of any policy with >> regard to header guard spelling. Having said that, apart from the >> fetch-negotiator.h header, all of these headers are using a reserved >> identifier (see C99 Standard 7.1.3). >> >> These headers were found, thus: >> >> $ git grep -n -E '^#ifn?def ' -- '*.h' | grep 'h\:1\:' | grep -v '^compat' >> | grep -v -E '[A-Z_]*_H$' >> alias.h:1:#ifndef __ALIAS_H__ >> commit-reach.h:1:#ifndef __COMMIT_REACH_H__ >> fetch-negotiator.h:1:#ifndef FETCH_NEGOTIATOR >> midx.h:1:#ifndef __MIDX_H__ >> t/helper/test-tool.h:1:#ifndef __TEST_TOOL_H__ >> vcs-svn/fast_export.h:1:#ifndef FAST_EXPORT_H_ >> vcs-svn/line_buffer.h:1:#ifndef LINE_BUFFER_H_ >> vcs-svn/sliding_window.h:1:#ifndef SLIDING_WINDOW_H_ >> vcs-svn/svndiff.h:1:#ifndef SVNDIFF_H_ >> vcs-svn/svndump.h:1:#ifndef SVNDUMP_H_ > > I think the ones with a trailing underscore are actually OK according to > the standard (not sure if your "all of these" was including the ones in > vcs-svn ;) ). Yes, trailing underscore is fine - "all of these" meant the headers in the patch, with the noted exception of fetch-negotiator.h. > I'm in favor of normalizing even the ones that aren't illegal, though > I'm OK either way on the vcs-svn bits if they're going away anyway. I wasn't sure about vcs-svn, but assumed that they might go away soon (hence the cc: list). I will be happy to add them to the patch, if that is not the case. Thanks. ATB, Ramsay Jones
[PATCH v2 13/15] apply: rename new_sha1_prefix and old_sha1_prefix
Rename these structure members to "new_oid_prefix" and "old_oid_prefix".
Signed-off-by: brian m. carlson
---
apply.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/apply.c b/apply.c
index 792ecea36a..b9eb02ec12 100644
--- a/apply.c
+++ b/apply.c
@@ -223,8 +223,8 @@ struct patch {
struct fragment *fragments;
char *result;
size_t resultsize;
- char old_sha1_prefix[GIT_MAX_HEXSZ + 1];
- char new_sha1_prefix[GIT_MAX_HEXSZ + 1];
+ char old_oid_prefix[GIT_MAX_HEXSZ + 1];
+ char new_oid_prefix[GIT_MAX_HEXSZ + 1];
struct patch *next;
/* three-way fallback result */
@@ -1099,8 +1099,8 @@ static int gitdiff_index(struct apply_state *state,
if (!ptr || ptr[1] != '.' || hexsz < ptr - line)
return 0;
len = ptr - line;
- memcpy(patch->old_sha1_prefix, line, len);
- patch->old_sha1_prefix[len] = 0;
+ memcpy(patch->old_oid_prefix, line, len);
+ patch->old_oid_prefix[len] = 0;
line = ptr + 2;
ptr = strchr(line, ' ');
@@ -1112,8 +1112,8 @@ static int gitdiff_index(struct apply_state *state,
if (hexsz < len)
return 0;
- memcpy(patch->new_sha1_prefix, line, len);
- patch->new_sha1_prefix[len] = 0;
+ memcpy(patch->new_oid_prefix, line, len);
+ patch->new_oid_prefix[len] = 0;
if (*ptr == ' ')
return gitdiff_oldmode(state, ptr + 1, patch);
return 0;
@@ -2205,7 +2205,7 @@ static void reverse_patches(struct patch *p)
SWAP(p->new_mode, p->old_mode);
SWAP(p->is_new, p->is_delete);
SWAP(p->lines_added, p->lines_deleted);
- SWAP(p->old_sha1_prefix, p->new_sha1_prefix);
+ SWAP(p->old_oid_prefix, p->new_oid_prefix);
for (; frag; frag = frag->next) {
SWAP(frag->newpos, frag->oldpos);
@@ -3149,10 +3149,10 @@ static int apply_binary(struct apply_state *state,
* For safety, we require patch index line to contain
* full hex textual object ID for old and new, at least for now.
*/
- if (strlen(patch->old_sha1_prefix) != hexsz ||
- strlen(patch->new_sha1_prefix) != hexsz ||
- get_oid_hex(patch->old_sha1_prefix, ) ||
- get_oid_hex(patch->new_sha1_prefix, ))
+ if (strlen(patch->old_oid_prefix) != hexsz ||
+ strlen(patch->new_oid_prefix) != hexsz ||
+ get_oid_hex(patch->old_oid_prefix, ) ||
+ get_oid_hex(patch->new_oid_prefix, ))
return error(_("cannot apply binary patch to '%s' "
"without full index line"), name);
@@ -3162,7 +3162,7 @@ static int apply_binary(struct apply_state *state,
* applies to.
*/
hash_object_file(img->buf, img->len, blob_type, );
- if (strcmp(oid_to_hex(), patch->old_sha1_prefix))
+ if (strcmp(oid_to_hex(), patch->old_oid_prefix))
return error(_("the patch applies to '%s' (%s), "
"which does not match the "
"current contents."),
@@ -3175,7 +3175,7 @@ static int apply_binary(struct apply_state *state,
"'%s' but it is not empty"), name);
}
- get_oid_hex(patch->new_sha1_prefix, );
+ get_oid_hex(patch->new_oid_prefix, );
if (is_null_oid()) {
clear_image(img);
return 0; /* deletion patch */
@@ -3191,7 +3191,7 @@ static int apply_binary(struct apply_state *state,
if (!result)
return error(_("the necessary postimage %s for "
"'%s' cannot be read"),
-patch->new_sha1_prefix, name);
+patch->new_oid_prefix, name);
clear_image(img);
img->buf = result;
img->len = size;
@@ -3207,9 +3207,9 @@ static int apply_binary(struct apply_state *state,
/* verify that the result matches */
hash_object_file(img->buf, img->len, blob_type, );
- if (strcmp(oid_to_hex(), patch->new_sha1_prefix))
+ if (strcmp(oid_to_hex(), patch->new_oid_prefix))
return error(_("binary patch to '%s' creates incorrect
result (expecting %s, got %s)"),
- name, patch->new_sha1_prefix, oid_to_hex());
+ name, patch->new_oid_prefix, oid_to_hex());
}
return 0;
@@ -3565,7 +3565,7 @@ static int try_threeway(struct apply_state *state,
/* Preimage the patch was prepared for */
if (patch->is_new)
write_object_file("", 0, blob_type, _oid);
- else if
[PATCH v2 01/15] object_id.cocci: match only expressions of type 'struct object_id'
From: SZEDER Gábor
Most of our semantic patches in 'contrib/coccinelle/object_id.cocci'
turn calls of SHA1-specific functions into calls of their
corresponding object_id counterparts, e.g. sha1_to_hex() to
oid_to_hex(). These semantic patches look something like this:
@@
expression E1;
@@
- sha1_to_hex(E1.hash)
+ oid_to_hex()
and match the access to the 'hash' field in any data type, not only in
'struct object_id', and, consquently, can produce wrong
transformations.
Case in point is the recent hash function transition patch "rerere:
convert to use the_hash_algo" [1], which, among other things, renamed
'struct rerere_dir's 'sha1' field to 'hash', and then 'make
coccicheck' started to suggest the following wrong transformations for
'rerere.c' [2]:
-return sha1_to_hex(id->collection->hash);
+return oid_to_hex(id->collection);
and
-DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->hash)));
+DIR *dir = opendir(git_path("rr-cache/%s", oid_to_hex(rr_dir)));
Avoid such wrong transformations by tightening semantic patches in
'object_id.cocci' to match only type of or pointers to 'struct
object_id'.
[1]
https://public-inbox.org/git/20181008215701.779099-15-sand...@crustytoothpaste.net/
[2] https://travis-ci.org/git/git/jobs/440463476#L580
Signed-off-by: SZEDER Gábor
Signed-off-by: brian m. carlson
---
contrib/coccinelle/object_id.cocci | 117 -
1 file changed, 63 insertions(+), 54 deletions(-)
diff --git a/contrib/coccinelle/object_id.cocci
b/contrib/coccinelle/object_id.cocci
index d8bdb48712..6a7cf3e02d 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -1,119 +1,127 @@
@@
-expression E1;
+struct object_id OID;
@@
-- is_null_sha1(E1.hash)
-+ is_null_oid()
+- is_null_sha1(OID.hash)
++ is_null_oid()
@@
-expression E1;
+struct object_id *OIDPTR;
@@
-- is_null_sha1(E1->hash)
-+ is_null_oid(E1)
+- is_null_sha1(OIDPTR->hash)
++ is_null_oid(OIDPTR)
@@
-expression E1;
+struct object_id OID;
@@
-- sha1_to_hex(E1.hash)
-+ oid_to_hex()
+- sha1_to_hex(OID.hash)
++ oid_to_hex()
@@
identifier f != oid_to_hex;
-expression E1;
+struct object_id *OIDPTR;
@@
f(...) {<...
-- sha1_to_hex(E1->hash)
-+ oid_to_hex(E1)
+- sha1_to_hex(OIDPTR->hash)
++ oid_to_hex(OIDPTR)
...>}
@@
-expression E1, E2;
+expression E;
+struct object_id OID;
@@
-- sha1_to_hex_r(E1, E2.hash)
-+ oid_to_hex_r(E1, )
+- sha1_to_hex_r(E, OID.hash)
++ oid_to_hex_r(E, )
@@
identifier f != oid_to_hex_r;
-expression E1, E2;
+expression E;
+struct object_id *OIDPTR;
@@
f(...) {<...
-- sha1_to_hex_r(E1, E2->hash)
-+ oid_to_hex_r(E1, E2)
+- sha1_to_hex_r(E, OIDPTR->hash)
++ oid_to_hex_r(E, OIDPTR)
...>}
@@
-expression E1;
+struct object_id OID;
@@
-- hashclr(E1.hash)
-+ oidclr()
+- hashclr(OID.hash)
++ oidclr()
@@
identifier f != oidclr;
-expression E1;
+struct object_id *OIDPTR;
@@
f(...) {<...
-- hashclr(E1->hash)
-+ oidclr(E1)
+- hashclr(OIDPTR->hash)
++ oidclr(OIDPTR)
...>}
@@
-expression E1, E2;
+struct object_id OID1, OID2;
@@
-- hashcmp(E1.hash, E2.hash)
-+ oidcmp(, )
+- hashcmp(OID1.hash, OID2.hash)
++ oidcmp(, )
@@
identifier f != oidcmp;
-expression E1, E2;
+struct object_id *OIDPTR1, OIDPTR2;
@@
f(...) {<...
-- hashcmp(E1->hash, E2->hash)
-+ oidcmp(E1, E2)
+- hashcmp(OIDPTR1->hash, OIDPTR2->hash)
++ oidcmp(OIDPTR1, OIDPTR2)
...>}
@@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
@@
-- hashcmp(E1->hash, E2.hash)
-+ oidcmp(E1, )
+- hashcmp(OIDPTR->hash, OID.hash)
++ oidcmp(OIDPTR, )
@@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
@@
-- hashcmp(E1.hash, E2->hash)
-+ oidcmp(, E2)
+- hashcmp(OID.hash, OIDPTR->hash)
++ oidcmp(, OIDPTR)
@@
-expression E1, E2;
+struct object_id OID1, OID2;
@@
-- hashcpy(E1.hash, E2.hash)
-+ oidcpy(, )
+- hashcpy(OID1.hash, OID2.hash)
++ oidcpy(, )
@@
identifier f != oidcpy;
-expression E1, E2;
+struct object_id *OIDPTR1;
+struct object_id *OIDPTR2;
@@
f(...) {<...
-- hashcpy(E1->hash, E2->hash)
-+ oidcpy(E1, E2)
+- hashcpy(OIDPTR1->hash, OIDPTR2->hash)
++ oidcpy(OIDPTR1, OIDPTR2)
...>}
@@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
@@
-- hashcpy(E1->hash, E2.hash)
-+ oidcpy(E1, )
+- hashcpy(OIDPTR->hash, OID.hash)
++ oidcpy(OIDPTR, )
@@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
@@
-- hashcpy(E1.hash, E2->hash)
-+ oidcpy(, E2)
+- hashcpy(OID.hash, OIDPTR->hash)
++ oidcpy(, OIDPTR)
@@
-expression E1, E2;
+struct object_id *OIDPTR1;
+struct object_id *OIDPTR2;
@@
-- oidcmp(E1, E2) == 0
-+ oideq(E1, E2)
+- oidcmp(OIDPTR1, OIDPTR2) == 0
++ oideq(OIDPTR1, OIDPTR2)
@@
identifier f != hasheq;
@@ -125,10 +133,11 @@ expression E1, E2;
...>}
@@
-expression E1, E2;
+struct object_id *OIDPTR1;
+struct object_id *OIDPTR2;
@@
-- oidcmp(E1, E2) != 0
-+ !oideq(E1, E2)
+- oidcmp(OIDPTR1, OIDPTR2) != 0
++
[PATCH v2 15/15] rerere: convert to use the_hash_algo
Since this data is stored in the .git directory, it makes sense for us
to use the same hash algorithm for it as for everything else. Convert
the remaining uses of SHA-1 to use the_hash_algo. Use GIT_MAX_RAWSZ for
allocations. Rename various struct members, local variables, and a
function to be named "hash" instead of "sha1".
Signed-off-by: brian m. carlson
---
rerere.c | 81 +---
1 file changed, 42 insertions(+), 39 deletions(-)
diff --git a/rerere.c b/rerere.c
index 7aa149e849..ceb98015ff 100644
--- a/rerere.c
+++ b/rerere.c
@@ -29,7 +29,7 @@ static int rerere_dir_alloc;
#define RR_HAS_POSTIMAGE 1
#define RR_HAS_PREIMAGE 2
static struct rerere_dir {
- unsigned char sha1[20];
+ unsigned char hash[GIT_MAX_HEXSZ];
int status_alloc, status_nr;
unsigned char *status;
} **rerere_dir;
@@ -52,7 +52,7 @@ static void free_rerere_id(struct string_list_item *item)
static const char *rerere_id_hex(const struct rerere_id *id)
{
- return sha1_to_hex(id->collection->sha1);
+ return sha1_to_hex(id->collection->hash);
}
static void fit_variant(struct rerere_dir *rr_dir, int variant)
@@ -115,7 +115,7 @@ static int is_rr_file(const char *name, const char
*filename, int *variant)
static void scan_rerere_dir(struct rerere_dir *rr_dir)
{
struct dirent *de;
- DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->sha1)));
+ DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->hash)));
if (!dir)
return;
@@ -133,24 +133,24 @@ static void scan_rerere_dir(struct rerere_dir *rr_dir)
closedir(dir);
}
-static const unsigned char *rerere_dir_sha1(size_t i, void *table)
+static const unsigned char *rerere_dir_hash(size_t i, void *table)
{
struct rerere_dir **rr_dir = table;
- return rr_dir[i]->sha1;
+ return rr_dir[i]->hash;
}
static struct rerere_dir *find_rerere_dir(const char *hex)
{
- unsigned char sha1[20];
+ unsigned char hash[GIT_MAX_RAWSZ];
struct rerere_dir *rr_dir;
int pos;
- if (get_sha1_hex(hex, sha1))
+ if (get_sha1_hex(hex, hash))
return NULL; /* BUG */
- pos = sha1_pos(sha1, rerere_dir, rerere_dir_nr, rerere_dir_sha1);
+ pos = sha1_pos(hash, rerere_dir, rerere_dir_nr, rerere_dir_hash);
if (pos < 0) {
rr_dir = xmalloc(sizeof(*rr_dir));
- hashcpy(rr_dir->sha1, sha1);
+ hashcpy(rr_dir->hash, hash);
rr_dir->status = NULL;
rr_dir->status_nr = 0;
rr_dir->status_alloc = 0;
@@ -207,26 +207,27 @@ static void read_rr(struct string_list *rr)
return;
while (!strbuf_getwholeline(, in, '\0')) {
char *path;
- unsigned char sha1[20];
+ unsigned char hash[GIT_MAX_RAWSZ];
struct rerere_id *id;
int variant;
+ const unsigned hexsz = the_hash_algo->hexsz;
/* There has to be the hash, tab, path and then NUL */
- if (buf.len < 42 || get_sha1_hex(buf.buf, sha1))
+ if (buf.len < hexsz + 2 || get_sha1_hex(buf.buf, hash))
die(_("corrupt MERGE_RR"));
- if (buf.buf[40] != '.') {
+ if (buf.buf[hexsz] != '.') {
variant = 0;
- path = buf.buf + 40;
+ path = buf.buf + hexsz;
} else {
errno = 0;
- variant = strtol(buf.buf + 41, , 10);
+ variant = strtol(buf.buf + hexsz + 1, , 10);
if (errno)
die(_("corrupt MERGE_RR"));
}
if (*(path++) != '\t')
die(_("corrupt MERGE_RR"));
- buf.buf[40] = '\0';
+ buf.buf[hexsz] = '\0';
id = new_rerere_id_hex(buf.buf);
id->variant = variant;
string_list_insert(rr, path)->util = id;
@@ -360,7 +361,7 @@ static void rerere_strbuf_putconflict(struct strbuf *buf,
int ch, size_t size)
}
static int handle_conflict(struct strbuf *out, struct rerere_io *io,
- int marker_size, git_SHA_CTX *ctx)
+ int marker_size, git_hash_ctx *ctx)
{
enum {
RR_SIDE_1 = 0, RR_SIDE_2, RR_ORIGINAL
@@ -398,10 +399,12 @@ static int handle_conflict(struct strbuf *out, struct
rerere_io *io,
strbuf_addbuf(out, );
rerere_strbuf_putconflict(out, '>', marker_size);
if (ctx) {
- git_SHA1_Update(ctx, one.buf ? one.buf : "",
- one.len + 1);
- git_SHA1_Update(ctx, two.buf ? two.buf : "",
-
[PATCH v2 14/15] submodule: make zero-oid comparison hash function agnostic
With SHA-256, the length of the all-zeros object ID is longer. Add a
function to git-submodule.sh to check if a full hex object ID is the
all-zeros value, and use it to check the output we're parsing from git
diff-files or diff-index.
Signed-off-by: brian m. carlson
---
git-submodule.sh | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 1b568e29b9..c09eb3e03d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -82,6 +82,11 @@ isnumber()
n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
}
+# Given a full hex object ID, is this the zero OID?
+is_zero_oid () {
+ echo "$1" | sane_egrep '^0+$' >/dev/null 2>&1
+}
+
# Sanitize the local git environment for use within a submodule. We
# can't simply use clear_local_git_env since we want to preserve some
# of the settings from GIT_CONFIG_PARAMETERS.
@@ -780,7 +785,7 @@ cmd_summary() {
while read -r mod_src mod_dst sha1_src sha1_dst status name
do
if test -z "$cached" &&
- test $sha1_dst =
+ is_zero_oid $sha1_dst
then
case "$mod_dst" in
16)
[PATCH v2 02/15] pack-bitmap-write: use GIT_MAX_RAWSZ for allocation
Reviewed-by: Stefan Beller
Signed-off-by: brian m. carlson
---
pack-bitmap-write.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index fc82f37a02..6f0c78d6aa 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -37,7 +37,7 @@ struct bitmap_writer {
struct progress *progress;
int show_progress;
- unsigned char pack_checksum[20];
+ unsigned char pack_checksum[GIT_MAX_RAWSZ];
};
static struct bitmap_writer writer;
[PATCH v2 07/15] packfile: express constants in terms of the_hash_algo
Replace uses of GIT_SHA1_RAWSZ with references to the_hash_algo to avoid
dependence on a particular hash length.
It's likely that in the future, we'll update the pack format to indicate
what hash algorithm it uses, and then this code will change. However,
at least on an interim basis, make it easier to develop on a pure
SHA-256 Git by using the_hash_algo here.
Signed-off-by: brian m. carlson
---
packfile.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/packfile.c b/packfile.c
index 841b36182f..17f993b5bf 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1121,13 +1121,14 @@ int unpack_object_header(struct packed_git *p,
void mark_bad_packed_object(struct packed_git *p, const unsigned char *sha1)
{
unsigned i;
+ const unsigned hashsz = the_hash_algo->rawsz;
for (i = 0; i < p->num_bad_objects; i++)
- if (hasheq(sha1, p->bad_object_sha1 + GIT_SHA1_RAWSZ * i))
+ if (hasheq(sha1, p->bad_object_sha1 + hashsz * i))
return;
p->bad_object_sha1 = xrealloc(p->bad_object_sha1,
st_mult(GIT_MAX_RAWSZ,
st_add(p->num_bad_objects, 1)));
- hashcpy(p->bad_object_sha1 + GIT_SHA1_RAWSZ * p->num_bad_objects, sha1);
+ hashcpy(p->bad_object_sha1 + hashsz * p->num_bad_objects, sha1);
p->num_bad_objects++;
}
[PATCH v2 05/15] builtin/fetch-pack: remove constants with parse_oid_hex
Instead of using GIT_SHA1_HEXSZ, use parse_oid_hex to compute a pointer
and use that in comparisons. This is both simpler to read and works
independent of the hash length. Update references to SHA-1 in the same
function to refer to object IDs instead.
Signed-off-by: brian m. carlson
---
builtin/fetch-pack.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 1a1bc63566..63e69a5801 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -16,13 +16,14 @@ static void add_sought_entry(struct ref ***sought, int *nr,
int *alloc,
{
struct ref *ref;
struct object_id oid;
+ const char *p;
- if (!get_oid_hex(name, )) {
- if (name[GIT_SHA1_HEXSZ] == ' ') {
- /* , find refname */
- name += GIT_SHA1_HEXSZ + 1;
- } else if (name[GIT_SHA1_HEXSZ] == '\0') {
- ; /* , leave sha1 as name */
+ if (!parse_oid_hex(name, , )) {
+ if (*p == ' ') {
+ /* , find refname */
+ name = p + 1;
+ } else if (*p == '\0') {
+ ; /* , leave oid as name */
} else {
/* , clear cruft from oid */
oidclr();
[PATCH v2 11/15] tag: express constant in terms of the_hash_algo
Signed-off-by: brian m. carlson
---
tag.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tag.c b/tag.c
index 1db663d716..7445b8f6ea 100644
--- a/tag.c
+++ b/tag.c
@@ -144,7 +144,7 @@ int parse_tag_buffer(struct repository *r, struct tag
*item, const void *data, u
return 0;
item->object.parsed = 1;
- if (size < GIT_SHA1_HEXSZ + 24)
+ if (size < the_hash_algo->hexsz + 24)
return -1;
if (memcmp("object ", bufptr, 7) || parse_oid_hex(bufptr + 7, ,
) || *bufptr++ != '\n')
return -1;
[PATCH v2 12/15] apply: replace hard-coded constants
Replace several 40-based constants with references to GIT_MAX_HEXSZ or
the_hash_algo, as appropriate.
Signed-off-by: brian m. carlson
---
apply.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/apply.c b/apply.c
index e485fbc6bc..792ecea36a 100644
--- a/apply.c
+++ b/apply.c
@@ -223,8 +223,8 @@ struct patch {
struct fragment *fragments;
char *result;
size_t resultsize;
- char old_sha1_prefix[41];
- char new_sha1_prefix[41];
+ char old_sha1_prefix[GIT_MAX_HEXSZ + 1];
+ char new_sha1_prefix[GIT_MAX_HEXSZ + 1];
struct patch *next;
/* three-way fallback result */
@@ -1093,9 +1093,10 @@ static int gitdiff_index(struct apply_state *state,
*/
const char *ptr, *eol;
int len;
+ const unsigned hexsz = the_hash_algo->hexsz;
ptr = strchr(line, '.');
- if (!ptr || ptr[1] != '.' || 40 < ptr - line)
+ if (!ptr || ptr[1] != '.' || hexsz < ptr - line)
return 0;
len = ptr - line;
memcpy(patch->old_sha1_prefix, line, len);
@@ -1109,7 +1110,7 @@ static int gitdiff_index(struct apply_state *state,
ptr = eol;
len = ptr - line;
- if (40 < len)
+ if (hexsz < len)
return 0;
memcpy(patch->new_sha1_prefix, line, len);
patch->new_sha1_prefix[len] = 0;
@@ -3142,13 +3143,14 @@ static int apply_binary(struct apply_state *state,
{
const char *name = patch->old_name ? patch->old_name : patch->new_name;
struct object_id oid;
+ const unsigned hexsz = the_hash_algo->hexsz;
/*
* For safety, we require patch index line to contain
-* full 40-byte textual SHA1 for old and new, at least for now.
+* full hex textual object ID for old and new, at least for now.
*/
- if (strlen(patch->old_sha1_prefix) != 40 ||
- strlen(patch->new_sha1_prefix) != 40 ||
+ if (strlen(patch->old_sha1_prefix) != hexsz ||
+ strlen(patch->new_sha1_prefix) != hexsz ||
get_oid_hex(patch->old_sha1_prefix, ) ||
get_oid_hex(patch->new_sha1_prefix, ))
return error(_("cannot apply binary patch to '%s' "
@@ -4055,7 +4057,7 @@ static int preimage_oid_in_gitlink_patch(struct patch *p,
struct object_id *oid)
starts_with(++preimage, heading) &&
/* does it record full SHA-1? */
!get_oid_hex(preimage + sizeof(heading) - 1, oid) &&
- preimage[sizeof(heading) + GIT_SHA1_HEXSZ - 1] == '\n' &&
+ preimage[sizeof(heading) + the_hash_algo->hexsz - 1] == '\n' &&
/* does the abbreviated name on the index line agree with it? */
starts_with(preimage + sizeof(heading) - 1, p->old_sha1_prefix))
return 0; /* it all looks fine */
[PATCH v2 10/15] transport: use parse_oid_hex instead of a constant
Use parse_oid_hex to compute a pointer instead of using GIT_SHA1_HEXSZ.
This simplifies the code and makes it independent of the hash length.
Signed-off-by: brian m. carlson
---
transport.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/transport.c b/transport.c
index 1c76d64aba..44b9ddf670 100644
--- a/transport.c
+++ b/transport.c
@@ -1346,15 +1346,16 @@ static void read_alternate_refs(const char *path,
fh = xfdopen(cmd.out, "r");
while (strbuf_getline_lf(, fh) != EOF) {
struct object_id oid;
+ const char *p;
- if (get_oid_hex(line.buf, ) ||
- line.buf[GIT_SHA1_HEXSZ] != ' ') {
+ if (parse_oid_hex(line.buf, , ) ||
+ *p != ' ') {
warning(_("invalid line while parsing alternate refs:
%s"),
line.buf);
break;
}
- cb(line.buf + GIT_SHA1_HEXSZ + 1, , data);
+ cb(p + 1, , data);
}
fclose(fh);
[PATCH v2 00/15] Hash function transition part 15
This is the fifteenth series in the ongoing hash function transition. This series includes several conversions to use the_hash_algo, combined with some use of parse_oid_hex and GIT_MAX_RAWSZ. Changes from v1: * Fix several other substantially similar issues in builtin/repack. * Fix a comment style issue. * Improve commit message as suggested by Stefan. * Pull in Gábor's patch and place it at the beginning of the series. SZEDER Gábor (1): object_id.cocci: match only expressions of type 'struct object_id' brian m. carlson (14): pack-bitmap-write: use GIT_MAX_RAWSZ for allocation builtin/repack: replace hard-coded constants builtin/mktree: remove hard-coded constant builtin/fetch-pack: remove constants with parse_oid_hex pack-revindex: express constants in terms of the_hash_algo packfile: express constants in terms of the_hash_algo refs/packed-backend: express constants using the_hash_algo upload-pack: express constants in terms of the_hash_algo transport: use parse_oid_hex instead of a constant tag: express constant in terms of the_hash_algo apply: replace hard-coded constants apply: rename new_sha1_prefix and old_sha1_prefix submodule: make zero-oid comparison hash function agnostic rerere: convert to use the_hash_algo apply.c| 50 ++-- builtin/fetch-pack.c | 13 ++-- builtin/mktree.c | 2 +- builtin/repack.c | 13 ++-- contrib/coccinelle/object_id.cocci | 117 - git-submodule.sh | 7 +- pack-bitmap-write.c| 2 +- pack-revindex.c| 10 ++- packfile.c | 5 +- refs/packed-backend.c | 14 ++-- rerere.c | 81 ++-- tag.c | 2 +- transport.c| 7 +- upload-pack.c | 13 ++-- 14 files changed, 181 insertions(+), 155 deletions(-) Range-diff against v1: -: -- > 1: 35d9cefd9a object_id.cocci: match only expressions of type 'struct object_id' 1: 223d6f6695 ! 2: bb2a15a6e9 pack-bitmap-write: use GIT_MAX_RAWSZ for allocation @@ -2,6 +2,7 @@ pack-bitmap-write: use GIT_MAX_RAWSZ for allocation +Reviewed-by: Stefan Beller Signed-off-by: brian m. carlson diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c 2: bbdb147d8d < -: -- builtin/repack: replace hard-coded constant -: -- > 3: 95bdac161e builtin/repack: replace hard-coded constants 3: bf7b55fe4e = 4: 2a28675cfc builtin/mktree: remove hard-coded constant 4: 155451f608 = 5: 7941bb0060 builtin/fetch-pack: remove constants with parse_oid_hex 5: 9ec7065d62 ! 6: 2ec9a22ea7 pack-revindex: express constants in terms of the_hash_algo @@ -3,7 +3,9 @@ pack-revindex: express constants in terms of the_hash_algo Express the various constants used in terms of the_hash_algo. +While we're at it, fix a comment style issue as well. +Reviewed-by: Stefan Beller Signed-off-by: brian m. carlson diff --git a/pack-revindex.c b/pack-revindex.c @@ -37,7 +39,8 @@ } - /* This knows the pack format -- the 20-byte trailer -+ /* This knows the pack format -- the hash trailer ++ /* ++ * This knows the pack format -- the hash trailer * follows immediately after the last object data. */ - p->revindex[num_ent].offset = p->pack_size - 20; 6: 5e8576c6e4 ! 7: 3ccb2b7217 packfile: express constants in terms of the_hash_algo @@ -5,6 +5,11 @@ Replace uses of GIT_SHA1_RAWSZ with references to the_hash_algo to avoid dependence on a particular hash length. +It's likely that in the future, we'll update the pack format to indicate +what hash algorithm it uses, and then this code will change. However, +at least on an interim basis, make it easier to develop on a pure +SHA-256 Git by using the_hash_algo here. + Signed-off-by: brian m. carlson diff --git a/packfile.c b/packfile.c 7: 4c7b6471db = 8: 39eb7e1069 refs/packed-backend: express constants using the_hash_algo 8: 8318dcb630 = 9: 55269d5fc2 upload-pack: express constants in terms of the_hash_algo 9: 16916c9fa2 = 10: d30536a3bd transport: use parse_oid_hex instead of a constant 10: 9d8e2bc4ae = 11: 2139fe1fe1 tag: express constant in terms of the_hash_algo 11: 58f91f2167 = 12: af815c4215 apply: replace hard-coded constants 12: 6899dfc4af = 13: f651b226c8 apply: rename new_sha1_prefix and old_sha1_prefix 13: cc974651cb = 14: bf1d450aa5 submodule: make zero-oid comparison hash function agnostic 14: b373f16c12 = 15: d984036c1c rerere: convert to use the_hash_algo
[PATCH v2 06/15] pack-revindex: express constants in terms of the_hash_algo
Express the various constants used in terms of the_hash_algo.
While we're at it, fix a comment style issue as well.
Reviewed-by: Stefan Beller
Signed-off-by: brian m. carlson
---
pack-revindex.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/pack-revindex.c b/pack-revindex.c
index bb521cf7fb..3c58784a5f 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -122,13 +122,14 @@ static void create_pack_revindex(struct packed_git *p)
unsigned num_ent = p->num_objects;
unsigned i;
const char *index = p->index_data;
+ const unsigned hashsz = the_hash_algo->rawsz;
ALLOC_ARRAY(p->revindex, num_ent + 1);
index += 4 * 256;
if (p->index_version > 1) {
const uint32_t *off_32 =
- (uint32_t *)(index + 8 + p->num_objects * (20 + 4));
+ (uint32_t *)(index + 8 + p->num_objects * (hashsz + 4));
const uint32_t *off_64 = off_32 + p->num_objects;
for (i = 0; i < num_ent; i++) {
uint32_t off = ntohl(*off_32++);
@@ -142,16 +143,17 @@ static void create_pack_revindex(struct packed_git *p)
}
} else {
for (i = 0; i < num_ent; i++) {
- uint32_t hl = *((uint32_t *)(index + 24 * i));
+ uint32_t hl = *((uint32_t *)(index + (hashsz + 4) * i));
p->revindex[i].offset = ntohl(hl);
p->revindex[i].nr = i;
}
}
- /* This knows the pack format -- the 20-byte trailer
+ /*
+* This knows the pack format -- the hash trailer
* follows immediately after the last object data.
*/
- p->revindex[num_ent].offset = p->pack_size - 20;
+ p->revindex[num_ent].offset = p->pack_size - hashsz;
p->revindex[num_ent].nr = -1;
sort_revindex(p->revindex, num_ent, p->pack_size);
}
[PATCH v2 09/15] upload-pack: express constants in terms of the_hash_algo
Convert all uses of the GIT_SHA1_HEXSZ to use the_hash_algo so that they
are appropriate for any given hash length.
Signed-off-by: brian m. carlson
---
upload-pack.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/upload-pack.c b/upload-pack.c
index 62a1000f44..1aae5dd828 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -443,6 +443,7 @@ static int do_reachable_revlist(struct child_process *cmd,
struct object *o;
char namebuf[GIT_MAX_HEXSZ + 2]; /* ^ + hash + LF */
int i;
+ const unsigned hexsz = the_hash_algo->hexsz;
cmd->argv = argv;
cmd->git_cmd = 1;
@@ -461,7 +462,7 @@ static int do_reachable_revlist(struct child_process *cmd,
goto error;
namebuf[0] = '^';
- namebuf[GIT_SHA1_HEXSZ + 1] = '\n';
+ namebuf[hexsz + 1] = '\n';
for (i = get_max_object_index(); 0 < i; ) {
o = get_indexed_object(--i);
if (!o)
@@ -470,11 +471,11 @@ static int do_reachable_revlist(struct child_process *cmd,
o->flags &= ~TMP_MARK;
if (!is_our_ref(o))
continue;
- memcpy(namebuf + 1, oid_to_hex(>oid), GIT_SHA1_HEXSZ);
- if (write_in_full(cmd->in, namebuf, GIT_SHA1_HEXSZ + 2) < 0)
+ memcpy(namebuf + 1, oid_to_hex(>oid), hexsz);
+ if (write_in_full(cmd->in, namebuf, hexsz + 2) < 0)
goto error;
}
- namebuf[GIT_SHA1_HEXSZ] = '\n';
+ namebuf[hexsz] = '\n';
for (i = 0; i < src->nr; i++) {
o = src->objects[i].item;
if (is_our_ref(o)) {
@@ -484,8 +485,8 @@ static int do_reachable_revlist(struct child_process *cmd,
}
if (reachable && o->type == OBJ_COMMIT)
o->flags |= TMP_MARK;
- memcpy(namebuf, oid_to_hex(>oid), GIT_SHA1_HEXSZ);
- if (write_in_full(cmd->in, namebuf, GIT_SHA1_HEXSZ + 1) < 0)
+ memcpy(namebuf, oid_to_hex(>oid), hexsz);
+ if (write_in_full(cmd->in, namebuf, hexsz + 1) < 0)
goto error;
}
close(cmd->in);
[PATCH v2 04/15] builtin/mktree: remove hard-coded constant
Instead of using a hard-coded constant for the size of a hex object ID,
switch to use the computed pointer from parse_oid_hex that points after
the parsed object ID.
Signed-off-by: brian m. carlson
---
builtin/mktree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/mktree.c b/builtin/mktree.c
index 2dc4ad6ba8..94e82b8504 100644
--- a/builtin/mktree.c
+++ b/builtin/mktree.c
@@ -98,7 +98,7 @@ static void mktree_line(char *buf, size_t len, int
nul_term_line, int allow_miss
*ntr++ = 0; /* now at the beginning of SHA1 */
- path = ntr + 41; /* at the beginning of name */
+ path = (char *)p + 1; /* at the beginning of name */
if (!nul_term_line && path[0] == '"') {
struct strbuf p_uq = STRBUF_INIT;
if (unquote_c_style(_uq, path, NULL))
[PATCH v2 03/15] builtin/repack: replace hard-coded constants
Note that while the error messages here are not translated, the end user
should never see them. We invoke git pack-objects shortly before both
invocations, so we can be fairly certain that the data we're receiving
is in fact valid.
Signed-off-by: brian m. carlson
---
builtin/repack.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index c6a7943d5c..0223f2880c 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -235,8 +235,8 @@ static void repack_promisor_objects(const struct
pack_objects_args *args,
while (strbuf_getline_lf(, out) != EOF) {
char *promisor_name;
int fd;
- if (line.len != 40)
- die("repack: Expecting 40 character sha1 lines only
from pack-objects.");
+ if (line.len != the_hash_algo->hexsz)
+ die("repack: Expecting full hex object ID lines only
from pack-objects.");
string_list_append(names, line.buf);
/*
@@ -407,8 +407,8 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
out = xfdopen(cmd.out, "r");
while (strbuf_getline_lf(, out) != EOF) {
- if (line.len != 40)
- die("repack: Expecting 40 character sha1 lines only
from pack-objects.");
+ if (line.len != the_hash_algo->hexsz)
+ die("repack: Expecting full hex object ID lines only
from pack-objects.");
string_list_append(, line.buf);
}
fclose(out);
@@ -535,14 +535,15 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
reprepare_packed_git(the_repository);
if (delete_redundant) {
+ const int hexsz = the_hash_algo->hexsz;
int opts = 0;
string_list_sort();
for_each_string_list_item(item, _packs) {
char *sha1;
size_t len = strlen(item->string);
- if (len < 40)
+ if (len < hexsz)
continue;
- sha1 = item->string + len - 40;
+ sha1 = item->string + len - hexsz;
if (!string_list_has_string(, sha1))
remove_redundant_pack(packdir, item->string);
}
Re: [PATCH v2 1/1] zlib.c: use size_t for size
On Sun, Oct 14, 2018 at 04:03:48PM +0100, Ramsay Jones wrote:
> > So I kind of wonder if a comment would be better than xsize_t here.
> > Something like:
> >
> > if (avail > len) {
> > /*
> > * This can never truncate because we know that len is smaller
> > * than avail, which is already a size_t.
> > */
> > avail = (size_t)len;
> > }
>
> Heh, you are, of course, correct! (that will learn me[1]). :-D
>
> Hmm, let's see if I can muster the enthusiasm to do all that
> testing again!
For the record, I am not opposed to including the comment _and_ using
xsize_t() to do the cast, giving us an assertion that the comment is
correct.
-Peff
[PATCH v2 08/15] refs/packed-backend: express constants using the_hash_algo
Switch uses of GIT_SHA1_HEXSZ to use the_hash_algo so that they are
appropriate for the any given hash length.
Signed-off-by: brian m. carlson
---
refs/packed-backend.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 74e2996e93..c01c7f5901 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -274,8 +274,8 @@ struct snapshot_record {
static int cmp_packed_ref_records(const void *v1, const void *v2)
{
const struct snapshot_record *e1 = v1, *e2 = v2;
- const char *r1 = e1->start + GIT_SHA1_HEXSZ + 1;
- const char *r2 = e2->start + GIT_SHA1_HEXSZ + 1;
+ const char *r1 = e1->start + the_hash_algo->hexsz + 1;
+ const char *r2 = e2->start + the_hash_algo->hexsz + 1;
while (1) {
if (*r1 == '\n')
@@ -297,7 +297,7 @@ static int cmp_packed_ref_records(const void *v1, const
void *v2)
*/
static int cmp_record_to_refname(const char *rec, const char *refname)
{
- const char *r1 = rec + GIT_SHA1_HEXSZ + 1;
+ const char *r1 = rec + the_hash_algo->hexsz + 1;
const char *r2 = refname;
while (1) {
@@ -344,7 +344,7 @@ static void sort_snapshot(struct snapshot *snapshot)
if (!eol)
/* The safety check should prevent this. */
BUG("unterminated line found in packed-refs");
- if (eol - pos < GIT_SHA1_HEXSZ + 2)
+ if (eol - pos < the_hash_algo->hexsz + 2)
die_invalid_line(snapshot->refs->path,
pos, eof - pos);
eol++;
@@ -456,7 +456,7 @@ static void verify_buffer_safe(struct snapshot *snapshot)
return;
last_line = find_start_of_record(start, eof - 1);
- if (*(eof - 1) != '\n' || eof - last_line < GIT_SHA1_HEXSZ + 2)
+ if (*(eof - 1) != '\n' || eof - last_line < the_hash_algo->hexsz + 2)
die_invalid_line(snapshot->refs->path,
last_line, eof - last_line);
}
@@ -796,7 +796,7 @@ static int next_record(struct packed_ref_iterator *iter)
iter->base.flags = REF_ISPACKED;
- if (iter->eof - p < GIT_SHA1_HEXSZ + 2 ||
+ if (iter->eof - p < the_hash_algo->hexsz + 2 ||
parse_oid_hex(p, >oid, ) ||
!isspace(*p++))
die_invalid_line(iter->snapshot->refs->path,
@@ -826,7 +826,7 @@ static int next_record(struct packed_ref_iterator *iter)
if (iter->pos < iter->eof && *iter->pos == '^') {
p = iter->pos + 1;
- if (iter->eof - p < GIT_SHA1_HEXSZ + 1 ||
+ if (iter->eof - p < the_hash_algo->hexsz + 1 ||
parse_oid_hex(p, >peeled, ) ||
*p++ != '\n')
die_invalid_line(iter->snapshot->refs->path,
Re: [RFC/PATCH] headers: normalize the spelling of some header guards
On Sun, Oct 14, 2018 at 09:13:09PM +0100, Ramsay Jones wrote: > This patch is marked RFC because I am not aware of any policy with > regard to header guard spelling. Having said that, apart from the > fetch-negotiator.h header, all of these headers are using a reserved > identifier (see C99 Standard 7.1.3). > > These headers were found, thus: > > $ git grep -n -E '^#ifn?def ' -- '*.h' | grep 'h\:1\:' | grep -v '^compat' > | grep -v -E '[A-Z_]*_H$' > alias.h:1:#ifndef __ALIAS_H__ > commit-reach.h:1:#ifndef __COMMIT_REACH_H__ > fetch-negotiator.h:1:#ifndef FETCH_NEGOTIATOR > midx.h:1:#ifndef __MIDX_H__ > t/helper/test-tool.h:1:#ifndef __TEST_TOOL_H__ > vcs-svn/fast_export.h:1:#ifndef FAST_EXPORT_H_ > vcs-svn/line_buffer.h:1:#ifndef LINE_BUFFER_H_ > vcs-svn/sliding_window.h:1:#ifndef SLIDING_WINDOW_H_ > vcs-svn/svndiff.h:1:#ifndef SVNDIFF_H_ > vcs-svn/svndump.h:1:#ifndef SVNDUMP_H_ I think the ones with a trailing underscore are actually OK according to the standard (not sure if your "all of these" was including the ones in vcs-svn ;) ). I'm in favor of normalizing even the ones that aren't illegal, though I'm OK either way on the vcs-svn bits if they're going away anyway. -Peff
Re: [RFC PATCH 2/3] Documentation/git-rev-list: s///
Matthew DeVore writes:
> -List commits that are reachable by following the `parent` links from the
> -given commit(s), but exclude commits that are reachable from the one(s)
> -given with a '{caret}' in front of them. The output is given in reverse
> -chronological order by default.
> +List objects that are reachable by following references from the given
> +object(s), but exclude objects that are reachable from the one(s) given
> +with a '{caret}' in front of them.
>
> +By default, only commit objects are shown, and the commits are shown in
> +reverse chronological order. The '--object' flag causes non-commit objects
> +to also be shown.
>
> +You can think of this as a set operation. Objects given on the command
> +line form a set of objects that are reachable from any of them, and then
> +objects reachable from any of the ones given with '{caret}' in front are
> +subtracted from that set. The remaining objects are what come out in the
> command's output. Various other options and paths parameters can be used
> to further limit the result.
I am not sure if this is a good rewrite. It gives a false
impression as if you'd not see anything if I did this:
git rev-list --objects ^master md/filter-trees:t/valgrind
especially if you change the SYNOPSIS and make it claim that the
command takes as starting point. Updating to
for those that are _shown_ is OK, but the s/commit/object/
for those that are given as the starting points is not a good change
at all, if I understand what the code is designed to do correctly.
It is more like "this is a set operation across commits. We also
show objects that are reachable from the commits in the resulting
set and are not reachable from the commits in the set that were
excluded when --objects option is given".
Re: [RFC/PATCH] headers: normalize the spelling of some header guards
On Sun, Oct 14, 2018 at 09:13:09PM +0100, Ramsay Jones wrote: > > Signed-off-by: Ramsay Jones > --- > > Hi Junio, > > This patch is marked RFC because I am not aware of any policy with > regard to header guard spelling. Having said that, apart from the > fetch-negotiator.h header, all of these headers are using a reserved > identifier (see C99 Standard 7.1.3). > > These headers were found, thus: > > $ git grep -n -E '^#ifn?def ' -- '*.h' | grep 'h\:1\:' | grep -v '^compat' > | grep -v -E '[A-Z_]*_H$' > alias.h:1:#ifndef __ALIAS_H__ > commit-reach.h:1:#ifndef __COMMIT_REACH_H__ > fetch-negotiator.h:1:#ifndef FETCH_NEGOTIATOR > midx.h:1:#ifndef __MIDX_H__ > t/helper/test-tool.h:1:#ifndef __TEST_TOOL_H__ > vcs-svn/fast_export.h:1:#ifndef FAST_EXPORT_H_ > vcs-svn/line_buffer.h:1:#ifndef LINE_BUFFER_H_ > vcs-svn/sliding_window.h:1:#ifndef SLIDING_WINDOW_H_ > vcs-svn/svndiff.h:1:#ifndef SVNDIFF_H_ > vcs-svn/svndump.h:1:#ifndef SVNDUMP_H_ > I think this is a good change. We should definitely get rid of the reserved identifiers, but I'm definitely not opposed to tidying things up. -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 signature.asc Description: PGP signature
Re: [RFC PATCH 1/3] list-objects: support for skipping tree traversal
Matthew DeVore writes:
> The tree:0 filter does not need to traverse the trees that it has
> filtered out, so optimize list-objects and list-objects-filter to skip
> traversing the trees entirely. Before this patch, we iterated over all
> children of the tree, and did nothing for all of them, which was
> wasteful.
>
> Signed-off-by: Matthew DeVore
> ---
> list-objects-filter.c | 11 +--
> list-objects-filter.h | 6 ++
> list-objects.c | 5 -
> t/t6112-rev-list-filters-objects.sh | 10 ++
> 4 files changed, 29 insertions(+), 3 deletions(-)
This step looks more like "ow, we could have done the tree:0 support
that is in 'next' better" than a part of "here is a series to do
tree:N for non zero value of N".
If that is the case, I'd prefer to see this step polished enough
before [2-3/3] of this RFC is worked on, so that we can merge the
tree:0 (but not yet tree:N) support that is solid down to 'master'
soonish.
> diff --git a/list-objects-filter.c b/list-objects-filter.c
> index 09b2b05d5..37fba456d 100644
> --- a/list-objects-filter.c
> +++ b/list-objects-filter.c
> @@ -102,9 +102,16 @@ static enum list_objects_filter_result filter_trees_none(
>
> case LOFS_BEGIN_TREE:
> case LOFS_BLOB:
> - if (filter_data->omits)
> + if (filter_data->omits) {
> oidset_insert(filter_data->omits, >oid);
> - return LOFR_MARK_SEEN; /* but not LOFR_DO_SHOW (hard omit) */
> + /* _MARK_SEEN but not _DO_SHOW (hard omit) */
> + return LOFR_MARK_SEEN;
> + }
> + else
> + /*
> + * Not collecting omits so no need to to traverse tree.
> + */
> + return LOFR_SKIP_TREE | LOFR_MARK_SEEN;
OK, so "not collecting omits" is synonymous to "N==0, we aren't
doing tree at any level", and at this point in the series before the
support for N>0 is introduced, we'd always take this "else" clause
because tree:0 is the only thing we support?
Style: our modern style is to use {} around the body which is a
single statement on the else clause when the body of the
corresponding if clause needs {} around (and vice versa), so
if (filter_data->omits) {
oidset_isnert(...);
return ...;
} else {
return ...;
}
> diff --git a/list-objects-filter.h b/list-objects-filter.h
> index a6f6b4990..52b4a84da 100644
> --- a/list-objects-filter.h
> +++ b/list-objects-filter.h
> @@ -24,6 +24,11 @@ struct oidset;
> * In general, objects should only be shown once, but
> * this result DOES NOT imply that we mark it SEEN.
> *
> + * _SKIP_TREE : Used in LOFS_BEGIN_TREE situation - indicates that
> + * the tree's children should not be iterated over. This
> + * is used as an optimization when all children will
> + * definitely be ignored.
> + *
> * Most of the time, you want the combination (_MARK_SEEN | _DO_SHOW)
> * but they can be used independently, such as when sparse-checkout
> * pattern matching is being applied.
> @@ -45,6 +50,7 @@ enum list_objects_filter_result {
> LOFR_ZERO = 0,
> LOFR_MARK_SEEN = 1<<0,
> LOFR_DO_SHOW = 1<<1,
> + LOFR_SKIP_TREE = 1<<2,
> };
>
> enum list_objects_filter_situation {
> diff --git a/list-objects.c b/list-objects.c
> index 7a1a0929d..d1e3d217c 100644
> --- a/list-objects.c
> +++ b/list-objects.c
> @@ -11,6 +11,7 @@
> #include "list-objects-filter-options.h"
> #include "packfile.h"
> #include "object-store.h"
> +#include "trace.h"
>
> struct traversal_context {
> struct rev_info *revs;
> @@ -184,7 +185,9 @@ static void process_tree(struct traversal_context *ctx,
> if (base->len)
> strbuf_addch(base, '/');
>
> - if (!failed_parse)
> + if (r & LOFR_SKIP_TREE)
> + trace_printf("Skipping contents of tree %s...\n", base->buf);
> + else if (!failed_parse)
> process_tree_contents(ctx, tree, base);
Even when failed_parse==true, i.e. we found that the tree object's
data cannot be understood, if we have skip-tree bit set, that means
we do not care---we won't be descending into its children anyway.
Makes sense.
> diff --git a/t/t6112-rev-list-filters-objects.sh
> b/t/t6112-rev-list-filters-objects.sh
> index 08e0c7db6..efb1bee2e 100755
> --- a/t/t6112-rev-list-filters-objects.sh
> +++ b/t/t6112-rev-list-filters-objects.sh
> @@ -244,6 +244,16 @@ test_expect_success 'verify tree:0 includes trees in
> "filtered" output' '
> test_cmp expected filtered_types
> '
>
> +# Make sure tree:0 does not iterate through any trees.
> +
> +test_expect_success 'filter a GIANT tree through tree:0' '
> + GIT_TRACE=1 git -C r3 rev-list \
> + --objects --filter=tree:0 HEAD 2>filter_trace &&
> + grep
Re: [BUG] gitignore documentation inconsistent with actual behaviour
Duy Nguyen writes: >> Our matching function comes from rsync originally, whose manpage says: >> >> use ’**’ to match anything, including slashes. >> >> I believe this is accurate as far as the implementation goes. > > No. "**" semantics is not the same as from rsync. The three cases > "**/", "/**/" and "/**" were requested by Junio if I remember > correctly. You can search the mail archive for more information. Perhaps spelling the rules out would be more benefitial for the purpose of this thread? I do not recall what I requested, but let me throw out my guesses (i.e. what I would have wished if I were making a request to implement something) to keep the thread alive, you can correct me, and people can take it from there to update the docs ;-) A double-asterisk, both of whose ends are adjacent to a directory boundary (i.e. the beginning of the pattern, the end of the pattern or a slash) macthes 0 or more levels of directories. e.g. **/a/b would match a/b, x/a/b, x/y/a/b, but not z-a/b. a/**/b would match a/b, a/x/b, but not a/z-b or a-z-b. What a double-asterisk that does not sit on a directory boundary, e.g. "a**b", "a**/b", "a/**b", or "**a/b", matches, as far as I am concerned, is undefined, meaning that (1) I do not care all that much what the code actually do when a pattern like that is given as long as it does not segfault, and (2) I do not think I would mind changing the behaviour as a "bugfix", if their current behaviour does not make sense and we can come up with a saner alternative. But the documentation probably should describe what these currently match as the starting point. Thanks.
[PATCH v10 18/21] stash: convert save to builtin
Add stash save to the helper and delete functions which are no
longer needed (`show_help()`, `save_stash()`, `push_stash()`,
`create_stash()`, `clear_stash()`, `untracked_files()` and
`no_changes()`).
The `-m` option is no longer supported as it might not make
sense to have two ways of passing a message. Even if this is
a change in behaviour, the documentation remains the same
because the `-m` parameter was omitted before.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 52 +++
git-stash.sh| 311 +---
2 files changed, 54 insertions(+), 309 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index befb944871..a0413f5d00 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -26,6 +26,8 @@ static const char * const git_stash_helper_usage[] = {
N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index]
[-q|--quiet]\n"
" [-u|--include-untracked] [-a|--all] [-m|--message
]\n"
" [--] [...]]"),
+ N_("git stash--helper save [-p|--patch] [-k|--[no-]keep-index]
[-q|--quiet]\n"
+ " [-u|--include-untracked] [-a|--all] []"),
NULL
};
@@ -81,6 +83,12 @@ static const char * const git_stash_helper_push_usage[] = {
NULL
};
+static const char * const git_stash_helper_save_usage[] = {
+ N_("git stash--helper save [-p|--patch] [-k|--[no-]keep-index]
[-q|--quiet]\n"
+ " [-u|--include-untracked] [-a|--all] []"),
+ NULL
+};
+
static const char *ref_stash = "refs/stash";
static struct strbuf stash_index_path = STRBUF_INIT;
@@ -1496,6 +1504,48 @@ static int push_stash(int argc, const char **argv, const
char *prefix)
keep_index, patch_mode, include_untracked);
}
+static int save_stash(int argc, const char **argv, const char *prefix)
+{
+ int keep_index = -1;
+ int patch_mode = 0;
+ int include_untracked = 0;
+ int quiet = 0;
+ int ret = 0;
+ char *stash_msg = NULL;
+ struct pathspec ps;
+ struct strbuf buf = STRBUF_INIT;
+ struct option options[] = {
+ OPT_BOOL('k', "keep-index", _index,
+N_("keep index")),
+ OPT_BOOL('p', "patch", _mode,
+N_("stash in patch mode")),
+ OPT__QUIET(, N_("quiet mode")),
+ OPT_BOOL('u', "include-untracked", _untracked,
+N_("include untracked files in stash")),
+ OPT_SET_INT('a', "all", _untracked,
+ N_("include ignore files"), 2),
+ OPT_STRING('m', "message", _msg, "message",
+ N_("stash message")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_stash_helper_save_usage,
+PARSE_OPT_KEEP_DASHDASH);
+
+ if (argc) {
+ strbuf_join_argv(, argc, argv, ' ');
+ stash_msg = buf.buf;
+ }
+
+ memset(, 0, sizeof(ps));
+ ret = do_push_stash(ps, stash_msg, quiet, keep_index, patch_mode,
+ include_untracked);
+
+ strbuf_release();
+ return ret;
+}
+
int cmd_stash__helper(int argc, const char **argv, const char *prefix)
{
pid_t pid = getpid();
@@ -1536,6 +1586,8 @@ int cmd_stash__helper(int argc, const char **argv, const
char *prefix)
return !!create_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "push"))
return !!push_stash(argc, argv, prefix);
+ else if (!strcmp(argv[0], "save"))
+ return !!save_stash(argc, argv, prefix);
usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index c3146f62ab..695f1feba3 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -36,314 +36,6 @@ else
reset_color=
fi
-no_changes () {
- git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" &&
- git diff-files --quiet --ignore-submodules -- "$@" &&
- (test -z "$untracked" || test -z "$(untracked_files "$@")")
-}
-
-untracked_files () {
- if test "$1" = "-z"
- then
- shift
- z=-z
- else
- z=
- fi
- excl_opt=--exclude-standard
- test "$untracked" = "all" && excl_opt=
- git ls-files -o $z $excl_opt -- "$@"
-}
-
-clear_stash () {
- if test $# != 0
- then
- die "$(gettext "git stash clear with parameters is
unimplemented")"
- fi
- if current=$(git rev-parse --verify --quiet $ref_stash)
- then
- git update-ref -d $ref_stash $current
- fi
-}
-
-create_stash () {
- stash_msg=
- untracked=
- while test $# != 0
- do
-
[PATCH v10 07/21] stash: mention options in `show` synopsis
Mention in the documentation, that `show` accepts any
option known to `git diff`.
Signed-off-by: Paul-Sebastian Ungureanu
---
Documentation/git-stash.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 7ef8c47911..e31ea7d303 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -9,7 +9,7 @@ SYNOPSIS
[verse]
'git stash' list []
-'git stash' show []
+'git stash' show [] []
'git stash' drop [-q|--quiet] []
'git stash' ( pop | apply ) [--index] [-q|--quiet] []
'git stash' branch []
@@ -106,7 +106,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
The command takes options applicable to the 'git log'
command to control what is shown and how. See linkgit:git-log[1].
-show []::
+show [] []::
Show the changes recorded in the stash entry as a diff between the
stashed contents and the commit back when the stash entry was first
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 05/21] stash: rename test cases to be more descriptive
Rename some test cases' labels to be more descriptive and under 80
characters per line.
Signed-off-by: Paul-Sebastian Ungureanu
---
t/t3903-stash.sh | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index de6cab1fe7..3114c7bc4c 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -604,7 +604,7 @@ test_expect_success 'stash show -p - no stashes on stack,
stash-like argument' '
test_cmp expected actual
'
-test_expect_success 'stash drop - fail early if specified stash is not a stash
reference' '
+test_expect_success 'drop: fail early if specified stash is not a stash ref' '
git stash clear &&
test_when_finished "git reset --hard HEAD && git stash clear" &&
git reset --hard &&
@@ -618,7 +618,7 @@ test_expect_success 'stash drop - fail early if specified
stash is not a stash r
git reset --hard HEAD
'
-test_expect_success 'stash pop - fail early if specified stash is not a stash
reference' '
+test_expect_success 'pop: fail early if specified stash is not a stash ref' '
git stash clear &&
test_when_finished "git reset --hard HEAD && git stash clear" &&
git reset --hard &&
@@ -682,7 +682,7 @@ test_expect_success 'invalid ref of the form "n", n >= N' '
git stash drop
'
-test_expect_success 'stash branch should not drop the stash if the branch
exists' '
+test_expect_success 'branch: do not drop the stash if the branch exists' '
git stash clear &&
echo foo >file &&
git add file &&
@@ -693,7 +693,7 @@ test_expect_success 'stash branch should not drop the stash
if the branch exists
git rev-parse stash@{0} --
'
-test_expect_success 'stash branch should not drop the stash if the apply
fails' '
+test_expect_success 'branch: should not drop the stash if the apply fails' '
git stash clear &&
git reset HEAD~1 --hard &&
echo foo >file &&
@@ -707,7 +707,7 @@ test_expect_success 'stash branch should not drop the stash
if the apply fails'
git rev-parse stash@{0} --
'
-test_expect_success 'stash apply shows status same as git status (relative to
current directory)' '
+test_expect_success 'apply: show same status as git status (relative to ./)' '
git stash clear &&
echo 1 >subdir/subfile1 &&
echo 2 >subdir/subfile2 &&
@@ -1048,7 +1048,7 @@ test_expect_success 'stash push -p with pathspec shows no
changes only once' '
test_i18ncmp expect actual
'
-test_expect_success 'stash push with pathspec shows no changes when there are
none' '
+test_expect_success 'push : show no changes when there are none' '
>foo &&
git add foo &&
git commit -m "tmp" &&
@@ -1058,7 +1058,7 @@ test_expect_success 'stash push with pathspec shows no
changes when there are no
test_i18ncmp expect actual
'
-test_expect_success 'stash push with pathspec not in the repository errors
out' '
+test_expect_success 'push: not in the repository errors out' '
>untracked &&
test_must_fail git stash push untracked &&
test_path_is_file untracked
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 09/21] stash: convert drop and clear to builtin
From: Joel Teichroeb
Add the drop and clear commands to the builtin helper. These two
are each simple, but are being added together as they are quite
related.
We have to unfortunately keep the drop and clear functions in the
shell script as functions are called with parameters internally
that are not valid when the commands are called externally. Once
pop is converted they can both be removed.
Signed-off-by: Joel Teichroeb
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 117
git-stash.sh| 4 +-
2 files changed, 119 insertions(+), 2 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index c43e0aacbb..aff6cda4c9 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -12,7 +12,14 @@
#include "rerere.h"
static const char * const git_stash_helper_usage[] = {
+ N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper apply [--index] [-q|--quiet] []"),
+ N_("git stash--helper clear"),
+ NULL
+};
+
+static const char * const git_stash_helper_drop_usage[] = {
+ N_("git stash--helper drop [-q|--quiet] []"),
NULL
};
@@ -21,6 +28,11 @@ static const char * const git_stash_helper_apply_usage[] = {
NULL
};
+static const char * const git_stash_helper_clear_usage[] = {
+ N_("git stash--helper clear"),
+ NULL
+};
+
static const char *ref_stash = "refs/stash";
static struct strbuf stash_index_path = STRBUF_INIT;
@@ -139,6 +151,32 @@ static int get_stash_info(struct stash_info *info, int
argc, const char **argv)
return !(ret == 0 || ret == 1);
}
+static int do_clear_stash(void)
+{
+ struct object_id obj;
+ if (get_oid(ref_stash, ))
+ return 0;
+
+ return delete_ref(NULL, ref_stash, , 0);
+}
+
+static int clear_stash(int argc, const char **argv, const char *prefix)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_stash_helper_clear_usage,
+PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (argc)
+ return error(_("git stash clear with parameters is "
+ "unimplemented"));
+
+ return do_clear_stash();
+}
+
static int reset_tree(struct object_id *i_tree, int update, int reset)
{
int nr_trees = 1;
@@ -426,6 +464,81 @@ static int apply_stash(int argc, const char **argv, const
char *prefix)
return ret;
}
+static int do_drop_stash(const char *prefix, struct stash_info *info, int
quiet)
+{
+ int ret;
+ struct child_process cp_reflog = CHILD_PROCESS_INIT;
+ struct child_process cp = CHILD_PROCESS_INIT;
+
+ /*
+* reflog does not provide a simple function for deleting refs. One will
+* need to be added to avoid implementing too much reflog code here
+*/
+
+ cp_reflog.git_cmd = 1;
+ argv_array_pushl(_reflog.args, "reflog", "delete", "--updateref",
+"--rewrite", NULL);
+ argv_array_push(_reflog.args, info->revision.buf);
+ ret = run_command(_reflog);
+ if (!ret) {
+ if (!quiet)
+ printf_ln(_("Dropped %s (%s)"), info->revision.buf,
+ oid_to_hex(>w_commit));
+ } else {
+ return error(_("%s: Could not drop stash entry"),
+info->revision.buf);
+ }
+
+ /*
+* This could easily be replaced by get_oid, but currently it will throw
+* a fatal error when a reflog is empty, which we can not recover from.
+*/
+ cp.git_cmd = 1;
+ /* Even though --quiet is specified, rev-parse still outputs the hash */
+ cp.no_stdout = 1;
+ argv_array_pushl(, "rev-parse", "--verify", "--quiet", NULL);
+ argv_array_pushf(, "%s@{0}", ref_stash);
+ ret = run_command();
+
+ /* do_clear_stash if we just dropped the last stash entry */
+ if (ret)
+ do_clear_stash();
+
+ return 0;
+}
+
+static void assert_stash_ref(struct stash_info *info)
+{
+ if (!info->is_stash_ref) {
+ free_stash_info(info);
+ error(_("'%s' is not a stash reference"), info->revision.buf);
+ exit(128);
+ }
+}
+
+static int drop_stash(int argc, const char **argv, const char *prefix)
+{
+ int ret;
+ int quiet = 0;
+ struct stash_info info;
+ struct option options[] = {
+ OPT__QUIET(, N_("be quiet, only report errors")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_stash_helper_drop_usage, 0);
+
+ if (get_stash_info(, argc, argv))
+ return -1;
+
+ assert_stash_ref();
+
+ ret = do_drop_stash(prefix, , quiet);
+
[PATCH v10 16/21] stash: convert push to builtin
Add stash push to the helper.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 241 +++-
git-stash.sh| 6 +-
2 files changed, 241 insertions(+), 6 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index b8e69c1050..23f1329637 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -23,6 +23,9 @@ static const char * const git_stash_helper_usage[] = {
N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet]
[]"),
N_("git stash--helper branch []"),
N_("git stash--helper clear"),
+ N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index]
[-q|--quiet]\n"
+ " [-u|--include-untracked] [-a|--all] [-m|--message
]\n"
+ " [--] [...]]"),
NULL
};
@@ -71,6 +74,13 @@ static const char * const git_stash_helper_create_usage[] = {
NULL
};
+static const char * const git_stash_helper_push_usage[] = {
+ N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index]
[-q|--quiet]\n"
+ " [-u|--include-untracked] [-a|--all] [-m|--message
]\n"
+ " [--] [...]]"),
+ NULL
+};
+
static const char *ref_stash = "refs/stash";
static struct strbuf stash_index_path = STRBUF_INIT;
@@ -1095,7 +1105,7 @@ static int stash_working_tree(struct stash_info *info,
struct pathspec ps)
static int do_create_stash(struct pathspec ps, char **stash_msg,
int include_untracked, int patch_mode,
- struct stash_info *info)
+ struct stash_info *info, struct strbuf *patch)
{
int ret = 0;
int flags = 0;
@@ -1109,7 +1119,6 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
struct strbuf commit_tree_label = STRBUF_INIT;
struct strbuf untracked_files = STRBUF_INIT;
struct strbuf stash_msg_buf = STRBUF_INIT;
- struct strbuf patch = STRBUF_INIT;
read_cache_preload(NULL);
refresh_cache(REFRESH_QUIET);
@@ -1160,7 +1169,7 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
untracked_commit_option = 1;
}
if (patch_mode) {
- ret = stash_patch(info, ps, );
+ ret = stash_patch(info, ps, patch);
*stash_msg = NULL;
if (ret < 0) {
fprintf_ln(stderr, _("Cannot save the current "
@@ -1234,7 +1243,8 @@ static int create_stash(int argc, const char **argv,
const char *prefix)
0);
memset(, 0, sizeof(ps));
- ret = do_create_stash(ps, _msg, include_untracked, 0, );
+ ret = do_create_stash(ps, _msg, include_untracked, 0, ,
+ NULL);
if (!ret)
printf_ln("%s", oid_to_hex(_commit));
@@ -1248,6 +1258,227 @@ static int create_stash(int argc, const char **argv,
const char *prefix)
return ret < 0;
}
+static int do_push_stash(struct pathspec ps, char *stash_msg, int quiet,
+int keep_index, int patch_mode, int include_untracked)
+{
+ int ret = 0;
+ struct stash_info info;
+ struct strbuf patch = STRBUF_INIT;
+
+ if (patch_mode && keep_index == -1)
+ keep_index = 1;
+
+ if (patch_mode && include_untracked) {
+ fprintf_ln(stderr, _("Can't use --patch and --include-untracked"
+" or --all at the same time"));
+ ret = -1;
+ goto done;
+ }
+
+ read_cache_preload(NULL);
+ if (!include_untracked && ps.nr) {
+ int i;
+ char *ps_matched = xcalloc(ps.nr, 1);
+
+ for (i = 0; i < active_nr; i++)
+ ce_path_match(_index, active_cache[i], ,
+ ps_matched);
+
+ if (report_path_error(ps_matched, , NULL)) {
+ fprintf_ln(stderr, _("Did you forget to 'git add'?"));
+ ret = -1;
+ free(ps_matched);
+ goto done;
+ }
+ free(ps_matched);
+ }
+
+ if (refresh_cache(REFRESH_QUIET)) {
+ ret = -1;
+ goto done;
+ }
+
+ if (!check_changes(ps, include_untracked)) {
+ if (!quiet)
+ printf_ln(_("No local changes to save"));
+ goto done;
+ }
+
+ if (!reflog_exists(ref_stash) && do_clear_stash()) {
+ ret = -1;
+ fprintf_ln(stderr, _("Cannot initialize stash"));
+ goto done;
+ }
+
+ if (do_create_stash(ps, _msg, include_untracked, patch_mode,
+ , )) {
+ ret = -1;
+ goto done;
+ }
+
+ if (do_store_stash(_commit, stash_msg, 1)) {
+
[PATCH v10 20/21] stash: optimize `get_untracked_files()` and `check_changes()`
This commits introduces a optimization by avoiding calling the
same functions again. For example, `git stash push -u`
would call at some points the following functions:
* `check_changes()` (inside `do_push_stash()`)
* `do_create_stash()`, which calls: `check_changes()` and
`get_untracked_files()`
Note that `check_changes()` also calls `get_untracked_files()`.
So, `check_changes()` is called 2 times and `get_untracked_files()`
3 times.
The old function `check_changes()` now consists of two functions:
`get_untracked_files()` and `check_changes_tracked_files()`.
These are the call chains for `push` and `create`:
* `push_stash()` -> `do_push_stash()` -> `do_create_stash()`
* `create_stash()` -> `do_create_stash()`
To prevent calling the same functions over and over again,
`check_changes()` inside `do_create_stash()` is now placed
in the caller functions (`create_stash()` and `do_push_stash()`).
This way `check_changes()` and `get_untracked files()` are called
only one time.
https://public-inbox.org/git/20180818223329.gj11...@hank.intra.tgummerer.com/
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash.c | 50 -
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/builtin/stash.c b/builtin/stash.c
index e945c13c42..d2365ada2e 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -881,18 +881,18 @@ static int get_untracked_files(struct pathspec ps, int
include_untracked,
}
/*
- * The return value of `check_changes()` can be:
+ * The return value of `check_changes_tracked_files()` can be:
*
* < 0 if there was an error
* = 0 if there are no changes.
* > 0 if there are changes.
*/
-static int check_changes(struct pathspec ps, int include_untracked)
+
+static int check_changes_tracked_files(struct pathspec ps)
{
int result;
struct rev_info rev;
struct object_id dummy;
- struct strbuf out = STRBUF_INIT;
/* No initial commit. */
if (get_oid("HEAD", ))
@@ -920,14 +920,26 @@ static int check_changes(struct pathspec ps, int
include_untracked)
if (diff_result_code(, result))
return 1;
+ return 0;
+}
+
+/*
+ * The function will fill `untracked_files` with the names of untracked files
+ * It will return 1 if there were any changes and 0 if there were not.
+ */
+
+static int check_changes(struct pathspec ps, int include_untracked,
+struct strbuf *untracked_files)
+{
+ int ret = 0;
+ if (check_changes_tracked_files(ps))
+ ret = 1;
+
if (include_untracked && get_untracked_files(ps, include_untracked,
-)) {
- strbuf_release();
- return 1;
- }
+untracked_files))
+ ret = 1;
- strbuf_release();
- return 0;
+ return ret;
}
static int save_untracked_files(struct stash_info *info, struct strbuf *msg,
@@ -1138,7 +1150,7 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
head_commit = lookup_commit(the_repository, >b_commit);
}
- if (!check_changes(ps, include_untracked)) {
+ if (!check_changes(ps, include_untracked, _files)) {
ret = 1;
goto done;
}
@@ -1163,8 +1175,7 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
goto done;
}
- if (include_untracked && get_untracked_files(ps, include_untracked,
-_files)) {
+ if (include_untracked) {
if (save_untracked_files(info, , untracked_files)) {
if (!quiet)
fprintf_ln(stderr, _("Cannot save "
@@ -1244,19 +1255,15 @@ static int create_stash(int argc, const char **argv,
const char *prefix)
stash_msg = stash_msg_buf.buf;
memset(, 0, sizeof(ps));
- ret = do_create_stash(ps, _msg, 0, 0, , NULL, 0);
+ if (!check_changes_tracked_files(ps))
+ return 0;
- if (!ret)
+ if (!(ret = do_create_stash(ps, _msg, 0, 0, , NULL, 0)))
printf_ln("%s", oid_to_hex(_commit));
strbuf_release(_msg_buf);
free(stash_msg);
-
- /*
-* ret can be 1 if there were no changes. In this case, we should
-* not error out.
-*/
- return ret < 0;
+ return ret;
}
static int do_push_stash(struct pathspec ps, char *stash_msg, int quiet,
@@ -1265,6 +1272,7 @@ static int do_push_stash(struct pathspec ps, char
*stash_msg, int quiet,
int ret = 0;
struct stash_info info;
struct strbuf patch = STRBUF_INIT;
+ struct strbuf untracked_files = STRBUF_INIT;
if (patch_mode && keep_index == -1)
keep_index = 1;
@@ -1299,7 +1307,7 @@ static int do_push_stash(struct pathspec ps, char
[PATCH v10 10/21] stash: convert branch to builtin
From: Joel Teichroeb
Add stash branch to the helper and delete the apply_to_branch
function from the shell script.
Checkout does not currently provide a function for checking out
a branch as cmd_checkout does a large amount of sanity checks
first that we require here.
Signed-off-by: Joel Teichroeb
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 46 +
git-stash.sh| 17 ++-
2 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index aff6cda4c9..c41aad7036 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -14,6 +14,7 @@
static const char * const git_stash_helper_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper apply [--index] [-q|--quiet] []"),
+ N_("git stash--helper branch []"),
N_("git stash--helper clear"),
NULL
};
@@ -28,6 +29,11 @@ static const char * const git_stash_helper_apply_usage[] = {
NULL
};
+static const char * const git_stash_helper_branch_usage[] = {
+ N_("git stash--helper branch []"),
+ NULL
+};
+
static const char * const git_stash_helper_clear_usage[] = {
N_("git stash--helper clear"),
NULL
@@ -539,6 +545,44 @@ static int drop_stash(int argc, const char **argv, const
char *prefix)
return ret;
}
+static int branch_stash(int argc, const char **argv, const char *prefix)
+{
+ int ret;
+ const char *branch = NULL;
+ struct stash_info info;
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct option options[] = {
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_stash_helper_branch_usage, 0);
+
+ if (!argc) {
+ fprintf_ln(stderr, _("No branch name specified"));
+ return -1;
+ }
+
+ branch = argv[0];
+
+ if (get_stash_info(, argc - 1, argv + 1))
+ return -1;
+
+ cp.git_cmd = 1;
+ argv_array_pushl(, "checkout", "-b", NULL);
+ argv_array_push(, branch);
+ argv_array_push(, oid_to_hex(_commit));
+ ret = run_command();
+ if (!ret)
+ ret = do_apply_stash(prefix, , 1, 0);
+ if (!ret && info.is_stash_ref)
+ ret = do_drop_stash(prefix, , 0);
+
+ free_stash_info();
+
+ return ret;
+}
+
int cmd_stash__helper(int argc, const char **argv, const char *prefix)
{
pid_t pid = getpid();
@@ -565,6 +609,8 @@ int cmd_stash__helper(int argc, const char **argv, const
char *prefix)
return !!clear_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "drop"))
return !!drop_stash(argc, argv, prefix);
+ else if (!strcmp(argv[0], "branch"))
+ return !!branch_stash(argc, argv, prefix);
usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index a99d5dc9e5..29d9f44255 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -598,20 +598,6 @@ drop_stash () {
clear_stash
}
-apply_to_branch () {
- test -n "$1" || die "$(gettext "No branch name specified")"
- branch=$1
- shift 1
-
- set -- --index "$@"
- assert_stash_like "$@"
-
- git checkout -b $branch $REV^ &&
- apply_stash "$@" && {
- test -z "$IS_STASH_REF" || drop_stash "$@"
- }
-}
-
test "$1" = "-p" && set "push" "$@"
PARSE_CACHE='--not-parsed'
@@ -673,7 +659,8 @@ pop)
;;
branch)
shift
- apply_to_branch "$@"
+ cd "$START_DIR"
+ git stash--helper branch "$@"
;;
*)
case $# in
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 13/21] stash: convert show to builtin
Add stash show to the helper and delete the show_stash, have_stash,
assert_stash_like, is_stash_like and parse_flags_and_rev functions
from the shell script now that they are no longer needed.
In shell version, although `git stash show` accepts `--index` and
`--quiet` options, it ignores them. In C, both options are passed
further to `git diff`.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 87 ++
git-stash.sh| 132 +---
2 files changed, 88 insertions(+), 131 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index a1249f6b76..3f7ace92f5 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -10,9 +10,12 @@
#include "run-command.h"
#include "dir.h"
#include "rerere.h"
+#include "revision.h"
+#include "log-tree.h"
static const char * const git_stash_helper_usage[] = {
N_("git stash--helper list []"),
+ N_("git stash--helper show [] []"),
N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet]
[]"),
N_("git stash--helper branch []"),
@@ -25,6 +28,11 @@ static const char * const git_stash_helper_list_usage[] = {
NULL
};
+static const char * const git_stash_helper_show_usage[] = {
+ N_("git stash--helper show [] []"),
+ NULL
+};
+
static const char * const git_stash_helper_drop_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
NULL
@@ -647,6 +655,83 @@ static int list_stash(int argc, const char **argv, const
char *prefix)
return run_command();
}
+static int show_stat = 1;
+static int show_patch;
+
+static int git_stash_config(const char *var, const char *value, void *cb)
+{
+ if (!strcmp(var, "stash.showstat")) {
+ show_stat = git_config_bool(var, value);
+ return 0;
+ }
+ if (!strcmp(var, "stash.showpatch")) {
+ show_patch = git_config_bool(var, value);
+ return 0;
+ }
+ return git_default_config(var, value, cb);
+}
+
+static int show_stash(int argc, const char **argv, const char *prefix)
+{
+ int i;
+ int opts = 0;
+ int ret = 0;
+ struct stash_info info;
+ struct rev_info rev;
+ struct argv_array stash_args = ARGV_ARRAY_INIT;
+ struct option options[] = {
+ OPT_END()
+ };
+
+ init_diff_ui_defaults();
+ git_config(git_diff_ui_config, NULL);
+ init_revisions(, prefix);
+
+ for (i = 1; i < argc; i++) {
+ if (argv[i][0] != '-')
+ argv_array_push(_args, argv[i]);
+ else
+ opts++;
+ }
+
+ ret = get_stash_info(, stash_args.argc, stash_args.argv);
+ argv_array_clear(_args);
+ if (ret)
+ return -1;
+
+ /*
+* The config settings are applied only if there are not passed
+* any options.
+*/
+ if (!opts) {
+ git_config(git_stash_config, NULL);
+ if (show_stat)
+ rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT;
+
+ if (show_patch)
+ rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
+
+ if (!show_stat && !show_patch) {
+ free_stash_info();
+ return 0;
+ }
+ }
+
+ argc = setup_revisions(argc, argv, , NULL);
+ if (argc > 1) {
+ free_stash_info();
+ usage_with_options(git_stash_helper_show_usage, options);
+ }
+
+ rev.diffopt.flags.recursive = 1;
+ setup_diff_pager();
+ diff_tree_oid(_commit, _commit, "", );
+ log_tree_diff_flush();
+
+ free_stash_info();
+ return diff_result_code(, 0);
+}
+
int cmd_stash__helper(int argc, const char **argv, const char *prefix)
{
pid_t pid = getpid();
@@ -679,6 +764,8 @@ int cmd_stash__helper(int argc, const char **argv, const
char *prefix)
return !!branch_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "list"))
return !!list_stash(argc, argv, prefix);
+ else if (!strcmp(argv[0], "show"))
+ return !!show_stash(argc, argv, prefix);
usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index 6052441aa2..0d05cbc1e5 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -378,35 +378,6 @@ save_stash () {
fi
}
-have_stash () {
- git rev-parse --verify --quiet $ref_stash >/dev/null
-}
-
-show_stash () {
- ALLOW_UNKNOWN_FLAGS=t
- assert_stash_like "$@"
-
- if test -z "$FLAGS"
- then
- if test "$(git config --bool stash.showStat || echo true)" =
"true"
- then
- FLAGS=--stat
-
[PATCH v10 15/21] stash: convert create to builtin
Add stash create to the helper.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 458
git-stash.sh| 2 +-
2 files changed, 459 insertions(+), 1 deletion(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 758a32572d..b8e69c1050 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -12,6 +12,9 @@
#include "rerere.h"
#include "revision.h"
#include "log-tree.h"
+#include "diffcore.h"
+
+#define INCLUDE_ALL_FILES 2
static const char * const git_stash_helper_usage[] = {
N_("git stash--helper list []"),
@@ -63,6 +66,11 @@ static const char * const git_stash_helper_store_usage[] = {
NULL
};
+static const char * const git_stash_helper_create_usage[] = {
+ N_("git stash--helper create []"),
+ NULL
+};
+
static const char *ref_stash = "refs/stash";
static struct strbuf stash_index_path = STRBUF_INIT;
@@ -290,6 +298,24 @@ static int reset_head(void)
return run_command();
}
+static void add_diff_to_buf(struct diff_queue_struct *q,
+ struct diff_options *options,
+ void *data)
+{
+ int i;
+
+ for (i = 0; i < q->nr; i++) {
+ strbuf_addstr(data, q->queue[i]->one->path);
+
+ /*
+* The reason we add "0" at the end of this strbuf
+* is because we will pass the output further to
+* "git update-index -z ...".
+*/
+ strbuf_addch(data, '\0');
+ }
+}
+
static int get_newly_staged(struct strbuf *out, struct object_id *c_tree)
{
struct child_process cp = CHILD_PROCESS_INIT;
@@ -792,6 +818,436 @@ static int store_stash(int argc, const char **argv, const
char *prefix)
return do_store_stash(, stash_msg, quiet);
}
+static void add_pathspecs(struct argv_array *args,
+ struct pathspec ps) {
+ int i;
+
+ for (i = 0; i < ps.nr; i++)
+ argv_array_push(args, ps.items[i].match);
+}
+
+/*
+ * `untracked_files` will be filled with the names of untracked files.
+ * The return value is:
+ *
+ * = 0 if there are not any untracked files
+ * > 0 if there are untracked files
+ */
+static int get_untracked_files(struct pathspec ps, int include_untracked,
+ struct strbuf *untracked_files)
+{
+ int i;
+ int max_len;
+ int found = 0;
+ char *seen;
+ struct dir_struct dir;
+
+ memset(, 0, sizeof(dir));
+ if (include_untracked != INCLUDE_ALL_FILES)
+ setup_standard_excludes();
+
+ seen = xcalloc(ps.nr, 1);
+
+ max_len = fill_directory(, the_repository->index, );
+ for (i = 0; i < dir.nr; i++) {
+ struct dir_entry *ent = dir.entries[i];
+ if (dir_path_match(_index, ent, , max_len, seen)) {
+ found++;
+ strbuf_addstr(untracked_files, ent->name);
+ /* NUL-terminate: will be fed to update-index -z */
+ strbuf_addch(untracked_files, 0);
+ }
+ free(ent);
+ }
+
+ free(seen);
+ free(dir.entries);
+ free(dir.ignored);
+ clear_directory();
+ return found;
+}
+
+/*
+ * The return value of `check_changes()` can be:
+ *
+ * < 0 if there was an error
+ * = 0 if there are no changes.
+ * > 0 if there are changes.
+ */
+static int check_changes(struct pathspec ps, int include_untracked)
+{
+ int result;
+ struct rev_info rev;
+ struct object_id dummy;
+ struct strbuf out = STRBUF_INIT;
+
+ /* No initial commit. */
+ if (get_oid("HEAD", ))
+ return -1;
+
+ if (read_cache() < 0)
+ return -1;
+
+ init_revisions(, NULL);
+ rev.prune_data = ps;
+
+ rev.diffopt.flags.quick = 1;
+ rev.diffopt.flags.ignore_submodules = 1;
+ rev.abbrev = 0;
+
+ add_head_to_pending();
+ diff_setup_done();
+
+ result = run_diff_index(, 1);
+ if (diff_result_code(, result))
+ return 1;
+
+ object_array_clear();
+ result = run_diff_files(, 0);
+ if (diff_result_code(, result))
+ return 1;
+
+ if (include_untracked && get_untracked_files(ps, include_untracked,
+)) {
+ strbuf_release();
+ return 1;
+ }
+
+ strbuf_release();
+ return 0;
+}
+
+static int save_untracked_files(struct stash_info *info, struct strbuf *msg,
+ struct strbuf files)
+{
+ int ret = 0;
+ struct strbuf untracked_msg = STRBUF_INIT;
+ struct strbuf out = STRBUF_INIT;
+ struct child_process cp_upd_index = CHILD_PROCESS_INIT;
+ struct child_process cp_write_tree = CHILD_PROCESS_INIT;
+
+ cp_upd_index.git_cmd = 1;
+
[PATCH v10 02/21] strbuf.c: add `strbuf_join_argv()`
Implement `strbuf_join_argv()` to join arguments
into a strbuf.
Signed-off-by: Paul-Sebastian Ungureanu
---
strbuf.c | 13 +
strbuf.h | 7 +++
2 files changed, 20 insertions(+)
diff --git a/strbuf.c b/strbuf.c
index 64041c3c24..c8a104099a 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -259,6 +259,19 @@ void strbuf_addbuf(struct strbuf *sb, const struct strbuf
*sb2)
strbuf_setlen(sb, sb->len + sb2->len);
}
+void strbuf_join_argv(struct strbuf *buf,
+ int argc, const char **argv, char delim)
+{
+ if (!argc)
+ return;
+
+ strbuf_addstr(buf, *argv);
+ while (--argc) {
+ strbuf_addch(buf, delim);
+ strbuf_addstr(buf, *(++argv));
+ }
+}
+
void strbuf_addchars(struct strbuf *sb, int c, size_t n)
{
strbuf_grow(sb, n);
diff --git a/strbuf.h b/strbuf.h
index 60a35aef16..4ec912f4b7 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -284,6 +284,13 @@ static inline void strbuf_addstr(struct strbuf *sb, const
char *s)
*/
extern void strbuf_addbuf(struct strbuf *sb, const struct strbuf *sb2);
+/**
+ * Join the arguments into a buffer. `delim` is put between every
+ * two arguments.
+ */
+extern void strbuf_join_argv(struct strbuf *buf, int argc,
+const char **argv, char delim);
+
/**
* This function can be used to expand a format string containing
* placeholders. To that end, it parses the string and calls the specified
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 19/21] stash: convert `stash--helper.c` into `stash.c`
The old shell script `git-stash.sh` was removed and replaced
entirely by `builtin/stash.c`. In order to do that, `create` and
`push` were adapted to work without `stash.sh`. For example, before
this commit, `git stash create` called `git stash--helper create
--message "$*"`. If it called `git stash--helper create "$@"`, then
some of these changes wouldn't have been necessary.
This commit also removes the word `helper` since now stash is
called directly and not by a shell script.
Signed-off-by: Paul-Sebastian Ungureanu
---
.gitignore | 1 -
Makefile | 3 +-
builtin.h| 2 +-
builtin/{stash--helper.c => stash.c} | 166 +++
git-stash.sh | 153
git.c| 2 +-
6 files changed, 96 insertions(+), 231 deletions(-)
rename builtin/{stash--helper.c => stash.c} (90%)
delete mode 100755 git-stash.sh
diff --git a/.gitignore b/.gitignore
index b59661cb88..ffceea7d59 100644
--- a/.gitignore
+++ b/.gitignore
@@ -157,7 +157,6 @@
/git-show-ref
/git-stage
/git-stash
-/git-stash--helper
/git-status
/git-stripspace
/git-submodule
diff --git a/Makefile b/Makefile
index f900c68e69..ac1787d113 100644
--- a/Makefile
+++ b/Makefile
@@ -617,7 +617,6 @@ SCRIPT_SH += git-quiltimport.sh
SCRIPT_SH += git-rebase.sh
SCRIPT_SH += git-remote-testgit.sh
SCRIPT_SH += git-request-pull.sh
-SCRIPT_SH += git-stash.sh
SCRIPT_SH += git-submodule.sh
SCRIPT_SH += git-web--browse.sh
@@ -1093,7 +1092,7 @@ BUILTIN_OBJS += builtin/shortlog.o
BUILTIN_OBJS += builtin/show-branch.o
BUILTIN_OBJS += builtin/show-index.o
BUILTIN_OBJS += builtin/show-ref.o
-BUILTIN_OBJS += builtin/stash--helper.o
+BUILTIN_OBJS += builtin/stash.o
BUILTIN_OBJS += builtin/stripspace.o
BUILTIN_OBJS += builtin/submodule--helper.o
BUILTIN_OBJS += builtin/symbolic-ref.o
diff --git a/builtin.h b/builtin.h
index 317bc338f7..e60f0fc58f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -223,7 +223,7 @@ extern int cmd_show(int argc, const char **argv, const char
*prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
extern int cmd_show_index(int argc, const char **argv, const char *prefix);
extern int cmd_status(int argc, const char **argv, const char *prefix);
-extern int cmd_stash__helper(int argc, const char **argv, const char *prefix);
+extern int cmd_stash(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_submodule__helper(int argc, const char **argv, const char
*prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/builtin/stash--helper.c b/builtin/stash.c
similarity index 90%
rename from builtin/stash--helper.c
rename to builtin/stash.c
index a0413f5d00..e945c13c42 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash.c
@@ -16,75 +16,70 @@
#define INCLUDE_ALL_FILES 2
-static const char * const git_stash_helper_usage[] = {
- N_("git stash--helper list []"),
- N_("git stash--helper show [] []"),
- N_("git stash--helper drop [-q|--quiet] []"),
- N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet]
[]"),
- N_("git stash--helper branch []"),
- N_("git stash--helper clear"),
- N_("git stash--helper [push [-p|--patch] [-k|--[no-]keep-index]
[-q|--quiet]\n"
+static const char * const git_stash_usage[] = {
+ N_("git stash list []"),
+ N_("git stash show [] []"),
+ N_("git stash drop [-q|--quiet] []"),
+ N_("git stash ( pop | apply ) [--index] [-q|--quiet] []"),
+ N_("git stash branch []"),
+ N_("git stash clear"),
+ N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
" [-u|--include-untracked] [-a|--all] [-m|--message
]\n"
" [--] [...]]"),
- N_("git stash--helper save [-p|--patch] [-k|--[no-]keep-index]
[-q|--quiet]\n"
+ N_("git stash save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n"
" [-u|--include-untracked] [-a|--all] []"),
NULL
};
-static const char * const git_stash_helper_list_usage[] = {
- N_("git stash--helper list []"),
+static const char * const git_stash_list_usage[] = {
+ N_("git stash list []"),
NULL
};
-static const char * const git_stash_helper_show_usage[] = {
- N_("git stash--helper show [] []"),
+static const char * const git_stash_show_usage[] = {
+ N_("git stash show [] []"),
NULL
};
-static const char * const git_stash_helper_drop_usage[] = {
- N_("git stash--helper drop [-q|--quiet] []"),
+static const char * const git_stash_drop_usage[] = {
+ N_("git stash drop [-q|--quiet] []"),
NULL
};
-static const char * const git_stash_helper_pop_usage[] = {
- N_("git stash--helper pop [--index] [-q|--quiet] []"),
[PATCH v10 00/21] Convert "git stash" to C builtin
Hello, This is a new iteration of `git stash`, based on the last review. This iteration fixes some code styling issues, bring some changes to `do_push_stash()` and `do_create_stash()` to be closer to API by following Thomas Gummerer's review of last iteration [1]. Also, there were some missing messages [2], which are now included. [1] https://public-inbox.org/git/20181002203730.gi2...@hank.intra.tgummerer.com/ [2] https://github.com/git-for-windows/git/commit/2af24038a95a3879aa0c29d91a43180b9465247e Joel Teichroeb (5): stash: improve option parsing test coverage stash: convert apply to builtin stash: convert drop and clear to builtin stash: convert branch to builtin stash: convert pop to builtin Paul-Sebastian Ungureanu (16): sha1-name.c: add `get_oidf()` which acts like `get_oid()` strbuf.c: add `strbuf_join_argv()` t3903: modernize style stash: rename test cases to be more descriptive stash: add tests for `git stash show` config stash: mention options in `show` synopsis stash: convert list to builtin stash: convert show to builtin stash: convert store to builtin stash: convert create to builtin stash: convert push to builtin stash: make push -q quiet stash: convert save to builtin stash: convert `stash--helper.c` into `stash.c` stash: optimize `get_untracked_files()` and `check_changes()` stash: replace all `write-tree` child processes with API calls Documentation/git-stash.txt |4 +- Makefile |2 +- builtin.h|1 + builtin/stash.c | 1605 ++ cache.h |1 + git-stash.sh | 752 git.c|1 + sha1-name.c | 19 + strbuf.c | 13 + strbuf.h |7 + t/t3903-stash.sh | 192 ++-- t/t3907-stash-show-config.sh | 83 ++ 12 files changed, 1859 insertions(+), 821 deletions(-) create mode 100644 builtin/stash.c delete mode 100755 git-stash.sh create mode 100755 t/t3907-stash-show-config.sh -- 2.19.0.rc0.23.g10a62394e7
[PATCH v10 06/21] stash: add tests for `git stash show` config
This commit introduces tests for `git stash show`
config. It tests all the cases where `stash.showStat`
and `stash.showPatch` are unset or set to true / false.
Signed-off-by: Paul-Sebastian Ungureanu
---
t/t3907-stash-show-config.sh | 83
1 file changed, 83 insertions(+)
create mode 100755 t/t3907-stash-show-config.sh
diff --git a/t/t3907-stash-show-config.sh b/t/t3907-stash-show-config.sh
new file mode 100755
index 00..10914bba7b
--- /dev/null
+++ b/t/t3907-stash-show-config.sh
@@ -0,0 +1,83 @@
+#!/bin/sh
+
+test_description='Test git stash show configuration.'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit file
+'
+
+# takes three parameters:
+# 1. the stash.showStat value (or "")
+# 2. the stash.showPatch value (or "")
+# 3. the diff options of the expected output (or nothing for no output)
+test_stat_and_patch () {
+ if test "" = "$1"
+ then
+ test_unconfig stash.showStat
+ else
+ test_config stash.showStat "$1"
+ fi &&
+
+ if test "" = "$2"
+ then
+ test_unconfig stash.showPatch
+ else
+ test_config stash.showPatch "$2"
+ fi &&
+
+ shift 2 &&
+ echo 2 >file.t &&
+ if test $# != 0
+ then
+ git diff "$@" >expect
+ fi &&
+ git stash &&
+ git stash show >actual &&
+
+ if test $# = 0
+ then
+ test_must_be_empty actual
+ else
+ test_cmp expect actual
+ fi
+}
+
+test_expect_success 'showStat unset showPatch unset' '
+ test_stat_and_patch "" "" --stat
+'
+
+test_expect_success 'showStat unset showPatch false' '
+ test_stat_and_patch "" false --stat
+'
+
+test_expect_success 'showStat unset showPatch true' '
+ test_stat_and_patch "" true --stat -p
+'
+
+test_expect_success 'showStat false showPatch unset' '
+ test_stat_and_patch false ""
+'
+
+test_expect_success 'showStat false showPatch false' '
+ test_stat_and_patch false false
+'
+
+test_expect_success 'showStat false showPatch true' '
+ test_stat_and_patch false true -p
+'
+
+test_expect_success 'showStat true showPatch unset' '
+ test_stat_and_patch true "" --stat
+'
+
+test_expect_success 'showStat true showPatch false' '
+ test_stat_and_patch true false --stat
+'
+
+test_expect_success 'showStat true showPatch true' '
+ test_stat_and_patch true true --stat -p
+'
+
+test_done
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 21/21] stash: replace all `write-tree` child processes with API calls
This commit replaces spawning `git write-tree` with API calls.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash.c | 41 -
1 file changed, 12 insertions(+), 29 deletions(-)
diff --git a/builtin/stash.c b/builtin/stash.c
index d2365ada2e..651d05c820 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -947,9 +947,8 @@ static int save_untracked_files(struct stash_info *info,
struct strbuf *msg,
{
int ret = 0;
struct strbuf untracked_msg = STRBUF_INIT;
- struct strbuf out = STRBUF_INIT;
struct child_process cp_upd_index = CHILD_PROCESS_INIT;
- struct child_process cp_write_tree = CHILD_PROCESS_INIT;
+ struct index_state istate = { NULL };
cp_upd_index.git_cmd = 1;
argv_array_pushl(_upd_index.args, "update-index", "-z", "--add",
@@ -964,15 +963,11 @@ static int save_untracked_files(struct stash_info *info,
struct strbuf *msg,
goto done;
}
- cp_write_tree.git_cmd = 1;
- argv_array_push(_write_tree.args, "write-tree");
- argv_array_pushf(_write_tree.env_array, "GIT_INDEX_FILE=%s",
-stash_index_path.buf);
- if (pipe_command(_write_tree, NULL, 0, , 0,NULL, 0)) {
+ if (write_index_as_tree(>u_tree, , stash_index_path.buf, 0,
+ NULL)) {
ret = -1;
goto done;
}
- get_oid_hex(out.buf, >u_tree);
if (commit_tree(untracked_msg.buf, untracked_msg.len,
>u_tree, NULL, >u_commit, NULL, NULL)) {
@@ -981,8 +976,8 @@ static int save_untracked_files(struct stash_info *info,
struct strbuf *msg,
}
done:
+ discard_index();
strbuf_release(_msg);
- strbuf_release();
remove_path(stash_index_path.buf);
return ret;
}
@@ -991,11 +986,10 @@ static int stash_patch(struct stash_info *info, struct
pathspec ps,
struct strbuf *out_patch, int quiet)
{
int ret = 0;
- struct strbuf out = STRBUF_INIT;
struct child_process cp_read_tree = CHILD_PROCESS_INIT;
struct child_process cp_add_i = CHILD_PROCESS_INIT;
- struct child_process cp_write_tree = CHILD_PROCESS_INIT;
struct child_process cp_diff_tree = CHILD_PROCESS_INIT;
+ struct index_state istate = { NULL };
remove_path(stash_index_path.buf);
@@ -1021,17 +1015,12 @@ static int stash_patch(struct stash_info *info, struct
pathspec ps,
}
/* State of the working tree. */
- cp_write_tree.git_cmd = 1;
- argv_array_push(_write_tree.args, "write-tree");
- argv_array_pushf(_write_tree.env_array, "GIT_INDEX_FILE=%s",
-stash_index_path.buf);
- if (pipe_command(_write_tree, NULL, 0, , 0,NULL, 0)) {
+ if (write_index_as_tree(>w_tree, , stash_index_path.buf, 0,
+ NULL)) {
ret = -1;
goto done;
}
- get_oid_hex(out.buf, >w_tree);
-
cp_diff_tree.git_cmd = 1;
argv_array_pushl(_diff_tree.args, "diff-tree", "-p", "HEAD",
oid_to_hex(>w_tree), "--", NULL);
@@ -1047,7 +1036,7 @@ static int stash_patch(struct stash_info *info, struct
pathspec ps,
}
done:
- strbuf_release();
+ discard_index();
remove_path(stash_index_path.buf);
return ret;
}
@@ -1057,9 +1046,8 @@ static int stash_working_tree(struct stash_info *info,
struct pathspec ps)
int ret = 0;
struct rev_info rev;
struct child_process cp_upd_index = CHILD_PROCESS_INIT;
- struct child_process cp_write_tree = CHILD_PROCESS_INIT;
- struct strbuf out = STRBUF_INIT;
struct strbuf diff_output = STRBUF_INIT;
+ struct index_state istate = { NULL };
set_alternate_index_output(stash_index_path.buf);
if (reset_tree(>i_tree, 0, 0)) {
@@ -1099,20 +1087,15 @@ static int stash_working_tree(struct stash_info *info,
struct pathspec ps)
goto done;
}
- cp_write_tree.git_cmd = 1;
- argv_array_push(_write_tree.args, "write-tree");
- argv_array_pushf(_write_tree.env_array, "GIT_INDEX_FILE=%s",
-stash_index_path.buf);
- if (pipe_command(_write_tree, NULL, 0, , 0,NULL, 0)) {
+ if (write_index_as_tree(>w_tree, , stash_index_path.buf, 0,
+ NULL)) {
ret = -1;
goto done;
}
- get_oid_hex(out.buf, >w_tree);
-
done:
+ discard_index();
UNLEAK(rev);
- strbuf_release();
object_array_clear();
strbuf_release(_output);
remove_path(stash_index_path.buf);
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 17/21] stash: make push -q quiet
There is a change in behaviour with this commit. When there was
no initial commit, the shell version of stash would still display
a message. This commit makes `push` to not display any message if
`--quiet` or `-q` is specified. Add tests for `--quiet`.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 55 +++--
t/t3903-stash.sh| 23 +
2 files changed, 59 insertions(+), 19 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 23f1329637..befb944871 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -973,7 +973,7 @@ static int save_untracked_files(struct stash_info *info,
struct strbuf *msg,
}
static int stash_patch(struct stash_info *info, struct pathspec ps,
- struct strbuf *out_patch)
+ struct strbuf *out_patch, int quiet)
{
int ret = 0;
struct strbuf out = STRBUF_INIT;
@@ -1026,7 +1026,8 @@ static int stash_patch(struct stash_info *info, struct
pathspec ps,
}
if (!out_patch->len) {
- fprintf_ln(stderr, _("No changes selected"));
+ if (!quiet)
+ fprintf_ln(stderr, _("No changes selected"));
ret = 1;
}
@@ -1105,7 +1106,8 @@ static int stash_working_tree(struct stash_info *info,
struct pathspec ps)
static int do_create_stash(struct pathspec ps, char **stash_msg,
int include_untracked, int patch_mode,
- struct stash_info *info, struct strbuf *patch)
+ struct stash_info *info, struct strbuf *patch,
+ int quiet)
{
int ret = 0;
int flags = 0;
@@ -1124,7 +1126,9 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
refresh_cache(REFRESH_QUIET);
if (get_oid("HEAD", >b_commit)) {
- fprintf_ln(stderr, _("You do not have the initial commit yet"));
+ if (!quiet)
+ fprintf_ln(stderr, _("You do not have "
+"the initial commit yet"));
ret = -1;
*stash_msg = NULL;
goto done;
@@ -1151,7 +1155,9 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
if (write_cache_as_tree(>i_tree, 0, NULL) ||
commit_tree(commit_tree_label.buf, commit_tree_label.len,
>i_tree, parents, >i_commit, NULL, NULL)) {
- fprintf_ln(stderr, _("Cannot save the current index state"));
+ if (!quiet)
+ fprintf_ln(stderr, _("Cannot save the current "
+"index state"));
ret = -1;
*stash_msg = NULL;
goto done;
@@ -1160,8 +1166,9 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
if (include_untracked && get_untracked_files(ps, include_untracked,
_files)) {
if (save_untracked_files(info, , untracked_files)) {
- fprintf_ln(stderr, _("Cannot save "
-"the untracked files"));
+ if (!quiet)
+ fprintf_ln(stderr, _("Cannot save "
+"the untracked files"));
ret = -1;
*stash_msg = NULL;
goto done;
@@ -1169,19 +1176,21 @@ static int do_create_stash(struct pathspec ps, char
**stash_msg,
untracked_commit_option = 1;
}
if (patch_mode) {
- ret = stash_patch(info, ps, patch);
+ ret = stash_patch(info, ps, patch, quiet);
*stash_msg = NULL;
if (ret < 0) {
- fprintf_ln(stderr, _("Cannot save the current "
-"worktree state"));
+ if (!quiet)
+ fprintf_ln(stderr, _("Cannot save the current "
+"worktree state"));
goto done;
} else if (ret > 0) {
goto done;
}
} else {
if (stash_working_tree(info, ps)) {
- fprintf_ln(stderr, _("Cannot save the current "
-"worktree state"));
+ if (!quiet)
+ fprintf_ln(stderr, _("Cannot save the current "
+"worktree state"));
ret = -1;
*stash_msg = NULL;
goto done;
@@ -1210,7 +1219,9 @@ static int do_create_stash(struct pathspec ps, char
[PATCH v10 04/21] t3903: modernize style
Remove whitespaces after redirection operators and wrap
long lines.
Signed-off-by: Paul-Sebastian Ungureanu
---
t/t3903-stash.sh | 120 ---
1 file changed, 61 insertions(+), 59 deletions(-)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index af7586d43d..de6cab1fe7 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -8,22 +8,22 @@ test_description='Test git stash'
. ./test-lib.sh
test_expect_success 'stash some dirty working directory' '
- echo 1 > file &&
+ echo 1 >file &&
git add file &&
echo unrelated >other-file &&
git add other-file &&
test_tick &&
git commit -m initial &&
- echo 2 > file &&
+ echo 2 >file &&
git add file &&
- echo 3 > file &&
+ echo 3 >file &&
test_tick &&
git stash &&
git diff-files --quiet &&
git diff-index --cached --quiet HEAD
'
-cat > expect << EOF
+cat >expect < output &&
+ git diff stash^2..stash >output &&
test_cmp output expect
'
@@ -74,7 +74,7 @@ test_expect_success 'apply stashed changes' '
test_expect_success 'apply stashed changes (including index)' '
git reset --hard HEAD^ &&
- echo 6 > other-file &&
+ echo 6 >other-file &&
git add other-file &&
test_tick &&
git commit -m other-file &&
@@ -99,12 +99,12 @@ test_expect_success 'stash drop complains of extra options'
'
test_expect_success 'drop top stash' '
git reset --hard &&
- git stash list > stashlist1 &&
- echo 7 > file &&
+ git stash list >expected &&
+ echo 7 >file &&
git stash &&
git stash drop &&
- git stash list > stashlist2 &&
- test_cmp stashlist1 stashlist2 &&
+ git stash list >actual &&
+ test_cmp expected actual &&
git stash apply &&
test 3 = $(cat file) &&
test 1 = $(git show :file) &&
@@ -113,9 +113,9 @@ test_expect_success 'drop top stash' '
test_expect_success 'drop middle stash' '
git reset --hard &&
- echo 8 > file &&
+ echo 8 >file &&
git stash &&
- echo 9 > file &&
+ echo 9 >file &&
git stash &&
git stash drop stash@{1} &&
test 2 = $(git stash list | wc -l) &&
@@ -160,7 +160,7 @@ test_expect_success 'stash pop' '
test 0 = $(git stash list | wc -l)
'
-cat > expect << EOF
+cat >expect < expect1 << EOF
+cat >expect1 < expect2 << EOF
+cat >expect2 < file &&
+ echo foo >file &&
git commit file -m first &&
- echo bar > file &&
- echo bar2 > file2 &&
+ echo bar >file &&
+ echo bar2 >file2 &&
git add file2 &&
git stash &&
- echo baz > file &&
+ echo baz >file &&
git commit file -m second &&
git stash branch stashbranch &&
test refs/heads/stashbranch = $(git symbolic-ref HEAD) &&
test $(git rev-parse HEAD) = $(git rev-parse master^) &&
- git diff --cached > output &&
+ git diff --cached >output &&
test_cmp output expect &&
- git diff > output &&
+ git diff >output &&
test_cmp output expect1 &&
git add file &&
git commit -m alternate\ second &&
- git diff master..stashbranch > output &&
+ git diff master..stashbranch >output &&
test_cmp output expect2 &&
test 0 = $(git stash list | wc -l)
'
test_expect_success 'apply -q is quiet' '
- echo foo > file &&
+ echo foo >file &&
git stash &&
- git stash apply -q > output.out 2>&1 &&
+ git stash apply -q >output.out 2>&1 &&
test_must_be_empty output.out
'
test_expect_success 'save -q is quiet' '
- git stash save --quiet > output.out 2>&1 &&
+ git stash save --quiet >output.out 2>&1 &&
test_must_be_empty output.out
'
test_expect_success 'pop -q is quiet' '
- git stash pop -q > output.out 2>&1 &&
+ git stash pop -q >output.out 2>&1 &&
test_must_be_empty output.out
'
test_expect_success 'pop -q --index works and is quiet' '
- echo foo > file &&
+ echo foo >file &&
git add file &&
git stash save --quiet &&
- git stash pop -q --index > output.out 2>&1 &&
+ git stash pop -q --index >output.out 2>&1 &&
test foo = "$(git show :file)" &&
test_must_be_empty output.out
'
test_expect_success 'drop -q is quiet' '
git stash &&
- git stash drop -q > output.out 2>&1 &&
+ git stash drop -q >output.out 2>&1 &&
test_must_be_empty output.out
'
test_expect_success 'stash -k' '
- echo bar3 > file &&
- echo bar4 > file2 &&
+ echo bar3 >file &&
+ echo bar4 >file2 &&
git add file2 &&
git stash -k &&
test bar,bar4 = $(cat file),$(cat file2)
'
test_expect_success 'stash --no-keep-index' '
- echo bar33 > file &&
- echo bar44 > file2 &&
+ echo bar33
[PATCH v10 11/21] stash: convert pop to builtin
From: Joel Teichroeb
Add stash pop to the helper and delete the pop_stash, drop_stash,
assert_stash_ref functions from the shell script now that they
are no longer needed.
Signed-off-by: Joel Teichroeb
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 39 +-
git-stash.sh| 47 ++---
2 files changed, 40 insertions(+), 46 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index c41aad7036..33f4e83353 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -13,7 +13,7 @@
static const char * const git_stash_helper_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
- N_("git stash--helper apply [--index] [-q|--quiet] []"),
+ N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet]
[]"),
N_("git stash--helper branch []"),
N_("git stash--helper clear"),
NULL
@@ -24,6 +24,11 @@ static const char * const git_stash_helper_drop_usage[] = {
NULL
};
+static const char * const git_stash_helper_pop_usage[] = {
+ N_("git stash--helper pop [--index] [-q|--quiet] []"),
+ NULL
+};
+
static const char * const git_stash_helper_apply_usage[] = {
N_("git stash--helper apply [--index] [-q|--quiet] []"),
NULL
@@ -545,6 +550,36 @@ static int drop_stash(int argc, const char **argv, const
char *prefix)
return ret;
}
+static int pop_stash(int argc, const char **argv, const char *prefix)
+{
+ int ret;
+ int index = 0;
+ int quiet = 0;
+ struct stash_info info;
+ struct option options[] = {
+ OPT__QUIET(, N_("be quiet, only report errors")),
+ OPT_BOOL(0, "index", ,
+N_("attempt to recreate the index")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_stash_helper_pop_usage, 0);
+
+ if (get_stash_info(, argc, argv))
+ return -1;
+
+ assert_stash_ref();
+ if ((ret = do_apply_stash(prefix, , index, quiet)))
+ printf_ln(_("The stash entry is kept in case "
+ "you need it again."));
+ else
+ ret = do_drop_stash(prefix, , quiet);
+
+ free_stash_info();
+ return ret;
+}
+
static int branch_stash(int argc, const char **argv, const char *prefix)
{
int ret;
@@ -609,6 +644,8 @@ int cmd_stash__helper(int argc, const char **argv, const
char *prefix)
return !!clear_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "drop"))
return !!drop_stash(argc, argv, prefix);
+ else if (!strcmp(argv[0], "pop"))
+ return !!pop_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "branch"))
return !!branch_stash(argc, argv, prefix);
diff --git a/git-stash.sh b/git-stash.sh
index 29d9f44255..8f2640fe90 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -554,50 +554,6 @@ assert_stash_like() {
}
}
-is_stash_ref() {
- is_stash_like "$@" && test -n "$IS_STASH_REF"
-}
-
-assert_stash_ref() {
- is_stash_ref "$@" || {
- args="$*"
- die "$(eval_gettext "'\$args' is not a stash reference")"
- }
-}
-
-apply_stash () {
- cd "$START_DIR"
- git stash--helper apply "$@"
- res=$?
- cd_to_toplevel
- return $res
-}
-
-pop_stash() {
- assert_stash_ref "$@"
-
- if apply_stash "$@"
- then
- drop_stash "$@"
- else
- status=$?
- say "$(gettext "The stash entry is kept in case you need it
again.")"
- exit $status
- fi
-}
-
-drop_stash () {
- assert_stash_ref "$@"
-
- git reflog delete --updateref --rewrite "${REV}" &&
- say "$(eval_gettext "Dropped \${REV} (\$s)")" ||
- die "$(eval_gettext "\${REV}: Could not drop stash entry")"
-
- # clear_stash if we just dropped the last stash entry
- git rev-parse --verify --quiet "$ref_stash@{0}" >/dev/null ||
- clear_stash
-}
-
test "$1" = "-p" && set "push" "$@"
PARSE_CACHE='--not-parsed'
@@ -655,7 +611,8 @@ drop)
;;
pop)
shift
- pop_stash "$@"
+ cd "$START_DIR"
+ git stash--helper pop "$@"
;;
branch)
shift
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 14/21] stash: convert store to builtin
Add stash store to the helper and delete the store_stash function
from the shell script.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 62 +
git-stash.sh| 43 ++--
2 files changed, 64 insertions(+), 41 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 3f7ace92f5..758a32572d 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -58,6 +58,11 @@ static const char * const git_stash_helper_clear_usage[] = {
NULL
};
+static const char * const git_stash_helper_store_usage[] = {
+ N_("git stash--helper store [-m|--message ] [-q|--quiet]
"),
+ NULL
+};
+
static const char *ref_stash = "refs/stash";
static struct strbuf stash_index_path = STRBUF_INIT;
@@ -732,6 +737,61 @@ static int show_stash(int argc, const char **argv, const
char *prefix)
return diff_result_code(, 0);
}
+static int do_store_stash(const struct object_id *w_commit, const char
*stash_msg,
+ int quiet)
+{
+ if (!stash_msg)
+ stash_msg = "Created via \"git stash store\".";
+
+ if (update_ref(stash_msg, ref_stash, w_commit, NULL,
+ REF_FORCE_CREATE_REFLOG,
+ quiet ? UPDATE_REFS_QUIET_ON_ERR :
+ UPDATE_REFS_MSG_ON_ERR)) {
+ if (!quiet) {
+ fprintf_ln(stderr, _("Cannot update %s with %s"),
+ ref_stash, oid_to_hex(w_commit));
+ }
+ return -1;
+ }
+
+ return 0;
+}
+
+static int store_stash(int argc, const char **argv, const char *prefix)
+{
+ int quiet = 0;
+ const char *stash_msg = NULL;
+ struct object_id obj;
+ struct object_context dummy;
+ struct option options[] = {
+ OPT__QUIET(, N_("be quiet")),
+ OPT_STRING('m', "message", _msg, "message",
+ N_("stash message")),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_stash_helper_store_usage,
+PARSE_OPT_KEEP_UNKNOWN);
+
+ if (argc != 1) {
+ if (!quiet)
+ fprintf_ln(stderr, _("\"git stash store\" requires one "
+" argument"));
+ return -1;
+ }
+
+ if (get_oid_with_context(argv[0], quiet ? GET_OID_QUIETLY : 0, ,
+)) {
+ if (!quiet)
+ fprintf_ln(stderr, _("Cannot update %s with %s"),
+ref_stash, argv[0]);
+ return -1;
+ }
+
+ return do_store_stash(, stash_msg, quiet);
+}
+
int cmd_stash__helper(int argc, const char **argv, const char *prefix)
{
pid_t pid = getpid();
@@ -766,6 +826,8 @@ int cmd_stash__helper(int argc, const char **argv, const
char *prefix)
return !!list_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "show"))
return !!show_stash(argc, argv, prefix);
+ else if (!strcmp(argv[0], "store"))
+ return !!store_stash(argc, argv, prefix);
usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index 0d05cbc1e5..5739c51527 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -191,45 +191,6 @@ create_stash () {
die "$(gettext "Cannot record working tree state")"
}
-store_stash () {
- while test $# != 0
- do
- case "$1" in
- -m|--message)
- shift
- stash_msg="$1"
- ;;
- -m*)
- stash_msg=${1#-m}
- ;;
- --message=*)
- stash_msg=${1#--message=}
- ;;
- -q|--quiet)
- quiet=t
- ;;
- *)
- break
- ;;
- esac
- shift
- done
- test $# = 1 ||
- die "$(eval_gettext "\"$dashless store\" requires one
argument")"
-
- w_commit="$1"
- if test -z "$stash_msg"
- then
- stash_msg="Created via \"git stash store\"."
- fi
-
- git update-ref --create-reflog -m "$stash_msg" $ref_stash $w_commit
- ret=$?
- test $ret != 0 && test -z "$quiet" &&
- die "$(eval_gettext "Cannot update \$ref_stash with \$w_commit")"
- return $ret
-}
-
push_stash () {
keep_index=
patch_mode=
@@ -308,7 +269,7 @@ push_stash () {
clear_stash || die "$(gettext "Cannot initialize stash")"
create_stash -m "$stash_msg" -u "$untracked" --
[PATCH v10 08/21] stash: convert apply to builtin
From: Joel Teichroeb
Add a builtin helper for performing stash commands. Converting
all at once proved hard to review, so starting with just apply
lets conversion get started without the other commands being
finished.
The helper is being implemented as a drop in replacement for
stash so that when it is complete it can simply be renamed and
the shell script deleted.
Delete the contents of the apply_stash shell function and replace
it with a call to stash--helper apply until pop is also
converted.
Signed-off-by: Joel Teichroeb
Signed-off-by: Paul-Sebastian Ungureanu
---
.gitignore | 1 +
Makefile| 1 +
builtin.h | 1 +
builtin/stash--helper.c | 454
git-stash.sh| 78 +--
git.c | 1 +
6 files changed, 465 insertions(+), 71 deletions(-)
create mode 100644 builtin/stash--helper.c
diff --git a/.gitignore b/.gitignore
index ffceea7d59..b59661cb88 100644
--- a/.gitignore
+++ b/.gitignore
@@ -157,6 +157,7 @@
/git-show-ref
/git-stage
/git-stash
+/git-stash--helper
/git-status
/git-stripspace
/git-submodule
diff --git a/Makefile b/Makefile
index d03df31c2a..f900c68e69 100644
--- a/Makefile
+++ b/Makefile
@@ -1093,6 +1093,7 @@ BUILTIN_OBJS += builtin/shortlog.o
BUILTIN_OBJS += builtin/show-branch.o
BUILTIN_OBJS += builtin/show-index.o
BUILTIN_OBJS += builtin/show-ref.o
+BUILTIN_OBJS += builtin/stash--helper.o
BUILTIN_OBJS += builtin/stripspace.o
BUILTIN_OBJS += builtin/submodule--helper.o
BUILTIN_OBJS += builtin/symbolic-ref.o
diff --git a/builtin.h b/builtin.h
index 99206df4bd..317bc338f7 100644
--- a/builtin.h
+++ b/builtin.h
@@ -223,6 +223,7 @@ extern int cmd_show(int argc, const char **argv, const char
*prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
extern int cmd_show_index(int argc, const char **argv, const char *prefix);
extern int cmd_status(int argc, const char **argv, const char *prefix);
+extern int cmd_stash__helper(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_submodule__helper(int argc, const char **argv, const char
*prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
new file mode 100644
index 00..c43e0aacbb
--- /dev/null
+++ b/builtin/stash--helper.c
@@ -0,0 +1,454 @@
+#include "builtin.h"
+#include "config.h"
+#include "parse-options.h"
+#include "refs.h"
+#include "lockfile.h"
+#include "cache-tree.h"
+#include "unpack-trees.h"
+#include "merge-recursive.h"
+#include "argv-array.h"
+#include "run-command.h"
+#include "dir.h"
+#include "rerere.h"
+
+static const char * const git_stash_helper_usage[] = {
+ N_("git stash--helper apply [--index] [-q|--quiet] []"),
+ NULL
+};
+
+static const char * const git_stash_helper_apply_usage[] = {
+ N_("git stash--helper apply [--index] [-q|--quiet] []"),
+ NULL
+};
+
+static const char *ref_stash = "refs/stash";
+static struct strbuf stash_index_path = STRBUF_INIT;
+
+/*
+ * w_commit is set to the commit containing the working tree
+ * b_commit is set to the base commit
+ * i_commit is set to the commit containing the index tree
+ * u_commit is set to the commit containing the untracked files tree
+ * w_tree is set to the working tree
+ * b_tree is set to the base tree
+ * i_tree is set to the index tree
+ * u_tree is set to the untracked files tree
+ */
+
+struct stash_info {
+ struct object_id w_commit;
+ struct object_id b_commit;
+ struct object_id i_commit;
+ struct object_id u_commit;
+ struct object_id w_tree;
+ struct object_id b_tree;
+ struct object_id i_tree;
+ struct object_id u_tree;
+ struct strbuf revision;
+ int is_stash_ref;
+ int has_u;
+};
+
+static void free_stash_info(struct stash_info *info)
+{
+ strbuf_release(>revision);
+}
+
+static void assert_stash_like(struct stash_info *info, const char *revision)
+{
+ if (get_oidf(>b_commit, "%s^1", revision) ||
+ get_oidf(>w_tree, "%s:", revision) ||
+ get_oidf(>b_tree, "%s^1:", revision) ||
+ get_oidf(>i_tree, "%s^2:", revision)) {
+ error(_("'%s' is not a stash-like commit"), revision);
+ free_stash_info(info);
+ exit(128);
+ }
+}
+
+static int get_stash_info(struct stash_info *info, int argc, const char **argv)
+{
+ int ret;
+ char *end_of_rev;
+ char *expanded_ref;
+ const char *revision;
+ const char *commit = NULL;
+ struct object_id dummy;
+ struct strbuf symbolic = STRBUF_INIT;
+
+ if (argc > 1) {
+ int i;
+ struct strbuf refs_msg = STRBUF_INIT;
+ for (i = 0; i < argc; i++)
+ strbuf_addf(_msg, " '%s'", argv[i]);
+
[PATCH v10 01/21] sha1-name.c: add `get_oidf()` which acts like `get_oid()`
Compared to `get_oid()`, `get_oidf()` has as parameters
a pointer to `object_id`, a printf format string and
additional arguments. This will help simplify the code
in subsequent commits.
Original-idea-by: Johannes Schindelin
Signed-off-by: Paul-Sebastian Ungureanu
---
cache.h | 1 +
sha1-name.c | 19 +++
2 files changed, 20 insertions(+)
diff --git a/cache.h b/cache.h
index b1fd3d58ab..d93b2e25a5 100644
--- a/cache.h
+++ b/cache.h
@@ -1309,6 +1309,7 @@ struct object_context {
GET_OID_BLOB)
extern int get_oid(const char *str, struct object_id *oid);
+extern int get_oidf(struct object_id *oid, const char *fmt, ...);
extern int get_oid_commit(const char *str, struct object_id *oid);
extern int get_oid_committish(const char *str, struct object_id *oid);
extern int get_oid_tree(const char *str, struct object_id *oid);
diff --git a/sha1-name.c b/sha1-name.c
index c9cc1318b7..261b960bbd 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -1471,6 +1471,25 @@ int get_oid(const char *name, struct object_id *oid)
return get_oid_with_context(name, 0, oid, );
}
+/*
+ * This returns a non-zero value if the string (built using printf
+ * format and the given arguments) is not a valid object.
+ */
+int get_oidf(struct object_id *oid, const char *fmt, ...)
+{
+ va_list ap;
+ int ret;
+ struct strbuf sb = STRBUF_INIT;
+
+ va_start(ap, fmt);
+ strbuf_vaddf(, fmt, ap);
+ va_end(ap);
+
+ ret = get_oid(sb.buf, oid);
+ strbuf_release();
+
+ return ret;
+}
/*
* Many callers know that the user meant to name a commit-ish by
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 12/21] stash: convert list to builtin
Add stash list to the helper and delete the list_stash function
from the shell script.
Signed-off-by: Paul-Sebastian Ungureanu
---
builtin/stash--helper.c | 31 +++
git-stash.sh| 7 +--
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
index 33f4e83353..a1249f6b76 100644
--- a/builtin/stash--helper.c
+++ b/builtin/stash--helper.c
@@ -12,6 +12,7 @@
#include "rerere.h"
static const char * const git_stash_helper_usage[] = {
+ N_("git stash--helper list []"),
N_("git stash--helper drop [-q|--quiet] []"),
N_("git stash--helper ( pop | apply ) [--index] [-q|--quiet]
[]"),
N_("git stash--helper branch []"),
@@ -19,6 +20,11 @@ static const char * const git_stash_helper_usage[] = {
NULL
};
+static const char * const git_stash_helper_list_usage[] = {
+ N_("git stash--helper list []"),
+ NULL
+};
+
static const char * const git_stash_helper_drop_usage[] = {
N_("git stash--helper drop [-q|--quiet] []"),
NULL
@@ -618,6 +624,29 @@ static int branch_stash(int argc, const char **argv, const
char *prefix)
return ret;
}
+static int list_stash(int argc, const char **argv, const char *prefix)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ struct option options[] = {
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, prefix, options,
+git_stash_helper_list_usage,
+PARSE_OPT_KEEP_UNKNOWN);
+
+ if (!ref_exists(ref_stash))
+ return 0;
+
+ cp.git_cmd = 1;
+ argv_array_pushl(, "log", "--format=%gd: %gs", "-g",
+"--first-parent", "-m", NULL);
+ argv_array_pushv(, argv);
+ argv_array_push(, ref_stash);
+ argv_array_push(, "--");
+ return run_command();
+}
+
int cmd_stash__helper(int argc, const char **argv, const char *prefix)
{
pid_t pid = getpid();
@@ -648,6 +677,8 @@ int cmd_stash__helper(int argc, const char **argv, const
char *prefix)
return !!pop_stash(argc, argv, prefix);
else if (!strcmp(argv[0], "branch"))
return !!branch_stash(argc, argv, prefix);
+ else if (!strcmp(argv[0], "list"))
+ return !!list_stash(argc, argv, prefix);
usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
git_stash_helper_usage, options);
diff --git a/git-stash.sh b/git-stash.sh
index 8f2640fe90..6052441aa2 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -382,11 +382,6 @@ have_stash () {
git rev-parse --verify --quiet $ref_stash >/dev/null
}
-list_stash () {
- have_stash || return 0
- git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --
-}
-
show_stash () {
ALLOW_UNKNOWN_FLAGS=t
assert_stash_like "$@"
@@ -574,7 +569,7 @@ test -n "$seen_non_option" || set "push" "$@"
case "$1" in
list)
shift
- list_stash "$@"
+ git stash--helper list "$@"
;;
show)
shift
--
2.19.0.rc0.23.g10a62394e7
[PATCH v10 03/21] stash: improve option parsing test coverage
From: Joel Teichroeb
In preparation for converting the stash command incrementally to
a builtin command, this patch improves test coverage of the option
parsing. Both for having too many parameters, or too few.
Signed-off-by: Joel Teichroeb
Signed-off-by: Paul-Sebastian Ungureanu
---
t/t3903-stash.sh | 35 +++
1 file changed, 35 insertions(+)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 1f871d3cca..af7586d43d 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -444,6 +444,36 @@ test_expect_failure 'stash file to directory' '
test foo = "$(cat file/file)"
'
+test_expect_success 'giving too many ref arguments does not modify files' '
+ git stash clear &&
+ test_when_finished "git reset --hard HEAD" &&
+ echo foo >file2 &&
+ git stash &&
+ echo bar >file2 &&
+ git stash &&
+ test-tool chmtime =123456789 file2 &&
+ for type in apply pop "branch stash-branch"
+ do
+ test_must_fail git stash $type stash@{0} stash@{1} 2>err &&
+ test_i18ngrep "Too many revisions" err &&
+ test 123456789 = $(test-tool chmtime -g file2) || return 1
+ done
+'
+
+test_expect_success 'drop: too many arguments errors out (does nothing)' '
+ git stash list >expect &&
+ test_must_fail git stash drop stash@{0} stash@{1} 2>err &&
+ test_i18ngrep "Too many revisions" err &&
+ git stash list >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'show: too many arguments errors out (does nothing)' '
+ test_must_fail git stash show stash@{0} stash@{1} 2>err 1>out &&
+ test_i18ngrep "Too many revisions" err &&
+ test_must_be_empty out
+'
+
test_expect_success 'stash create - no changes' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
@@ -479,6 +509,11 @@ test_expect_success 'stash branch - stashes on stack,
stash-like argument' '
test $(git ls-files --modified | wc -l) -eq 1
'
+test_expect_success 'stash branch complains with no arguments' '
+ test_must_fail git stash branch 2>err &&
+ test_i18ngrep "No branch name specified" err
+'
+
test_expect_success 'stash show format defaults to --stat' '
git stash clear &&
test_when_finished "git reset --hard HEAD" &&
--
2.19.0.rc0.23.g10a62394e7
[RFC/PATCH] headers: normalize the spelling of some header guards
Signed-off-by: Ramsay Jones --- Hi Junio, This patch is marked RFC because I am not aware of any policy with regard to header guard spelling. Having said that, apart from the fetch-negotiator.h header, all of these headers are using a reserved identifier (see C99 Standard 7.1.3). These headers were found, thus: $ git grep -n -E '^#ifn?def ' -- '*.h' | grep 'h\:1\:' | grep -v '^compat' | grep -v -E '[A-Z_]*_H$' alias.h:1:#ifndef __ALIAS_H__ commit-reach.h:1:#ifndef __COMMIT_REACH_H__ fetch-negotiator.h:1:#ifndef FETCH_NEGOTIATOR midx.h:1:#ifndef __MIDX_H__ t/helper/test-tool.h:1:#ifndef __TEST_TOOL_H__ vcs-svn/fast_export.h:1:#ifndef FAST_EXPORT_H_ vcs-svn/line_buffer.h:1:#ifndef LINE_BUFFER_H_ vcs-svn/sliding_window.h:1:#ifndef SLIDING_WINDOW_H_ vcs-svn/svndiff.h:1:#ifndef SVNDIFF_H_ vcs-svn/svndump.h:1:#ifndef SVNDUMP_H_ $ Note that I haven't included the headers in vcs-svn because there is a patch being discussed which would move that directory to contrib. ATB, Ramsay Jones alias.h | 4 ++-- commit-reach.h | 4 ++-- fetch-negotiator.h | 4 ++-- midx.h | 4 ++-- t/helper/test-tool.h | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/alias.h b/alias.h index 79933f2457..aef4843bb7 100644 --- a/alias.h +++ b/alias.h @@ -1,5 +1,5 @@ -#ifndef __ALIAS_H__ -#define __ALIAS_H__ +#ifndef ALIAS_H +#define ALIAS_H struct string_list; diff --git a/commit-reach.h b/commit-reach.h index 7d313e2975..122a23a24d 100644 --- a/commit-reach.h +++ b/commit-reach.h @@ -1,5 +1,5 @@ -#ifndef __COMMIT_REACH_H__ -#define __COMMIT_REACH_H__ +#ifndef COMMIT_REACH_H +#define COMMIT_REACH_H #include "commit-slab.h" diff --git a/fetch-negotiator.h b/fetch-negotiator.h index ddb44a22dc..9e3967ce66 100644 --- a/fetch-negotiator.h +++ b/fetch-negotiator.h @@ -1,5 +1,5 @@ -#ifndef FETCH_NEGOTIATOR -#define FETCH_NEGOTIATOR +#ifndef FETCH_NEGOTIATOR_H +#define FETCH_NEGOTIATOR_H struct commit; diff --git a/midx.h b/midx.h index ce80b91c68..ee83702309 100644 --- a/midx.h +++ b/midx.h @@ -1,5 +1,5 @@ -#ifndef __MIDX_H__ -#define __MIDX_H__ +#ifndef MIDX_H +#define MIDX_H #include "repository.h" diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index e4890566da..71f470b871 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -1,5 +1,5 @@ -#ifndef __TEST_TOOL_H__ -#define __TEST_TOOL_H__ +#ifndef TEST_TOOL_H +#define TEST_TOOL_H #include "git-compat-util.h" -- 2.19.0
[PATCH] Typo `dashes ?` -> `dashes?`
---
parse-options.c | 4 ++--
t/t0040-parse-options.sh | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 3b874a83a0c89..88512212392ae 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -352,7 +352,7 @@ static void check_typos(const char *arg, const struct
option *options)
return;
if (starts_with(arg, "no-")) {
- error ("did you mean `--%s` (with two dashes ?)", arg);
+ error ("did you mean `--%s` (with two dashes?)", arg);
exit(129);
}
@@ -360,7 +360,7 @@ static void check_typos(const char *arg, const struct
option *options)
if (!options->long_name)
continue;
if (starts_with(options->long_name, arg)) {
- error ("did you mean `--%s` (with two dashes ?)", arg);
+ error ("did you mean `--%s` (with two dashes?)", arg);
exit(129);
}
}
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 5b0560fa20e34..16fd333bd3895 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -222,7 +222,7 @@ test_expect_success 'non ambiguous option (after two
options it abbreviates)' '
'
cat >typo.err <<\EOF
-error: did you mean `--boolean` (with two dashes ?)
+error: did you mean `--boolean` (with two dashes?)
EOF
test_expect_success 'detect possible typos' '
@@ -232,7 +232,7 @@ test_expect_success 'detect possible typos' '
'
cat >typo.err <<\EOF
-error: did you mean `--ambiguous` (with two dashes ?)
+error: did you mean `--ambiguous` (with two dashes?)
EOF
test_expect_success 'detect possible typos' '
--
https://github.com/git/git/pull/540
Re: [PATCH v2 1/1] zlib.c: use size_t for size
On 14/10/18 03:52, Jeff King wrote:
> On Sun, Oct 14, 2018 at 03:16:36AM +0100, Ramsay Jones wrote:
>
>> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
>> index b059b86aee..3b5f2c38b3 100644
>> --- a/builtin/pack-objects.c
>> +++ b/builtin/pack-objects.c
>> @@ -269,12 +269,12 @@ static void copy_pack_data(struct hashfile *f,
>> off_t len)
>> {
>> unsigned char *in;
>> -unsigned long avail;
>> +size_t avail;
>>
>> while (len) {
>> in = use_pack(p, w_curs, offset, );
>> if (avail > len)
>> -avail = (unsigned long)len;
>> +avail = xsize_t(len);
>
> We don't actually care about truncation here. The idea is that we take a
> bite-sized chunk via use_pack, and loop as necessary. So mod-2^32
> truncation via a cast would be bad (we might not make forward progress),
> but truncating to SIZE_MAX would be fine.
>
> That said, we know that would not truncate here, because we must
> strictly be shrinking "avail", which was already a size_t (unless "len"
> is negative, but then we are really screwed ;) ).
>
> So I kind of wonder if a comment would be better than xsize_t here.
> Something like:
>
> if (avail > len) {
> /*
>* This can never truncate because we know that len is smaller
>* than avail, which is already a size_t.
>*/
> avail = (size_t)len;
> }
Heh, you are, of course, correct! (that will learn me[1]). :-D
Hmm, let's see if I can muster the enthusiasm to do all that
testing again!
ATB,
Ramsay Jones
[1] Since I started with my patch, when I had finished 'paring
it back', the result didn't have this xsize_t() call. In order
to make the result 'v2 + SZEDER's patch' (which I thought was
quite neat) I added this call right at the end. :-P
Re: [PATCH 15/16] commit-reach: make can_all_from_reach... linear
Am 05.10.2018 um 21:42 schrieb Jeff King:
> On Fri, Oct 05, 2018 at 09:36:28PM +0200, René Scharfe wrote:
>
>> Am 05.10.2018 um 21:08 schrieb Jeff King:
>>> On Fri, Oct 05, 2018 at 08:48:27PM +0200, René Scharfe wrote:
+#define DEFINE_SORT(name, type, compare) \
+static int compare##_void(const void *one, const void *two)
\
+{ \
+ return compare(one, two); \
+} \
+static void name(type base, size_t nmemb) \
+{ \
+ const type dummy = NULL;\
+ if (nmemb > 1) \
+ qsort(base, nmemb, sizeof(base[0]), compare##_void);\
+ else if (0) \
+ compare(dummy, dummy); \
+}
>>>
>>> I do like that this removes the need to have the code block aspart of
>>> the macro.
>>>
>>> Did you measure to see if there is any runtime impact?
>>
>> No, but I wouldn't expect any -- the generated code should be the same
>> in most cases.
>>
>> Here's an example: https://godbolt.org/z/gwXENy.
>
> OK, that's good enough for me.
>
>> The typed comparison function can be inlined into the one with the void
>> pointers, though.
>
> Right, that makes sense. I suspect it depends on the comparison function
> being static, but in a DEFINE_SORT() world, they generally could be.
>
> So I like this approach.
It still has some repetition, converted code is a bit longer than the
current one, and I don't know how to build a Coccinelle rule that would
do that conversion.
Looked for a possibility to at least leave QSORT call-sites alone by
enhancing that macro, but didn't find any. Found a few websites
showing off mindblowing macros, thouhg, this one in particular:
https://github.com/pfultz2/Cloak/wiki/C-Preprocessor-tricks,-tips,-and-idioms
Anyway, drove the generative approach a bit further, and came up with
the new DEFINE_SORT below. I'm unsure about the name; perhaps it should
be called DEFINE_SORT_BY_COMPARE_FUNCTION_BODY, but that's a bit long.
It handles casts and const attributes behind the scenes and avoids
repetition, but looks a bit weird, as it is placed where a function
signature would go.
Apart from that the macro is simple and doesn't use any tricks or
added checks. It just sets up boilerplate functions to offer type-safe
sorting.
diffcore-rename.c and refs/packed-backend.c receive special treatment in
the patch because their compare functions are used outside of sorting as
well. I made them take typed pointers nevertheless and used them from
DEFINE_SORT; the wrapper generated by that macro is supposed to be
private. Given that such reuse is rare and I think we don't need a way
to make it public.
What do y'all think about this direction?
---
bisect.c| 8 ++--
builtin/describe.c | 6 ++
builtin/fmt-merge-msg.c | 10 --
builtin/index-pack.c| 14 --
builtin/name-rev.c | 5 ++---
builtin/pack-objects.c | 7 ++-
builtin/remote.c| 6 ++
builtin/shortlog.c | 15 ---
commit-graph.c | 6 ++
delta-islands.c | 8 +++-
diff.c | 8 +++-
diffcore-delta.c| 7 ++-
diffcore-order.c| 7 ++-
diffcore-rename.c | 11 +++
git-compat-util.h | 15 +++
help.c | 7 ++-
line-log.c | 7 ++-
midx.c | 12
pack-check.c| 6 ++
pathspec.c | 8 ++--
refs/packed-backend.c | 11 ---
sha1-array.c| 4 ++--
sha1-name.c | 4 ++--
23 files changed, 84 insertions(+), 108 deletions(-)
diff --git a/bisect.c b/bisect.c
index e8b17cf7e1..25257c2e69 100644
--- a/bisect.c
+++ b/bisect.c
@@ -192,12 +192,8 @@ struct commit_dist {
int distance;
};
-static int compare_commit_dist(const void *a_, const void *b_)
+DEFINE_SORT(static, sort_by_commit_dist, struct commit_dist *, a, b)
{
- struct commit_dist *a, *b;
-
- a = (struct commit_dist *)a_;
- b = (struct commit_dist *)b_;
if (a->distance != b->distance)
return b->distance - a->distance; /* desc sort */
return oidcmp(>commit->object.oid, >commit->object.oid);
@@ -223,7 +219,7 @@ static struct commit_list *best_bisection_sorted(struct
commit_list *list, int n
array[cnt].distance = distance;
cnt++;
}
- QSORT(array, cnt, compare_commit_dist);
+ sort_by_commit_dist(array, cnt);
for (p = list, i = 0; i < cnt; i++)
Re: [PATCH v5 17/23] userdiff.c: remove implicit dependency on the_index
On Wed, Oct 10, 2018 at 4:51 PM Jeff King wrote: > > On Fri, Sep 21, 2018 at 05:57:33PM +0200, Nguyễn Thái Ngọc Duy wrote: > > > diff --git a/diff.c b/diff.c > > index 140d0e86df..5256b9eabc 100644 > > --- a/diff.c > > +++ b/diff.c > > @@ -2093,23 +2093,25 @@ static void diff_words_flush(struct emit_callback > > *ecbdata) > > } > > } > > > > -static void diff_filespec_load_driver(struct diff_filespec *one) > > +static void diff_filespec_load_driver(struct diff_filespec *one, > > + struct index_state *istate) > > I hadn't looked at this topic until today, when I tried merging next > with some of my other local bits and ran into conflicts. I found the > idea of passing index_state here, rather than the whole "struct > repository", a bit curious. > > I get why you're doing it: your topic here only cares about removing > index dependencies, so you did the minimal thing to move that forward. > > But if you think about what this function is doing, it is quite clearly > dependent on the whole repository, since the userdiff config we're > looking up may come from repo config. > > So I think in the long run this probably should take a repository > argument, and use r->index instead of a separate istate argument. That > said, I'm not totally opposed to taking this incremental step and > letting somebody later sort out the config portions. I wonder if it > would be worth calling that out in the commit message to help that later > person. But I guess it is a bit late if this is already in next. Maybe. When I made this series, I tried to reduce the access scope as much as possible. If it turns out it needs more, we can always turn "struct index_state *" into "struct repository *" later, -- Duy
Re: [PATCH v8 0/7] speed up index load through parallelization
On Wed, Oct 10, 2018 at 5:59 PM Ben Peart wrote:
> @@ -3460,14 +3479,18 @@ static struct index_entry_offset_table
> *read_ieot_extension(const char *mmap, si
>
> /* validate the version is IEOT_VERSION */
> ext_version = get_be32(index);
> - if (ext_version != IEOT_VERSION)
> + if (ext_version != IEOT_VERSION) {
> + error("invalid IEOT version %d", ext_version);
Please wrap this string in _() so that it can be translated.
>return NULL;
> + }
> index += sizeof(uint32_t);
>
> /* extension size - version bytes / bytes per entry */
> nr = (extsize - sizeof(uint32_t)) / (sizeof(uint32_t) +
> sizeof(uint32_t));
> - if (!nr)
> + if (!nr) {
> + error("invalid number of IEOT entries %d", nr);
Ditto. And reporting extsize may be more useful than nr, which we know
is zero, but we don't know why it's calculated zero unless we know
extsize.
--
Duy
Re: What's cooking in git.git (Oct 2018, #01; Wed, 10)
On Wed, Oct 10, 2018 at 7:43 AM Junio C Hamano wrote: > * nd/per-worktree-ref-iteration (2018-10-07) 9 commits > - SQUASH??? > - reflog expire: cover reflog from all worktrees > - fsck: check HEAD and reflog from other worktrees > - fsck: Move fsck_head_link() to get_default_heads() to avoid some globals > - revision.c: better error reporting on ref from different worktrees > - revision.c: correct a parameter name > - refs: new ref types to make per-worktree refs visible to all worktrees > - Add a place for (not) sharing stuff between worktrees > - refs.c: indent with tabs, not spaces > > What's the status of this topic? There's one bug spotted by Eric and a few test cases to be added in the reflog patch. -- Duy
Re: [BUG] gitignore documentation inconsistent with actual behaviour
On Thu, Oct 11, 2018 at 2:41 PM Ævar Arnfjörð Bjarmason wrote: > > > On Thu, Oct 11 2018, dana wrote: > > > Hello, > > > > I'm a contributor to ripgrep, which is a grep-like tool that supports using > > gitignore files to control which files are searched in a repo (or any other > > directory tree). ripgrep's support for the patterns in these files is based > > on > > git's official documentation, as seen here: > > > > https://git-scm.com/docs/gitignore > > > > One of the most common reports on the ripgrep bug tracker is that it does > > not > > allow patterns like the following real-world examples, where a ** is used > > along > > with other text within the same path component: > > > > **/**$$*.java > > **.orig > > **local.properties > > !**.sha1 > > > > The reason it doesn't allow them is that the gitignore documentation > > explicitly > > states that they're invalid: > > > > A leading "**" followed by a slash means match in all directories... > > > > A trailing "/**" matches everything inside... > > > > A slash followed by two consecutive asterisks then a slash matches zero or > > more directories... > > > > Other consecutive asterisks are considered invalid. Perhaps "undefined" is a better word than "invalid". > > git itself happily accepts these patterns, however, apparently treating the > > ** > > like a single * without fnmatch(3)'s FNM_PATHNAME flag set (in other words, > > it > > matches / as a regular character, thus crossing directory boundaries). > > > > ripgrep's developer is loathe to reverse-engineer this undocumented > > behaviour, > > and so the reports keep coming, both for ripgrep itself and for down-stream > > consumers of it and its ignore crate (including most notably Microsoft's VS > > Code > > editor). > > > > My request: Assuming that git's actual handling of these patterns is > > intended, > > would it be possible to make it 'official' and explicitly add it to the > > documentation? You mean "**" in the fourth case is interpreted as "*"? Yes I guess we could rephrase it as "Other consecutive asterisks are considered normal wildcard asterisks" > Yeah those docs seem wrong. In general the docs for the matching > function are quite bad. I have on my TODO list to factor this out into > some gitwildmatch manpage, but right now the bit in gitignore is all we > have. > > Our matching function comes from rsync originally, whose manpage says: > > use ’**’ to match anything, including slashes. > > I believe this is accurate as far as the implementation goes. No. "**" semantics is not the same as from rsync. The three cases "**/", "/**/" and "/**" were requested by Junio if I remember correctly. You can search the mail archive for more information. -- Duy
Re: [Question] builtin/branch.c
On Sat, Oct 13, 2018 at 10:12 AM Tao Qingyun wrote:
> Hi, I am learning `builtin/branch.c`. I find that it will call `branch_get`
> before create and [un]set upstream, and die with "no such branch" if failed.
> but `branch_get` seems never fail, it is a get_or_create. Also, it was
> confused that getting a branch before it has created.
>
> builtin/branch.c #811
>
> } else if (argc > 0 && argc <= 2) {
> struct branch *branch = branch_get(argv[0]);
>
> if (!branch)
> die(_("no such branch '%s'"), argv[0]);
>From my reading of the source you're correct. That !branch case is
pointless. The only way that function can fail is in the x*() family
of functions, which'll make the function die instead of returning
NULL.
Re: Does git load index file into memory?
On 10/12, Farhan Khan wrote: > Hi all, > > Does git load the entire index file into memory when it wants to > edit/view it? I ask because I wonder if this can become a problem with > the index file becomes arbitrarily large, like for the Linux kernel. Yes, currently git always loads the entire index file for any operation on it. This is not particularly a problem for projects like the linux kernel, as the index file for it is relatively small, ~6MB at the moment. It is more of a problem for larger repositories, such as the windows repository, which has and index file of ~300-400MB, iirc, where loading the index has a significant cost. There's some patch series in progress to improve the performance, e.g. Ben Pearts series to load the index in parallel [*1*]. For writing the index to disk again, the split index feature can help improve performance. See also 'man git-update-index' and "core.splitIndex" in 'man git-config'. [*1*]: https://public-inbox.org/git/20181010155938.20996-1-peart...@gmail.com/ > Thanks, > -- > Farhan Khan > PGP Fingerprint: B28D 2726 E2BC A97E 3854 5ABE 9A9F 00BC D525 16EE
