submodule: refactor logic to determine changed submodules

There are currently two instances (fetch and push) where we want to determine if submodules have changed given some revision specification. These two instances don't use the same logic to generate a list of changed submodules and as a result there is a fair amount of code duplication. This patch refactors these two code paths such that they both use the same logic to generate a list of changed submodules. This also makes it easier for future callers to be able to reuse this logic as they only need to create an argv_array with the revision specification to be using during the revision walk. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Williams committed May 1, 2017 at 18:02 UTC aacc5c1a81c004efccff8075edb78acdf5f15264
1 file changed +105 -142
submodule.c
+105 -142
@@ -617,6 +617,94 @@ const struct submodule *submodule_from_ce(const struct cache_entry *ce)
617 return submodule_from_path(null_sha1, ce->name);
618 }
619
620 +static struct oid_array *submodule_commits(struct string_list *submodules,
621 + const char *path)
622 +{
623 + struct string_list_item *item;
624 +
625 + item = string_list_insert(submodules, path);
626 + if (item->util)
627 + return (struct oid_array *) item->util;
628 +
629 + /* NEEDSWORK: should we have oid_array_init()? */
630 + item->util = xcalloc(1, sizeof(struct oid_array));
631 + return (struct oid_array *) item->util;
632 +}
633 +
634 +static void collect_changed_submodules_cb(struct diff_queue_struct *q,
635 + struct diff_options *options,
636 + void *data)
637 +{
638 + int i;
639 + struct string_list *changed = data;
640 +
641 + for (i = 0; i < q->nr; i++) {
642 + struct diff_filepair *p = q->queue[i];
643 + struct oid_array *commits;
644 + if (!S_ISGITLINK(p->two->mode))
645 + continue;
646 +
647 + if (S_ISGITLINK(p->one->mode)) {
648 + /*
649 + * NEEDSWORK: We should honor the name configured in
650 + * the .gitmodules file of the commit we are examining
651 + * here to be able to correctly follow submodules
652 + * being moved around.
653 + */
654 + commits = submodule_commits(changed, p->two->path);
655 + oid_array_append(commits, &p->two->oid);
656 + } else {
657 + /* Submodule is new or was moved here */
658 + /*
659 + * NEEDSWORK: When the .git directories of submodules
660 + * live inside the superprojects .git directory some
661 + * day we should fetch new submodules directly into
662 + * that location too when config or options request
663 + * that so they can be checked out from there.
664 + */
665 + continue;
666 + }
667 + }
668 +}
669 +
670 +/*
671 + * Collect the paths of submodules in 'changed' which have changed based on
672 + * the revisions as specified in 'argv'. Each entry in 'changed' will also
673 + * have a corresponding 'struct oid_array' (in the 'util' field) which lists
674 + * what the submodule pointers were updated to during the change.
675 + */
676 +static void collect_changed_submodules(struct string_list *changed,
677 + struct argv_array *argv)
678 +{
679 + struct rev_info rev;
680 + const struct commit *commit;
681 +
682 + init_revisions(&rev, NULL);
683 + setup_revisions(argv->argc, argv->argv, &rev, NULL);
684 + if (prepare_revision_walk(&rev))
685 + die("revision walk setup failed");
686 +
687 + while ((commit = get_revision(&rev))) {
688 + struct rev_info diff_rev;
689 +
690 + init_revisions(&diff_rev, NULL);
691 + diff_rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
692 + diff_rev.diffopt.format_callback = collect_changed_submodules_cb;
693 + diff_rev.diffopt.format_callback_data = changed;
694 + diff_tree_combined_merge(commit, 1, &diff_rev);
695 + }
696 +
697 + reset_revision_walk();
698 +}
699 +
700 +static void free_submodules_oids(struct string_list *submodules)
701 +{
702 + struct string_list_item *item;
703 + for_each_string_list_item(item, submodules)
704 + oid_array_clear((struct oid_array *) item->util);
705 + string_list_clear(submodules, 1);
706 +}
707 +
708 static int has_remote(const char *refname, const struct object_id *oid,
709 int flags, void *cb_data)
710 {
@@ -729,92 +817,31 @@ static int submodule_needs_pushing(const char *path, struct oid_array *commits)
817 return 0;
818 }
819
732 -static struct oid_array *submodule_commits(struct string_list *submodules,
733 - const char *path)
734 -{
735 - struct string_list_item *item;
736 -
737 - item = string_list_insert(submodules, path);
738 - if (item->util)
739 - return (struct oid_array *) item->util;
740 -
741 - /* NEEDSWORK: should we have oid_array_init()? */
742 - item->util = xcalloc(1, sizeof(struct oid_array));
743 - return (struct oid_array *) item->util;
744 -}
745 -
746 -static void collect_submodules_from_diff(struct diff_queue_struct *q,
747 - struct diff_options *options,
748 - void *data)
749 -{
750 - int i;
751 - struct string_list *submodules = data;
752 -
753 - for (i = 0; i < q->nr; i++) {
754 - struct diff_filepair *p = q->queue[i];
755 - struct oid_array *commits;
756 - if (!S_ISGITLINK(p->two->mode))
757 - continue;
758 - commits = submodule_commits(submodules, p->two->path);
759 - oid_array_append(commits, &p->two->oid);
760 - }
761 -}
762 -
763 -static void find_unpushed_submodule_commits(struct commit *commit,
764 - struct string_list *needs_pushing)
765 -{
766 - struct rev_info rev;
767 -
768 - init_revisions(&rev, NULL);
769 - rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
770 - rev.diffopt.format_callback = collect_submodules_from_diff;
771 - rev.diffopt.format_callback_data = needs_pushing;
772 - diff_tree_combined_merge(commit, 1, &rev);
773 -}
774 -
775 -static void free_submodules_oids(struct string_list *submodules)
776 -{
777 - struct string_list_item *item;
778 - for_each_string_list_item(item, submodules)
779 - oid_array_clear((struct oid_array *) item->util);
780 - string_list_clear(submodules, 1);
781 -}
782 -
820 int find_unpushed_submodules(struct oid_array *commits,
821 const char *remotes_name, struct string_list *needs_pushing)
822 {
786 - struct rev_info rev;
787 - struct commit *commit;
823 struct string_list submodules = STRING_LIST_INIT_DUP;
824 struct string_list_item *submodule;
825 struct argv_array argv = ARGV_ARRAY_INIT;
826
792 - init_revisions(&rev, NULL);
793 -
827 /* argv.argv[0] will be ignored by setup_revisions */
828 argv_array_push(&argv, "find_unpushed_submodules");
829 oid_array_for_each_unique(commits, append_oid_to_argv, &argv);
830 argv_array_push(&argv, "--not");
831 argv_array_pushf(&argv, "--remotes=%s", remotes_name);
832
800 - setup_revisions(argv.argc, argv.argv, &rev, NULL);
801 - if (prepare_revision_walk(&rev))
802 - die("revision walk setup failed");
803 -
804 - while ((commit = get_revision(&rev)) != NULL)
805 - find_unpushed_submodule_commits(commit, &submodules);
806 -
807 - reset_revision_walk();
808 - argv_array_clear(&argv);
833 + collect_changed_submodules(&submodules, &argv);
834
835 for_each_string_list_item(submodule, &submodules) {
811 - struct oid_array *commits = (struct oid_array *) submodule->util;
836 + struct oid_array *commits = submodule->util;
837 + const char *path = submodule->string;
838
813 - if (submodule_needs_pushing(submodule->string, commits))
814 - string_list_insert(needs_pushing, submodule->string);
839 + if (submodule_needs_pushing(path, commits))
840 + string_list_insert(needs_pushing, path);
841 }
842
843 free_submodules_oids(&submodules);
844 + argv_array_clear(&argv);
845
846 return needs_pushing->nr;
847 }
@@ -931,61 +958,6 @@ int push_unpushed_submodules(struct oid_array *commits,
958 return ret;
959 }
960
934 -static int is_submodule_commit_present(const char *path, unsigned char sha1[20])
935 -{
936 - int is_present = 0;
937 - if (!add_submodule_odb(path) && lookup_commit_reference(sha1)) {
938 - /* Even if the submodule is checked out and the commit is
939 - * present, make sure it is reachable from a ref. */
940 - struct child_process cp = CHILD_PROCESS_INIT;
941 - const char *argv[] = {"rev-list", "-n", "1", NULL, "--not", "--all", NULL};
942 - struct strbuf buf = STRBUF_INIT;
943 -
944 - argv[3] = sha1_to_hex(sha1);
945 - cp.argv = argv;
946 - prepare_submodule_repo_env(&cp.env_array);
947 - cp.git_cmd = 1;
948 - cp.no_stdin = 1;
949 - cp.dir = path;
950 - if (!capture_command(&cp, &buf, 1024) && !buf.len)
951 - is_present = 1;
952 -
953 - strbuf_release(&buf);
954 - }
955 - return is_present;
956 -}
957 -
958 -static void submodule_collect_changed_cb(struct diff_queue_struct *q,
959 - struct diff_options *options,
960 - void *data)
961 -{
962 - int i;
963 - for (i = 0; i < q->nr; i++) {
964 - struct diff_filepair *p = q->queue[i];
965 - if (!S_ISGITLINK(p->two->mode))
966 - continue;
967 -
968 - if (S_ISGITLINK(p->one->mode)) {
969 - /* NEEDSWORK: We should honor the name configured in
970 - * the .gitmodules file of the commit we are examining
971 - * here to be able to correctly follow submodules
972 - * being moved around. */
973 - struct string_list_item *path;
974 - path = unsorted_string_list_lookup(&changed_submodule_paths, p->two->path);
975 - if (!path && !is_submodule_commit_present(p->two->path, p->two->oid.hash))
976 - string_list_append(&changed_submodule_paths, p->two->path);
977 - } else {
978 - /* Submodule is new or was moved here */
979 - /* NEEDSWORK: When the .git directories of submodules
980 - * live inside the superprojects .git directory some
981 - * day we should fetch new submodules directly into
982 - * that location too when config or options request
983 - * that so they can be checked out from there. */
984 - continue;
985 - }
986 - }
987 -}
988 -
961 static int append_oid_to_array(const char *ref, const struct object_id *oid,
962 int flags, void *data)
963 {
@@ -1006,45 +978,36 @@ void check_for_new_submodule_commits(struct object_id *oid)
978
979 static void calculate_changed_submodule_paths(void)
980 {
1009 - struct rev_info rev;
1010 - struct commit *commit;
981 struct argv_array argv = ARGV_ARRAY_INIT;
982 + struct string_list changed_submodules = STRING_LIST_INIT_DUP;
983 + const struct string_list_item *item;
984
985 /* No need to check if there are no submodules configured */
986 if (!submodule_from_path(NULL, NULL))
987 return;
988
1017 - init_revisions(&rev, NULL);
989 argv_array_push(&argv, "--"); /* argv[0] program name */
990 oid_array_for_each_unique(&ref_tips_after_fetch,
991 append_oid_to_argv, &argv);
992 argv_array_push(&argv, "--not");
993 oid_array_for_each_unique(&ref_tips_before_fetch,
994 append_oid_to_argv, &argv);
1024 - setup_revisions(argv.argc, argv.argv, &rev, NULL);
1025 - if (prepare_revision_walk(&rev))
1026 - die("revision walk setup failed");
995
996 /*
997 * Collect all submodules (whether checked out or not) for which new
998 * commits have been recorded upstream in "changed_submodule_paths".
999 */
1032 - while ((commit = get_revision(&rev))) {
1033 - struct commit_list *parent = commit->parents;
1034 - while (parent) {
1035 - struct diff_options diff_opts;
1036 - diff_setup(&diff_opts);
1037 - DIFF_OPT_SET(&diff_opts, RECURSIVE);
1038 - diff_opts.output_format |= DIFF_FORMAT_CALLBACK;
1039 - diff_opts.format_callback = submodule_collect_changed_cb;
1040 - diff_setup_done(&diff_opts);
1041 - diff_tree_sha1(parent->item->object.oid.hash, commit->object.oid.hash, "", &diff_opts);
1042 - diffcore_std(&diff_opts);
1043 - diff_flush(&diff_opts);
1044 - parent = parent->next;
1045 - }
1000 + collect_changed_submodules(&changed_submodules, &argv);
1001 +
1002 + for_each_string_list_item(item, &changed_submodules) {
1003 + struct oid_array *commits = item->util;
1004 + const char *path = item->string;
1005 +
1006 + if (!submodule_has_commits(path, commits))
1007 + string_list_append(&changed_submodule_paths, path);
1008 }
1009
1010 + free_submodules_oids(&changed_submodules);
1011 argv_array_clear(&argv);
1012 oid_array_clear(&ref_tips_before_fetch);
1013 oid_array_clear(&ref_tips_after_fetch);