From: Junio C Hamano <gitster@pobox•com>
To: Heiko Voigt <hvoigt@hvoigt•net>
Cc: git@vger•kernel.org, Fredrik Gustafsson <iveqy@iveqy•com>,
Jens Lehmann <jens.lehmann@web•de>
Subject: Re: [PATCH v5 2/3] Refactor submodule push check to use string list instead of integer
Date: Mon, 13 Feb 2012 19:28:24 -0800 [thread overview]
Message-ID: <7vbop29jqf.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20120213092900.GC15585@t1405.greatnet.de> (Heiko Voigt's message of "Mon, 13 Feb 2012 10:29:00 +0100")
Heiko Voigt <hvoigt@hvoigt•net> writes:
> This allows us to tell the user which submodules have not been pushed.
> Additionally this is helpful when we want to automatically try to push
> submodules that have not been pushed.
Makes sense.
> diff --git a/submodule.c b/submodule.c
> index 645ff5d..3c714c2 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -357,21 +357,20 @@ static void collect_submodules_from_diff(struct diff_queue_struct *q,
> void *data)
> {
> int i;
> - int *needs_pushing = data;
> + struct string_list *needs_pushing = data;
>
> for (i = 0; i < q->nr; i++) {
> struct diff_filepair *p = q->queue[i];
> if (!S_ISGITLINK(p->two->mode))
> continue;
> if (submodule_needs_pushing(p->two->path, p->two->sha1)) {
> - *needs_pushing = 1;
> - break;
> + if (!string_list_has_string(needs_pushing, p->two->path))
> + string_list_insert(needs_pushing, p->two->path);
Does string_list API have "look for this and insert if it doesn't exist
but otherwise don't do anything"? Running get_entry_index() to answer
has_string() once and then calling it again to find where to insert to
respond to insert() looks a bit wasteful.
Just wondering.
> }
> }
> }
>
> -
> -static void commit_need_pushing(struct commit *commit, int *needs_pushing)
> +static void commit_need_pushing(struct commit *commit, struct string_list *needs_pushing)
> {
> struct rev_info rev;
>
> @@ -382,14 +381,15 @@ static void commit_need_pushing(struct commit *commit, int *needs_pushing)
> diff_tree_combined_merge(commit, 1, &rev);
> }
>
> -int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remotes_name)
> +int check_submodule_needs_pushing(unsigned char new_sha1[20],
> + const char *remotes_name, struct string_list *needs_pushing)
> {
> struct rev_info rev;
> struct commit *commit;
> const char *argv[] = {NULL, NULL, "--not", "NULL", NULL};
> int argc = ARRAY_SIZE(argv) - 1;
> char *sha1_copy;
> - int needs_pushing = 0;
> +
> struct strbuf remotes_arg = STRBUF_INIT;
>
> strbuf_addf(&remotes_arg, "--remotes=%s", remotes_name);
> @@ -401,14 +401,14 @@ int check_submodule_needs_pushing(unsigned char new_sha1[20], const char *remote
> if (prepare_revision_walk(&rev))
> die("revision walk setup failed");
>
> - while ((commit = get_revision(&rev)) && !needs_pushing)
> - commit_need_pushing(commit, &needs_pushing);
> + while ((commit = get_revision(&rev)) != NULL)
> + commit_need_pushing(commit, needs_pushing);
Now the helper function to find list of submodules that need pushing given
one commit starting to look more and more misnamed. It used to be "learn
if something needs pushing", but now it is "find what needs pushing".
Can somebody think of a good adjective to describe a submodule (or a set
of submodules) in this state, so that we can name this helper function
find_blue_submodules(), if the adjective were "blue"?
"Unpushed" submodule is the word used in the later part of the patch.
next prev parent reply other threads:[~2012-02-14 3:28 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-13 9:25 [PATCH v5 0/3] push: submodule support Heiko Voigt
2012-02-13 9:27 ` [PATCH v5 1/3] Teach revision walking machinery to walk multiple times sequencially Heiko Voigt
2012-02-14 1:33 ` Junio C Hamano
2012-03-26 19:32 ` Heiko Voigt
2012-03-26 21:28 ` Junio C Hamano
2012-02-13 9:29 ` [PATCH v5 2/3] Refactor submodule push check to use string list instead of integer Heiko Voigt
2012-02-14 3:28 ` Junio C Hamano [this message]
2012-03-26 19:33 ` Heiko Voigt
2012-03-26 19:55 ` Heiko Voigt
2012-03-26 21:29 ` Junio C Hamano
2012-02-13 9:30 ` [PATCH v5 3/3] push: teach --recurse-submodules the on-demand option Heiko Voigt
2012-02-14 3:34 ` Junio C Hamano
2012-02-15 22:28 ` Jens Lehmann
2012-03-26 19:33 ` Heiko Voigt
2012-05-13 14:47 ` [RFC/PATCH] read from 2 filedescriptors simultaneously into one strbuf Heiko Voigt
2012-03-26 21:22 ` [PATCH v5 3/3] push: teach --recurse-submodules the on-demand option Zbigniew Jędrzejewski-Szmek
2012-03-28 15:30 ` Heiko Voigt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vbop29jqf.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=hvoigt@hvoigt$(echo .)net \
--cc=iveqy@iveqy$(echo .)com \
--cc=jens.lehmann@web$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox