pack-objects: split add_object_entry

This function actually does three things: 1. Check whether we've already added the object to our packing list. 2. Check whether the object meets our criteria for adding. 3. Actually add the object to our packing list. It's a little hard to see these three phases, because they happen linearly in the rather long function. Instead, this patch breaks them up into three separate helper functions. The result is a little easier to follow, though it unfortunately suffers from some optimization interdependencies between the stages (e.g., during step 3 we use the packing list index from step 1 and the packfile information from step 2). More importantly, though, the various parts can be composed differently, as they will be in the next patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 21, 2013 at 09:00 UTC ce2bc42456b88c5f01f7f591cf0cc9db1a5bfc3d
1 file changed +78 -20
builtin/pack-objects.c
+78 -20
@@ -800,41 +800,69 @@ static int no_try_delta(const char *path)
800 return 0;
801 }
802
803 -static int add_object_entry(const unsigned char *sha1, enum object_type type,
804 - const char *name, int exclude)
803 +/*
804 + * When adding an object, check whether we have already added it
805 + * to our packing list. If so, we can skip. However, if we are
806 + * being asked to excludei t, but the previous mention was to include
807 + * it, make sure to adjust its flags and tweak our numbers accordingly.
808 + *
809 + * As an optimization, we pass out the index position where we would have
810 + * found the item, since that saves us from having to look it up again a
811 + * few lines later when we want to add the new entry.
812 + */
813 +static int have_duplicate_entry(const unsigned char *sha1,
814 + int exclude,
815 + uint32_t *index_pos)
816 {
817 struct object_entry *entry;
807 - struct packed_git *p, *found_pack = NULL;
808 - off_t found_offset = 0;
809 - uint32_t hash = pack_name_hash(name);
810 - uint32_t index_pos;
818
812 - entry = packlist_find(&to_pack, sha1, &index_pos);
813 - if (entry) {
814 - if (exclude) {
815 - if (!entry->preferred_base)
816 - nr_result--;
817 - entry->preferred_base = 1;
818 - }
819 + entry = packlist_find(&to_pack, sha1, index_pos);
820 + if (!entry)
821 return 0;
822 +
823 + if (exclude) {
824 + if (!entry->preferred_base)
825 + nr_result--;
826 + entry->preferred_base = 1;
827 }
828
829 + return 1;
830 +}
831 +
832 +/*
833 + * Check whether we want the object in the pack (e.g., we do not want
834 + * objects found in non-local stores if the "--local" option was used).
835 + *
836 + * As a side effect of this check, we will find the packed version of this
837 + * object, if any. We therefore pass out the pack information to avoid having
838 + * to look it up again later.
839 + */
840 +static int want_object_in_pack(const unsigned char *sha1,
841 + int exclude,
842 + struct packed_git **found_pack,
843 + off_t *found_offset)
844 +{
845 + struct packed_git *p;
846 +
847 if (!exclude && local && has_loose_object_nonlocal(sha1))
848 return 0;
849
850 + *found_pack = NULL;
851 + *found_offset = 0;
852 +
853 for (p = packed_git; p; p = p->next) {
854 off_t offset = find_pack_entry_one(sha1, p);
855 if (offset) {
828 - if (!found_pack) {
856 + if (!*found_pack) {
857 if (!is_pack_valid(p)) {
858 warning("packfile %s cannot be accessed", p->pack_name);
859 continue;
860 }
833 - found_offset = offset;
834 - found_pack = p;
861 + *found_offset = offset;
862 + *found_pack = p;
863 }
864 if (exclude)
837 - break;
865 + return 1;
866 if (incremental)
867 return 0;
868 if (local && !p->pack_local)
@@ -844,6 +872,20 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
872 }
873 }
874
875 + return 1;
876 +}
877 +
878 +static void create_object_entry(const unsigned char *sha1,
879 + enum object_type type,
880 + uint32_t hash,
881 + int exclude,
882 + int no_try_delta,
883 + uint32_t index_pos,
884 + struct packed_git *found_pack,
885 + off_t found_offset)
886 +{
887 + struct object_entry *entry;
888 +
889 entry = packlist_alloc(&to_pack, sha1, index_pos);
890 entry->hash = hash;
891 if (type)
@@ -857,11 +899,27 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
899 entry->in_pack_offset = found_offset;
900 }
901
860 - display_progress(progress_state, to_pack.nr_objects);
902 + entry->no_try_delta = no_try_delta;
903 +}
904 +
905 +static int add_object_entry(const unsigned char *sha1, enum object_type type,
906 + const char *name, int exclude)
907 +{
908 + struct packed_git *found_pack;
909 + off_t found_offset;
910 + uint32_t index_pos;
911
862 - if (name && no_try_delta(name))
863 - entry->no_try_delta = 1;
912 + if (have_duplicate_entry(sha1, exclude, &index_pos))
913 + return 0;
914
915 + if (!want_object_in_pack(sha1, exclude, &found_pack, &found_offset))
916 + return 0;
917 +
918 + create_object_entry(sha1, type, pack_name_hash(name),
919 + exclude, name && no_try_delta(name),
920 + index_pos, found_pack, found_offset);
921 +
922 + display_progress(progress_state, to_pack.nr_objects);
923 return 1;
924 }
925