From 54fd80e3f8d7850576c60fa8baf7269df3c9e6a3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 12 Apr 2018 13:32:27 +0100 Subject: [PATCH] revwalk: fix uninteresting revs sometimes not limiting graphwalk When we want to limit our graphwalk, we use the heuristic of checking whether the newest limiting (uninteresting) revision is newer than the oldest interesting revision. We do so by inspecting whether the first item's commit time of the user-supplied list of revisions is newer than the last added interesting revision. This is wrong though, as the user supplied list is in no way guaranteed to be sorted by increasing commit dates. This could lead us to abort the revwalk early before applying all relevant limiting revisions, outputting revisions which should in fact have been hidden. Fix the heuristic by instead checking whether _any_ of the limiting commits was made earlier than the last interesting commit. Add a test. --- src/revwalk.c | 33 +++++------------- tests/resources/revwalk.git/HEAD | 1 + tests/resources/revwalk.git/config | 6 ++++ tests/resources/revwalk.git/description | 1 + .../resources/revwalk.git/objects/info/packs | 2 ++ ...dacb9971981a1a3264fd473da5b800f2715959.idx | Bin 0 -> 1520 bytes ...acb9971981a1a3264fd473da5b800f2715959.pack | Bin 0 -> 1862 bytes tests/resources/revwalk.git/packed-refs | 7 ++++ tests/resources/revwalk.git/refs/.gitkeep | 0 tests/revwalk/basic.c | 27 ++++++++++++++ 10 files changed, 53 insertions(+), 24 deletions(-) create mode 100644 tests/resources/revwalk.git/HEAD create mode 100644 tests/resources/revwalk.git/config create mode 100644 tests/resources/revwalk.git/description create mode 100644 tests/resources/revwalk.git/objects/info/packs create mode 100644 tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.idx create mode 100644 tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.pack create mode 100644 tests/resources/revwalk.git/packed-refs create mode 100644 tests/resources/revwalk.git/refs/.gitkeep diff --git a/src/revwalk.c b/src/revwalk.c index b1aa8f91a..eb228a522 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -375,20 +375,6 @@ static int add_parents_to_list(git_revwalk *walk, git_commit_list_node *commit, return 0; } -static int everybody_uninteresting(git_commit_list *orig) -{ - git_commit_list *list = orig; - - while (list) { - git_commit_list_node *commit = list->item; - list = list->next; - if (!commit->uninteresting) - return 0; - } - - return 1; -} - /* How many unintersting commits we want to look at after we run out of interesting ones */ #define SLOP 5 @@ -398,16 +384,15 @@ static int still_interesting(git_commit_list *list, int64_t time, int slop) if (!list) return 0; - /* - * If the destination list has commits with an earlier date - * than our source we want to continue looking. - */ - if (time <= list->item->time) - return SLOP; - - /* If we find interesting commits, we reset the slop count */ - if (!everybody_uninteresting(list)) - return SLOP; + for (; list; list = list->next) { + /* + * If the destination list has commits with an earlier date than + * our source or if it still contains interesting commits we + * want to continue looking. + */ + if (!list->item->uninteresting || list->item->time > time) + return SLOP; + } /* Everything's uninteresting, reduce the count */ return slop - 1; diff --git a/tests/resources/revwalk.git/HEAD b/tests/resources/revwalk.git/HEAD new file mode 100644 index 000000000..cb089cd89 --- /dev/null +++ b/tests/resources/revwalk.git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/master diff --git a/tests/resources/revwalk.git/config b/tests/resources/revwalk.git/config new file mode 100644 index 000000000..7c968c3b5 --- /dev/null +++ b/tests/resources/revwalk.git/config @@ -0,0 +1,6 @@ +[core] + bare = true + repositoryformatversion = 0 + filemode = false + symlinks = false + ignorecase = true diff --git a/tests/resources/revwalk.git/description b/tests/resources/revwalk.git/description new file mode 100644 index 000000000..498b267a8 --- /dev/null +++ b/tests/resources/revwalk.git/description @@ -0,0 +1 @@ +Unnamed repository; edit this file 'description' to name the repository. diff --git a/tests/resources/revwalk.git/objects/info/packs b/tests/resources/revwalk.git/objects/info/packs new file mode 100644 index 000000000..d8d85b895 --- /dev/null +++ b/tests/resources/revwalk.git/objects/info/packs @@ -0,0 +1,2 @@ +P pack-9adacb9971981a1a3264fd473da5b800f2715959.pack + diff --git a/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.idx b/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.idx new file mode 100644 index 0000000000000000000000000000000000000000..e157b386eca606351c67d3486a6ec55069c957bb GIT binary patch literal 1520 zcmexg;-AdGz`z8=*Z?C?5F|I8Ff-6UgfI(G4L-~YlppMv9q2}M%mEa`hdF`rgfJIS z4L-~blt;%rKrwun7bs6E<^$>>1@i;d(+dj#?HW>;)&964`;PEq0vjg%E8f3CHA?6) zJDX%hcyG=8PrD}FHM=Z-;|6EDfPvM6uLgp9w?FHjx%bHw6VJ~%GKO8wXZwY?;&ZKR zmioRpSaBux`gW~Q@21#ePN(yBE=YLl{o~rCj^FP*SxSs&Y*D>-NbK7ARgWvOqmP(b zT0JeX5)+V4Sy)o*CtkOmUx(wjp!0)D?ghWj=U-&boE776R`S-R^8cP^8{?AZ-&km$>?$J>e|nAsg7OIDf)D=6za!ZZGz*t*h<17p(q&)~q#rE^lY& z3q=_!d|WxH=VYpL>LiIBI`0BlM5P`a`JtWpyKqNId75|r#eKJB<7~RU|?YT0W6mKfV4fZ=ve~Hx|$3OjMst1j4qIt2h!Vs z#qcX2P5{bF0@a-arm<;2xs^bgXV$ILGYe-(Ng1X5b+=u*gW*$QWTgKxk20Zi%cX6( P<{Ez1{9v(h>iw$#Mmgs^ literal 0 HcmV?d00001 diff --git a/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.pack b/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.pack new file mode 100644 index 0000000000000000000000000000000000000000..2a61f94032e627a829af0ce7ae2c34a4299ce965 GIT binary patch literal 1862 zcmZ{le>BtkAICp;vsf+HM%?YX-Q~xw`7wm)-qP`F*tBojWy&fxnY)l*{a6j7D8$z- z<;S*b5)+XUam$IBb#ZBovs_6=Zt31kt`O#4Tb*<7b-v#}-v2+&^Zj@|p8+0T{r~`g z)}F!lFt#AK+hZfh5$4{Xymwz5){jT@0&mwVv6w1<#i+S*hO4`G>{t8UvHA;sREOBB z(3^MoRt{`TR7AL#!uv5F$Y|2%_TJbc=yY{F2EB`eUbDO^QCJIDO4}k*pDbw(PE)-- z^Md&eu2B`Wr?dHp74Q}YMVp@DKAz$#K_StFu`+tM4u~Rvb$3tExgq9$COi4jbYz-Ne&m z%DF-*HM_sS`#KeKZI86fMtE8U4pGG&Za#{v92T0)c!1s-8z?ik_S-Ed^{}|o>G_+= zv8heE5AY4^B7tio88F@e-$fiC_DlC;->vGGwDpE2jhZsscvjItMIT7g8Y{@#D+UL1 zoa#A+^g5_+9rKe*$urX13I64m`95{ngdDRP`N8)+zF+p8x$v%&{B!f*!%%U4U;9OL zJJPLe=k%&zV6}aEbs%nb4)W}s_&5$kF*~U(d_q5AT6L*}6aRj3S`tT0U4rAj9 zk7d2Rw)+C`&N@JS0n|~;o&C=MiLU$@I$T`n9Fx6OL8|(*X%S>Eap-x)Y(uSt)Zdm_ ziyW_&*4DTfW$jL{W}04lw(|sMb7o1Eg>6AYz-YBJnuMr~H9A7AwJEzlhs;x4ySHJv zgZypli<1Y_S{5VVKIu+~GX6XEf9c%b>D32H??uQ(nrn-2)4!b31c-T44$WM1>di-| zq=wD!FUt6mFDUSkrtYkxbo-;7ucEBUcq)P=b;F8g>XkAmLv$$-A8P5#A_r9~Uef}D z;i3DTZ6F*OM-$*5u1^>ffy*gr8AS{Wk92cfrjoPplVH_Eze#xhTnaGmhA$H8Ye>W- zh{kSp5%tXdX9O$t>GUIK`;s;c%sL z8-wVH$Yj&Vt_iEU;A8m5g{McJlL$VZgw$EsmaqrY>cm>MUaX3He^-}1Y1&JhBJw)jN&-Po5LlyJ5!Op5T#j)Oq2+@)i0?Ts>2*^B!_{WpXVN;OLjy0rn1 z`5!1e(GM^UO2c~dOE!kfdC;gXDm4vOaKl;AVwPybYykb(1mlqR-mz{*Z_I3#Dv|KAa=)CbW+gZ3nPc+mFKhAoz_@Obe)Cso^jciEO;WszRqo?-S=ZZ5+) zcPL3JF^qROKI!yjviomeD9l~$CMe4(klAstrPk(u9AU`AtBd#z>p(Q$!y|@oG&}nh zYe45vj(NrCbMZ%+Z7GI5hl&F}r`c@z%=X*Fse=I>eB_qWnOl;>PNeQf`&;}WgcEUz znEtz#T?Aw!$G;(csW>mj`}sZgFNskn1jlK0j@u?K`vK!{g!_46{u@0VVZeeLp{)WUucUm+zh8Qn@>*@TR!X+#>Awa*zRFS`J~v_QVQB)p zF-r93bP6bjL52Orr;tm$)X{q=JNlIu;hWRX7V~{tep(yhsnUanbXzuquk=}$*|k{ z+_ZXkf;cE7duRtX_^i%-pL=Fi4D6%oJD$g5QA@msA&`A~HGYb8YzM+3szC1jB=wgX zilwjc{VR!9*l<*#V&P_}13gs$tRRg)2)n8k7TS8k*mdZIwjZC$nuYP6oA0J&treu5 zBRV3iv@|B{yRbItq(I2$A1Eu0)&uU_Ln9_Zsb8}NPh~^09Uid^!LIzYuC+$p2Q~zQ zAu(VtFc^n>xiT`+gMdVU0kIJ@AI%nk*G3Zfgg5+iaeNU9wf%_N*R7%%n2kSh;4gD( BY9;^x literal 0 HcmV?d00001 diff --git a/tests/resources/revwalk.git/packed-refs b/tests/resources/revwalk.git/packed-refs new file mode 100644 index 000000000..905a3db51 --- /dev/null +++ b/tests/resources/revwalk.git/packed-refs @@ -0,0 +1,7 @@ +# pack-refs with: peeled fully-peeled sorted +3ae0f53011bdb7e68f99bde4943449f36c1c318a refs/heads/A +061978578d7c9ff2ba92dd36d31fd8d809871030 refs/heads/B +743398b425d6c216d6cfaae3786b5bc436393ae5 refs/heads/C +790ba0facf6fd103699a5c40cd19dad277ff49cd refs/heads/D +d3d783066cf7d95def6844b9c5118c1e7bcce7df refs/heads/E +d3d783066cf7d95def6844b9c5118c1e7bcce7df refs/heads/master diff --git a/tests/resources/revwalk.git/refs/.gitkeep b/tests/resources/revwalk.git/refs/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/tests/revwalk/basic.c b/tests/revwalk/basic.c index 6a23701f3..1106bf4ce 100644 --- a/tests/revwalk/basic.c +++ b/tests/revwalk/basic.c @@ -555,3 +555,30 @@ void test_revwalk_basic__old_hidden_commit_two(void) cl_git_fail_with(GIT_ITEROVER, git_revwalk_next(&oid, _walk)); } + +/* + * Ensure that we correctly hide all parent commits of a newer + * commit when first hiding older commits. + * + * % git rev-list D ^B ^A ^E + * 790ba0facf6fd103699a5c40cd19dad277ff49cd + * b82cee5004151ae0c4f82b69fb71b87477664b6f + */ +void test_revwalk_basic__newer_hidden_commit_hides_old_commits(void) +{ + git_oid oid; + + revwalk_basic_setup_walk("revwalk.git"); + + cl_git_pass(git_revwalk_push_ref(_walk, "refs/heads/D")); + cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/B")); + cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/A")); + cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/E")); + + cl_git_pass(git_revwalk_next(&oid, _walk)); + cl_assert(git_oid_streq(&oid, "b82cee5004151ae0c4f82b69fb71b87477664b6f")); + cl_git_pass(git_revwalk_next(&oid, _walk)); + cl_assert(git_oid_streq(&oid, "790ba0facf6fd103699a5c40cd19dad277ff49cd")); + + cl_git_fail_with(GIT_ITEROVER, git_revwalk_next(&oid, _walk)); +}