Stephen Boyd <sboyd@xxxxxxxxxx> writes:
> I wonder if we need to make some other sort of form of
> "prerequisite-patch-id:" here and let that be a legacy form of the
> patch-id so that users know that they have a fixed version of this code?
> Maybe "prerequisite-stable-patch-id:"? Or we don't have to care because
> it's been broken for anything besides the most trivial type of patches
> and presumably users aren't able to use it with 'patch-id --stable'?
Do projects actively use -O<orderfile> when generating the patches?
I had an impression that not many do, and without -O<orderfile> in
the picture, --unstable/--stable would not matter, no?
So, I am not sure if this matters very much in practice.
>
> Documentation/git-format-patch.txt | 2 +-
> builtin/log.c | 2 +-
> builtin/patch-id.c | 17 +---------------
> diff.c | 32 +++++++++++++++++++++++++-----
> diff.h | 3 ++-
> patch-ids.c | 10 +++++-----
> patch-ids.h | 2 +-
> t/t4014-format-patch.sh | 8 ++++----
> 8 files changed, 42 insertions(+), 34 deletions(-)
>
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index e8cc792e7f5d..1af85d404f51 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -583,7 +583,7 @@ of 'base commit' in topological order before the patches can be applied.
> The 'base commit' is shown as "base-commit: " followed by the 40-hex of
> the commit object name. A 'prerequisite patch' is shown as
> "prerequisite-patch-id: " followed by the 40-hex 'patch id', which can
> -be obtained by passing the patch through the `git patch-id --unstable`
> +be obtained by passing the patch through the `git patch-id --stable`
> command.
>
> Imagine that on top of the public commit P, you applied well-known
> diff --git a/builtin/log.c b/builtin/log.c
> index e43ee12fb1dd..147850dc7317 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1435,7 +1435,7 @@ static void prepare_bases(struct base_tree_info *bases,
> struct object_id *patch_id;
> if (*commit_base_at(&commit_base, commit))
> continue;
> - if (commit_patch_id(commit, &diffopt, &oid, 0))
> + if (commit_patch_id(commit, &diffopt, &oid, 0, 1))
> die(_("cannot get patch id"));
> ALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);
> patch_id = bases->patch_id + bases->nr_patch_id;
> diff --git a/builtin/patch-id.c b/builtin/patch-id.c
> index 970d0d30b4f4..bd28b80b2d0f 100644
> --- a/builtin/patch-id.c
> +++ b/builtin/patch-id.c
> @@ -1,5 +1,6 @@
> #include "builtin.h"
> #include "config.h"
> +#include "diff.h"
>
> static void flush_current_id(int patchlen, struct object_id *id, struct object_id *result)
> {
> @@ -54,22 +55,6 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
> return 1;
> }
>
> -static void flush_one_hunk(struct object_id *result, git_SHA_CTX *ctx)
> -{
> - unsigned char hash[GIT_MAX_RAWSZ];
> - unsigned short carry = 0;
> - int i;
> -
> - git_SHA1_Final(hash, ctx);
> - git_SHA1_Init(ctx);
> - /* 20-byte sum, with carry */
> - for (i = 0; i < GIT_SHA1_RAWSZ; ++i) {
> - carry += result->hash[i] + hash[i];
> - result->hash[i] = carry;
> - carry >>= 8;
> - }
> -}
> -
> static int get_one_patchid(struct object_id *next_oid, struct object_id *result,
> struct strbuf *line_buf, int stable)
> {
> diff --git a/diff.c b/diff.c
> index 4d3cf83a27e5..8e3675b416a2 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5988,6 +5988,22 @@ static int remove_space(char *line, int len)
> return dst - line;
> }
>
> +void flush_one_hunk(struct object_id *result, git_SHA_CTX *ctx)
> +{
> + unsigned char hash[GIT_MAX_RAWSZ];
> + unsigned short carry = 0;
> + int i;
> +
> + git_SHA1_Final(hash, ctx);
> + git_SHA1_Init(ctx);
> + /* 20-byte sum, with carry */
> + for (i = 0; i < GIT_SHA1_RAWSZ; ++i) {
> + carry += result->hash[i] + hash[i];
> + result->hash[i] = carry;
> + carry >>= 8;
> + }
> +}
> +
> static void patch_id_consume(void *priv, char *line, unsigned long len)
> {
> struct patch_id_t *data = priv;
> @@ -6012,8 +6028,8 @@ static void patch_id_add_mode(git_SHA_CTX *ctx, unsigned mode)
> git_SHA1_Update(ctx, buf, len);
> }
>
> -/* returns 0 upon success, and writes result into sha1 */
> -static int diff_get_patch_id(struct diff_options *options, struct object_id *oid, int diff_header_only)
> +/* returns 0 upon success, and writes result into oid */
> +static int diff_get_patch_id(struct diff_options *options, struct object_id *oid, int diff_header_only, int stable)
> {
> struct diff_queue_struct *q = &diff_queued_diff;
> int i;
> @@ -6023,6 +6039,7 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
> git_SHA1_Init(&ctx);
> memset(&data, 0, sizeof(struct patch_id_t));
> data.ctx = &ctx;
> + oidclr(oid);
>
> for (i = 0; i < q->nr; i++) {
> xpparam_t xpp;
> @@ -6098,17 +6115,22 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
> patch_id_consume, &data, &xpp, &xecfg))
> return error("unable to generate patch-id diff for %s",
> p->one->path);
> +
> + if (stable)
> + flush_one_hunk(oid, &ctx);
> }
>
> - git_SHA1_Final(oid->hash, &ctx);
> + if (!stable)
> + git_SHA1_Final(oid->hash, &ctx);
> +
> return 0;
> }
>
> -int diff_flush_patch_id(struct diff_options *options, struct object_id *oid, int diff_header_only)
> +int diff_flush_patch_id(struct diff_options *options, struct object_id *oid, int diff_header_only, int stable)
> {
> struct diff_queue_struct *q = &diff_queued_diff;
> int i;
> - int result = diff_get_patch_id(options, oid, diff_header_only);
> + int result = diff_get_patch_id(options, oid, diff_header_only, stable);
>
> for (i = 0; i < q->nr; i++)
> diff_free_filepair(q->queue[i]);
> diff --git a/diff.h b/diff.h
> index b20cbcc09142..de24a048649e 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -436,7 +436,8 @@ int run_diff_files(struct rev_info *revs, unsigned int option);
> int run_diff_index(struct rev_info *revs, int cached);
>
> int do_diff_cache(const struct object_id *, struct diff_options *);
> -int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
> +int diff_flush_patch_id(struct diff_options *, struct object_id *, int, int);
> +void flush_one_hunk(struct object_id *, git_SHA_CTX *);
>
> int diff_result_code(struct diff_options *, int);
>
> diff --git a/patch-ids.c b/patch-ids.c
> index c262e1be9c9c..f70d3966542d 100644
> --- a/patch-ids.c
> +++ b/patch-ids.c
> @@ -11,7 +11,7 @@ static int patch_id_defined(struct commit *commit)
> }
>
> int commit_patch_id(struct commit *commit, struct diff_options *options,
> - struct object_id *oid, int diff_header_only)
> + struct object_id *oid, int diff_header_only, int stable)
> {
> if (!patch_id_defined(commit))
> return -1;
> @@ -22,7 +22,7 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
> else
> diff_root_tree_oid(&commit->object.oid, "", options);
> diffcore_std(options);
> - return diff_flush_patch_id(options, oid, diff_header_only);
> + return diff_flush_patch_id(options, oid, diff_header_only, stable);
> }
>
> /*
> @@ -46,11 +46,11 @@ static int patch_id_neq(const void *cmpfn_data,
> struct patch_id *b = (void *)entry_or_key;
>
> if (is_null_oid(&a->patch_id) &&
> - commit_patch_id(a->commit, opt, &a->patch_id, 0))
> + commit_patch_id(a->commit, opt, &a->patch_id, 0, 0))
> return error("Could not get patch ID for %s",
> oid_to_hex(&a->commit->object.oid));
> if (is_null_oid(&b->patch_id) &&
> - commit_patch_id(b->commit, opt, &b->patch_id, 0))
> + commit_patch_id(b->commit, opt, &b->patch_id, 0, 0))
> return error("Could not get patch ID for %s",
> oid_to_hex(&b->commit->object.oid));
> return !oideq(&a->patch_id, &b->patch_id);
> @@ -80,7 +80,7 @@ static int init_patch_id_entry(struct patch_id *patch,
> struct object_id header_only_patch_id;
>
> patch->commit = commit;
> - if (commit_patch_id(commit, &ids->diffopts, &header_only_patch_id, 1))
> + if (commit_patch_id(commit, &ids->diffopts, &header_only_patch_id, 1, 0))
> return -1;
>
> hashmap_entry_init(patch, sha1hash(header_only_patch_id.hash));
> diff --git a/patch-ids.h b/patch-ids.h
> index 82a12b66f889..03bb04e7071f 100644
> --- a/patch-ids.h
> +++ b/patch-ids.h
> @@ -20,7 +20,7 @@ struct patch_ids {
> };
>
> int commit_patch_id(struct commit *commit, struct diff_options *options,
> - struct object_id *oid, int);
> + struct object_id *oid, int, int);
> int init_patch_ids(struct repository *, struct patch_ids *);
> int free_patch_ids(struct patch_ids *);
> struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index e82c6c7d9177..7a5833a4b275 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -1583,15 +1583,15 @@ test_expect_success 'format-patch --base' '
> git format-patch --stdout --base=HEAD~3 HEAD~.. | tail -n 7 >actual2 &&
> echo >expected &&
> echo "base-commit: $(git rev-parse HEAD~3)" >>expected &&
> - echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --unstable | awk "{print \$1}")" >>expected &&
> - echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --unstable | awk "{print \$1}")" >>expected &&
> + echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --stable | awk "{print \$1}")" >>expected &&
> + echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --stable | awk "{print \$1}")" >>expected &&
> signature >> expected &&
> test_cmp expected actual1 &&
> test_cmp expected actual2 &&
> echo >fail &&
> echo "base-commit: $(git rev-parse HEAD~3)" >>fail &&
> - echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --stable | awk "{print \$1}")" >>fail &&
> - echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --stable | awk "{print \$1}")" >>fail &&
> + echo "prerequisite-patch-id: $(git show --patch HEAD~2 | git patch-id --unstable | awk "{print \$1}")" >>fail &&
> + echo "prerequisite-patch-id: $(git show --patch HEAD~1 | git patch-id --unstable | awk "{print \$1}")" >>fail &&
> signature >> fail &&
> ! test_cmp fail actual1 &&
> ! test_cmp fail actual2
