From a06c409d58fac109eb3d28436d4394d6e54a59f2 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 21 Feb 2025 09:02:57 +0100 Subject: [PATCH 1/4] reproduce the overlay-iterator issue causing double-refs (#1850). --- Cargo.lock | 1 + gix-ref/tests/Cargo.toml | 1 + .../make_repo_for_1850_repro.tar | Bin 0 -> 50176 bytes .../fixtures/make_repo_for_1850_repro.sh | 17 ++++++ gix-ref/tests/refs/file/store/iter.rs | 51 ++++++++++++++++++ 5 files changed, 70 insertions(+) create mode 100644 gix-ref/tests/fixtures/generated-archives/make_repo_for_1850_repro.tar create mode 100755 gix-ref/tests/fixtures/make_repo_for_1850_repro.sh diff --git a/Cargo.lock b/Cargo.lock index 097826a9048..6d0431cc970 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2520,6 +2520,7 @@ dependencies = [ "gix-ref 0.50.0", "gix-testtools", "gix-validate 0.9.3", + "insta", ] [[package]] diff --git a/gix-ref/tests/Cargo.toml b/gix-ref/tests/Cargo.toml index 0b0345bd0eb..9ec1578b984 100644 --- a/gix-ref/tests/Cargo.toml +++ b/gix-ref/tests/Cargo.toml @@ -32,3 +32,4 @@ gix-hash = { path = "../../gix-hash" } gix-validate = { path = "../../gix-validate" } gix-lock = { path = "../../gix-lock" } gix-object = { path = "../../gix-object" } +insta = "1.42.1" diff --git a/gix-ref/tests/fixtures/generated-archives/make_repo_for_1850_repro.tar b/gix-ref/tests/fixtures/generated-archives/make_repo_for_1850_repro.tar new file mode 100644 index 0000000000000000000000000000000000000000..9e5178f73972490ee4113d2b57a3208f3d0d171c GIT binary patch literal 50176 zcmeHw`+E~dvUYy8{)*0^Z*3FnF48`G>uC7~G*Az?NEd5^@|8^RUZ}=DgvVY~bO1ZvWDObzY znmpgBRU6+lmW~+)n8ag$WXu;2B~wdH?u!9_9RS7SaJ=l1NSe=G|D8s4wf@Uq7{KTH zmxg}(ZO|!_AK0snz_-POaA6-oDcaD)qo`1$S%Pe);b9ox6Ue zmFYel)iFe_vjfdiz0)ZN0S2uC2ftD4cDq62PNh}uwu5r3(rGs`Gbs7J(2s*S*YY{b zpj_|Xt#pI-&bGhZy6fKoUFv{X?f5~vzEdsl%nn{^hh0DH<+7~zukc;=|FSl|o#5xo z|4MnMQd!CWufQP5`j^K3@lvG!HzMahPybgc+x4B*`hV5+7jeCm5wEfTD?2+Y`+q5m zkpVj0|B1R*Rhen`C0{SI|H_R@*8Z#2s_+B9WYDYj{WrJ&>YbonsWr@qqv*QWT@ zJI=3vX*jxi_bau$#`@RFm6iU#5^=w<(W3RA#3zM;-w(bJbkhAA`oCV?-dXAYgo4Za z(5Y6t_3du0v(u?I%JohIb^zip4MarhcPn>Woys;MJ$L+-{$Ji2{4*iT{y&MLw@dQ< zIk#)be*}P5^8ah_#dQ5kL(2VeRA`?B?YGfnT#Wtxuy+Gm;F9%k)M`6f|F6-2{kQV} zmd^pVzAd%FK`B1T`i&iu~ zxnz9P_s8uM03n(dD9Aw|Tm0?J_@ls19(0Vfy|f+#ryw;K7ZV+a!|7CXU5c~JgT5_5 zB31A)dLN8NVJ8qsmWlw-3Qqj@VKf=t1Jj)bXrzcmXxR#S(JAN?Qj+zt&|n83Z97l9 z0t^Xfm<%u`)rS!doJ=gAjd9KLT09wyd~g^*}k@@`-|1TF7}Ch#F#*IVh&< z*3A-)$SX5p`HwLoR$FH*+pV*kQ%HOH3b*Is*tC6aQS29<)dAl@G${N}Fp7xycp|*6 zY$PMfCfaaYtP|k`>z0Y5EpE6z0$=EECv_(6g}V&qOmx%yw^__}!oX~7nvc1)pnVdV z{l8U>m%r`dE77UsJ`uwwBZ4N9eg7@Q1sHLF4Ijt;h#Slw9P56}!q*KKaa=%AG)Re& zkmMu)|H?xwTpW(T4V*4er0}=`X>}h853x0vFkx42tTaJW#MBl33FNRsQJ7non^TO* zRA**18HoIY1nU$;Bo-#7&^Nz4y#D|ss(3OZP_;wGj7-p<^oTrVW;eh_!VFXv5fUUD zQ5=ztV=yXa+opg45Lt33X(8~G*T$!jjtXAwlfT#^i%ZCb$1;G4Lm%h5G6vK;mI#c8 z4l#fO6U>$0oCTnm=`-OS88e(@v$REK_MCydYBxSsZFiq4`NW5QW@lir0MnC zzB|CWcgSMg*fONb;;A5zC!oQ4EK(|M59!K7bc6#+X)XytaFaF1QL_^ad(j!TNOv-j zCF5GQCSecqq?A^B#kO+RW1NO7Q3ys9?r8{IA@PC_L3@G|_o?ijsyfRPRC<<_iGWX& z8|o#ZDmjZL=G2Gq0xgE1xNr$R2!Ud#Z##|IzS4`xK(qqM$oL%}mH>XHK!jlR+ih?( z7QYAG8x)T*C8~J8!{3b8?wDfHBR@@I{xAx-A^-iYCX{&M5|ry1QZ$8N=h1CbV-C z#t|(RFv<_cqmb~-u6g+D)1&`x9zA>bu=)I#`<3P|5C4lKlFjIkWdf!Us7$HumpZ}w z(f|n>7HPuXBdA=^i-X*ne9v#NH%2@uM6Ev{MNGk(Y=uHR#xj}j6F8d-h-OzLAzIr9 znL7)qm*;?ZaybnbQ7~bJtkm-Is{Xt*+=FP+>xcxgMqQe5$*Gj?LRQXc`3Ce~e1w$$ zr}VrruWw42OZ>kbB>!anznbKWO+hU0)4%!o-_-x3=XSmW*n)n4YbFDK0Obwy1;!Ne zMN~h*st9PQQ3o8ryud@K0*~S+t@k`@w2qD^Vu|nKX_s+k=xD!Eh_J2Y12(X_k+78p z;!};CL%D@6(nEfsSm%!Slyf~*s|Vtj0hNOM!v_g(6@gfEnwkI}81*(U6Wf!4zS@ID^#yE$4|AjhXs*G!V{UNe|Y?Q0t^m3YQM@?8X7ecnK!b zj!IN0K92?m5fIlwLR^}&%aQOoI;{FLnkVW9fd+mTD{tBm+%&g}shJBxV1Ntfu9n?Z zr|;}Qu#s9x)0Ma`3Fy@e_Q918m1I-5VWgy`J zbOX~Ff3>Qh=v&Q8Np!ZU?P=cT=;-xbK*G$%2v zw%rRxogdV#10y`Stl-#F%Sz)vp!@i280=Aoq&Ig;d~7NN-R{aTR|1jlu$jxU>SZ9Z z?PsvSUcRiq%sLTKws#^euGBkey-5bOMmk)AK}0`ZVvF=_hogw*DSrZ3#(H4su2i-)O>9{n-z#emA8laj{XZS$w`(8aR+?c2A_QynjjgAr8%TUiA) z98c(iS+-cacP|&je%tM}14hXW*PgyL_CsFTk6W4sPqA3!AI4hXOCoS8@iD3^B`SUb zLl<)lI9O`V*y?jOIwmq6ne~_D!d<`6eZ6kh%j=uE%RM``XZAoALQHJ}MXM=XszX~Z zA^pIzbmD4^-E`D5umpEIaLWxSP=)qucos_zY3cTs&J{#oLkx6Kn6Vk~V|;>i@}>p0 zm;Y!#-MTe}@d?dDfO+}s_VdiQnB{mh2~v|>ORet_yy@(yTP8m z=%&k~bhBJS%{)sAeSqIDCBy?77&IJId4I-?FT@f<3f-Pc{0b*N6djAsns#p5_dWkx7H?)6_8u|4|? z8l-?MCIVgEIK$)Ccu3aCDW>Ue1gm)YtVovadI2LJI4i?Xg^3wEy9(1@jQs*m%T6Mt z0QAw!N;*p-y*ep=(;lC*$rj_Q7FU+uh_JNfY$U-s*-6t53#K^VFU_*r;q&L3f%-+0 zzO6g!{@@HI8Fnt?Ik2`Q)QeCl0_xM>e_&g+RBd53Jbp!#_BUf*wV1L{F;xmYn~~S*0H4BJ436 z`T;mT@obDhL(%F4z(R*=ha}>CKznkzg`*P2NMJ~%#2hSQ{05Izfx$~Q=8#9T=E` zzGZ#^`4Rn8n+0~jE<9X0`$>VA!k&TQfWSF>jI$CocyNe^%pa^VjY~&dz+J;()-mZb z#fMgh4h0C->HtHF5l(K+0)(Hr2;daEVsREa!#;E!)y1Z3h4r%_@IY7I`ponsda(XY zVPLp3v9Lq4{stWwJ`m*T{U=mGv+1!?#>J4FhM%GdLT}Rn$uJf@EaH|pjdoum`2nOz z8?L}y_WEDPHp=LKpYc1K(Y9~W3b;i7m$w_`tp0D{LitMne^vc|@%hgM27vBI)(6lO zfP{tgbO;A4L7|Vcc_*AzCmlgzOU?tNJWq6GABnWi<}~RcLm0;sN*ABT0N50<6=dAO zesCfmqLb$hNOh1jC>*j6MJ*_LcF-#yDbX07ZU!7J<%*146mXi3(BNc{}e`2M=j?V}H^u4+WSSSF* zHbrI&l0+GgUnmR&A8SfnMH_l~vF z`#*}u1q~G4*w&X}F5Carokmvw*SA;u-!0Kn|4#k?i}gQr6+B^h2U0I+=8R1aKais> z`PtaUok_o?NpM4)^B~zz?gU(2^G?~2J52Tn&s>x zuS;w#l|ny0z9BJi$^NhIROjk{ZI^de_dmY|r+n`IXP`w**>t>Rf#%mnR`s)&tR)vnAf##-reyxkoi<_a-0! z#^aqUdmka@Z5hNW!x20Rgcv2R^j=z#px!O>6nSl!xFqPMakcb9j)I582UCuhoa(`V z2m>QlY3ar+b*5xIN2WQivLQnpJjMVg1C29?%he2A_5fD#*d^zKfg}=r#4t4eH-chB z))TJ5MG;8W34AQy!INKqMc&n~&wp;OMhb4>N#%hz(b(j*?q zJv=1l_e>!yw*0q@ozDLSc!FgjwXt!_iEsoqWPT~+cHR!I6liMzoX|G z4Rv5!q63^4M*i+Qv$to;i;y=si3B#cH}!+d#(DnaM;i>pB|OPJ)2SUjeAYaC@bK{w zvNR;CXkgak(wkQs#oPO@HjB6Or5Og;HLohAAyR5?Ns5BpPl44l#_bwjEEw?uK{zOY zEbRav?XRFQ#`hMHTC#%FBVZQZ486Hcz-Z*Fi-?<|DG2qgQm9Ez{qDO(6aqMeAU{WM z%<6x4J@bYXzeVH{{g2Y0bN+w5vAy#DKW81U+xDvZU+(jXk*~*6CK@v3mm*e_kQ1%; zQXJC~pon;upa(YE8axl355YgOHyGF15H&yF?op61+jJB#8F1PaP?BqYo<8)_Fe!k6 z*L9Sx<92FWY%kraJRhJ#{`AH3Uz$&z9vwb;+&p;l;_;EYg%a+Sb8C9QBR#x)h!oTw z&uq@=x5ks5wctJO$9eeAB-nd3*Azg|#)@WXq-k}M;2{K()DWBRAz2?uqAtUWF411o zKk^?1#I4(VpOQ2-rpO@MjMU|U$n@Ysu z41HXAM)rc?Woq6B-bhLynChm0==tzWlI*y*Vv%XG1pN;qGe*U&kjx+?a3sl${E zBx3-XGysC7E`#b5v{dKGJzA!+Op(ULSXh#DH`3i<;N##ZmizetgWERA-C66Lj3T6f zT1R^3@*^>f8Kwi-#cc-eAQ;jF9=~{GsdMEp+`CCII!h!0(s%?oOLIsDw7va+OTa-f z5gaBZO=kN;yXX36W+QJ`0{Pgad{bwR>(ik8CbHN6e*f9e|0%5XVqo1s2`_en*5vqD zS6_|d=SL47K6?hGTQ&~p0+{ed1e(p(2d$MiYc0L%@wI33Iw{v@{)yl+7dl@makJhnoU%h{9~ z0(<5-(C=v3#6{$dO-zLC;F91%@}8;K?|;P6pgDYdjOz)sFo5+4sXrlNwg2(_$>ZJK z|9TF^pLeZzL%{tG(yRf>CwkDGXujvur;B?&eTtr+FnrI)Rriyo+q!}xu5Fvl?}SXu zBMk1^HieSx!PE?7xK8J;@Pf`pKH8L3SkrBXe3=+^Q}ArV zeImF|EJZ;qW_np1HuBOj?4YOg5vFTmF3e?F%yJ3N79yJ>2mxurNp(-QjJNOMKf?TO zek_oKU`Y{wJYt2N91b9_kZUVFb|>TR9gq3+9lgXZrc37D#`yA(O|&vu~|5xMXU|goj!HL=g)<0DLT}ON(Bd z2J1Y8i6#4;nNf_Xpo&8`rEADjTA~2zKy=K!td(p_UL{DJ2y!oDt~fp|0s?^?;lLJU z9uNsa!Z#FE7;hOxKob!qk|!g0SlTRE zuoFu0h5^xMzE6MJmd3sFtT0nxq0pSfq8SrJlcwIm@y>=y1GN-0Pc^xK#x=a#`mFI2M%-l6n`r$TGK zm25m@(q$FjD;Z#I=7Amh-Mg1y9;GojX|C(mYoLb9Qj|o>GJEFx@4dB$j~@`}wU?g! zO7S$i;qc4H8E_nsZlPO*UGvf-b9t|~g*Bbmxyr&dJ!f9-@%+?8;QVGS>|KeRb0?`-gb|3G3#*8( zCWA!0y5WSK$wrrrf*NmlLy9wx!KV(w6$jPu_OzSX`v;=JuRcPLTwBC1vM8*ehXocJX(~_YmU=7#W-ok!VIXS9XP)Wy za0wC|Mtx2GgPcMS0gxb|VCB?}En$11G0W$vZ;h*AtPXVw2`nW5hW?rK=g4iOUohvH z;L*3oI*@R`QJxVO8lbb^haKE25Je1)DSSF(*Na6ccKt<5jh-2y@O5Ux=lD(k_(nxi0a<|a%}DBU@^I6H+)tUcLF3Afs%r#Gfnc*gs) zk}GfheL94|L7dh%PbeD8;!s}kkPC%uRGQ7E9kCHmpPS{fn1#jm@&Z3~hVvhzwe;}pL_U~4tR zl_-#k*i6%%Z!q_VNL(EM#ofU9`+tpEwX%x;UTFottkG4^|Fn9CDhL*OO=th090J03 z$kSoi^ItRjKb3AQDJD?IEV@Gtm&pfmNh?;veO~V3boSgr3lfAiV+qf2qe|-YU2tna z+Y->st$_qY`qn_|$IrVpklOz|MpQUAbnfFg^OF5vt2AnJ`M-7K1gz?RI|?rE?^X9d zY{&C104vWhB?|k0_7bo-q#-Kl*o!^Oy#Nq)fdE9)P2K_0$=M76+QjJ%Yl*^YlV)0! zQ_Gh?RHD!p%D;pt8=5Z8iJQGRWknfYOPpO8_DH0&En(85Lhuk(ytKNyRPLO@QK2zY zCfxR zy$_xI_q;W`0QY-nE7yVn1O)GEI8(1NBgsz%Je3wQEN!LuqWO&pl&IBtb1P1@it~=z@VHgprd%t2q}iRh0CY`yfH*DgDo7Hpso~waQ>K| znB_RuU{IKcReme%g=C}1Jdb*)P9K{}8OgB8aqoDvi5kI-4-*P0!30V~mn{O&=g4nC zg6@!K9RSm%4{Kwl2-zZeQqCC#HMoM345KWvF!3)!ycQvIK4U#w5O423>$v3|=l{v` zvPTEa`;Q*&9dOcx4$57b!arWWLOS4|=Co}J?M(^&+k&vD#Exj^tA|g1$~|QGX4l-u zOM>;lg z#Ur({9!uM5c@cf~J5@}4)^A09YJIcaqN?LW}{P9H(leJ*G-m^@^JylKCUv z5A3+q+RZWwmW{^(s91oL{61p;IE2T27#JjD^W^{26ww94_aCUlA27L^Or#*esC=mh zaE-- zVlKtC{HL5i5g z$lL=_VgJEeMo}M}y@lgq@_ri2BwD6em>rL`=oILP4Fsq-ES1;(6yr!}VF`(%W*l-{LoK;JHS?|nfUV@~q;O}zyAUj%0OsBho<_a&xYSX22 z5(H;TU~9<$k?o;ynFKFeuJU;c<|-2DcFnQ8stOT{7*`-OrNSI@rU>Zv4Ca|QOnNtq zSDy6POZNp1Kbf=mvdl~jISxa>4iXNPhwY?!#Zkiua0_h3(y5S^C3g~i7YF7R%*Ovq zq0W=)zCV;x;WcAR1SnLMMs)FRnW&S(yrAok4*o<$wmH};drG0H0z znXp_2m55gX;k1~+D6aTB@QhM@;UX_}=+a42kX zXbgw;TAs{S?W3d|Fq<3L@V!Wq2MGF6=b<@%MK&?9VE@1$^+HNrs5G1|z%kWK>omPN zf>!UBY-iGiV%HY{m}B~lYYsWN8H|4TA68#-yE$zjW?xe#I>F9>`4ggpaf_D707bs? ziZXxX3?F?B7=3z>?Kj9#Q5NaBij-tjMr|hToqib_^(#koT(qTRrGoF%oaMz%mD%Sb&+4xQ$GjS9I1l-LajOtt}USz}xL6+}V>Zy&)G zxz6AMN)lkg)PCd*Ywmho5sOlQ$R2r02q^%FiX-SKz)3%D9*?mE8^~NgpQ=L8to4-a z4FP-v9+t5r0447zA!LJE+BB2;5?tgPGZmw$?IXPxY;4rLv_UfsqVXjnP7m4{NLSqw zWFHb;ZawI-%T|_k246^VIa6s)FL!iEU?Cm_Ec`F|GZH(-koJ-!cz=yo z10uY$IfAgb0 zgJpDQ%)+ zr7`EluV5F}50wBuM2XX+GR}dFLkVC}nW_k|8oV2dVwx5#tmlM;Iz0%ii|n^Di7!;* zTW1X@!cmc{js1iQs<|I+!|A|BRM34~YCuKu$|jkZo@V0gIe+GjQj@ynoEDZRq=ots zCjn59x60xjo3hiX0#Xx%-NNQPh0*Dyk*Urjql7IM1sE`;BSf{tO~}(VCj`zHg*j*b zT{)EYmu#4d|EiC2!?`~s$|dLjmF@EU{r~!^{?}LGr`h=L{ltpY;Hj;#4UutC{?BFS z{uH9F@}Is#SB}H%6-FK|Au_fK?%QF@GdrpHQACXC`f~tr33(NnU(#0xb1kQGKN3m99`K7oCJeI;c!k6 z`M{T80kwEmt1)PefNj`X(KB@2H4GSVU>G)x!66ifvk$zNw(ZKuAOci+^U|!nzFpye zOPzJRwk#7{eQ+@z?nw5Zp9w4SAFiT%8`m;~4&}eR&yyb@>-A6aw~Pt^+5E3+b!V0T zwT!jM!km9)+C7&MiR~o^|LZCCSf>C;k{ME2HLKJ8EM@;1-(~-^nC24E<$k~5{V#l+ z-T&pCa^suE5^+jE|C_)6H(CEu@Ih(}CWN`U4_x4j+|(Lc6L_p|GgpdUkHXN te9qRZ7HfBl4YP3{HGBf|Gx(s=sBf;Kffo{R^>9Uj6$Mrl_={5De*tl}8WI2i literal 0 HcmV?d00001 diff --git a/gix-ref/tests/fixtures/make_repo_for_1850_repro.sh b/gix-ref/tests/fixtures/make_repo_for_1850_repro.sh new file mode 100755 index 00000000000..fee6ec02e81 --- /dev/null +++ b/gix-ref/tests/fixtures/make_repo_for_1850_repro.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +set -eu -o pipefail + +git init -q + +cat <.git/packed-refs +# pack-refs with: peeled fully-peeled sorted +17dad46c0ce3be4d4b6d45def031437ab2e40666 refs/heads/ig-branch-remote +83a70366fcc1255d35a00102138293bac673b331 refs/heads/ig-inttest +21b57230833a1733f6685e14eabe936a09689a1b refs/heads/ig-pr4021 +d773228d0ee0012fcca53fffe581b0fce0b1dc56 refs/heads/ig/aliases +ba37abe04f91fec76a6b9a817d40ee2daec47207 refs/heads/ig/cifail +EOF + +mkdir -p .git/refs/heads/ig/pr +echo d22f46f3d7d2504d56c573b5fe54919bd16be48a >.git/refs/heads/ig/push-name +echo 4dec145966c546402c5a9e28b932e7c8c939e01e >.git/refs/heads/ig-pr4021 diff --git a/gix-ref/tests/refs/file/store/iter.rs b/gix-ref/tests/refs/file/store/iter.rs index bcd80ded6cb..70022f69124 100644 --- a/gix-ref/tests/refs/file/store/iter.rs +++ b/gix-ref/tests/refs/file/store/iter.rs @@ -413,6 +413,57 @@ fn overlay_iter() -> crate::Result { Ok(()) } +#[test] +fn overlay_iter_reproduce_1850() -> crate::Result { + let store = store_at("make_repo_for_1850_repro.sh")?; + let ref_names = store + .iter()? + .all()? + .map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target))) + .collect::, _>>()?; + insta::assert_debug_snapshot!(ref_names, @r#" + [ + ( + "refs/heads/ig-branch-remote", + Object( + Sha1(17dad46c0ce3be4d4b6d45def031437ab2e40666), + ), + ), + ( + "refs/heads/ig-inttest", + Object( + Sha1(83a70366fcc1255d35a00102138293bac673b331), + ), + ), + ( + "refs/heads/ig/aliases", + Object( + Sha1(d773228d0ee0012fcca53fffe581b0fce0b1dc56), + ), + ), + ( + "refs/heads/ig/cifail", + Object( + Sha1(ba37abe04f91fec76a6b9a817d40ee2daec47207), + ), + ), + ( + "refs/heads/ig/push-name", + Object( + Sha1(d22f46f3d7d2504d56c573b5fe54919bd16be48a), + ), + ), + ( + "refs/heads/ig-pr4021", + Object( + Sha1(4dec145966c546402c5a9e28b932e7c8c939e01e), + ), + ), + ] + "#); + Ok(()) +} + #[test] fn overlay_iter_with_prefix_wont_allow_absolute_paths() -> crate::Result { let store = store_with_packed_refs()?; From 7b1b5bf864e74706aefeb1213e8bdb0545d5464a Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 21 Feb 2025 09:35:43 +0100 Subject: [PATCH 2/4] fix!: `fs::walkdir_sorted_new` now returns files first. It's only used by `gix_ref` for reference traversal, which now needs this to be sorted differently to work correctly. As this isn't easily possible in the parallel walk implementation, it's hereby removed, along with the `fs-walkdir-parallel` feature toggle. --- gix-features/Cargo.toml | 4 -- gix-features/src/fs.rs | 150 ++++++---------------------------------- 2 files changed, 21 insertions(+), 133 deletions(-) diff --git a/gix-features/Cargo.toml b/gix-features/Cargo.toml index 7a5d598bc6f..67983ee4f34 100644 --- a/gix-features/Cargo.toml +++ b/gix-features/Cargo.toml @@ -26,9 +26,6 @@ progress-unit-human-numbers = ["prodash?/unit-human"] ## Provide human readable byte units for progress bars. progress-unit-bytes = ["dep:bytesize", "prodash?/unit-bytes"] -## If set, walkdir iterators will be multi-threaded. -fs-walkdir-parallel = ["dep:jwalk", "dep:gix-utils"] - ## Provide utilities suitable for working with the `std::fs::read_dir()`. fs-read-dir = ["dep:gix-utils"] @@ -131,7 +128,6 @@ gix-utils = { version = "^0.1.14", path = "../gix-utils", optional = true } crossbeam-channel = { version = "0.5.0", optional = true } parking_lot = { version = "0.12.0", default-features = false, optional = true } -jwalk = { version = "0.8.1", optional = true } walkdir = { version = "2.3.2", optional = true } # used when parallel is off # hashing and 'fast-sha1' feature diff --git a/gix-features/src/fs.rs b/gix-features/src/fs.rs index 6f9d2191d67..5cf5b7e1afa 100644 --- a/gix-features/src/fs.rs +++ b/gix-features/src/fs.rs @@ -7,7 +7,7 @@ //! * [`jwalk::WalkDir`](https://docs.rs/jwalk/0.5.1/jwalk/type.WalkDir.html) if `parallel` feature is enabled //! * [walkdir::WalkDir](https://docs.rs/walkdir/2.3.1/walkdir/struct.WalkDir.html) otherwise -#[cfg(any(feature = "walkdir", feature = "fs-walkdir-parallel"))] +#[cfg(feature = "walkdir")] mod shared { /// The desired level of parallelism. pub enum Parallelism { @@ -21,7 +21,7 @@ mod shared { } } -#[cfg(any(feature = "walkdir", feature = "fs-walkdir-parallel", feature = "fs-read-dir"))] +#[cfg(any(feature = "walkdir", feature = "fs-read-dir"))] mod walkdir_precompose { use std::borrow::Cow; use std::ffi::OsStr; @@ -83,13 +83,13 @@ mod walkdir_precompose { /// A platform over entries in a directory, which may or may not precompose unicode after retrieving /// paths from the file system. - #[cfg(any(feature = "walkdir", feature = "fs-walkdir-parallel"))] + #[cfg(feature = "walkdir")] pub struct WalkDir { pub(crate) inner: Option, pub(crate) precompose_unicode: bool, } - #[cfg(any(feature = "walkdir", feature = "fs-walkdir-parallel"))] + #[cfg(feature = "walkdir")] pub struct WalkDirIter where T: Iterator>, @@ -99,7 +99,7 @@ mod walkdir_precompose { pub(crate) precompose_unicode: bool, } - #[cfg(any(feature = "walkdir", feature = "fs-walkdir-parallel"))] + #[cfg(feature = "walkdir")] impl Iterator for WalkDirIter where T: Iterator>, @@ -142,128 +142,7 @@ pub mod read_dir { } /// -#[cfg(feature = "fs-walkdir-parallel")] -pub mod walkdir { - use std::borrow::Cow; - use std::ffi::OsStr; - use std::fs::FileType; - use std::path::Path; - - use jwalk::WalkDir as WalkDirImpl; - pub use jwalk::{DirEntry as DirEntryGeneric, DirEntryIter as DirEntryIterGeneric, Error}; - - pub use super::shared::Parallelism; - - type DirEntryImpl = DirEntryGeneric<((), ())>; - - /// A directory entry returned by [DirEntryIter]. - pub type DirEntry = super::walkdir_precompose::DirEntry; - /// A platform to create a [DirEntryIter] from. - pub type WalkDir = super::walkdir_precompose::WalkDir; - - impl super::walkdir_precompose::DirEntryApi for DirEntryImpl { - fn path(&self) -> Cow<'_, Path> { - self.path().into() - } - - fn file_name(&self) -> Cow<'_, OsStr> { - self.file_name().into() - } - - fn file_type(&self) -> std::io::Result { - Ok(self.file_type()) - } - } - - impl IntoIterator for WalkDir { - type Item = Result; - type IntoIter = DirEntryIter; - - fn into_iter(self) -> Self::IntoIter { - DirEntryIter { - inner: self.inner.expect("always set (builder fix)").into_iter(), - precompose_unicode: self.precompose_unicode, - } - } - } - - impl WalkDir { - /// Set the minimum component depth of paths of entries. - pub fn min_depth(mut self, min: usize) -> Self { - self.inner = Some(self.inner.take().expect("always set").min_depth(min)); - self - } - /// Set the maximum component depth of paths of entries. - pub fn max_depth(mut self, max: usize) -> Self { - self.inner = Some(self.inner.take().expect("always set").max_depth(max)); - self - } - /// Follow symbolic links. - pub fn follow_links(mut self, toggle: bool) -> Self { - self.inner = Some(self.inner.take().expect("always set").follow_links(toggle)); - self - } - } - - impl From for jwalk::Parallelism { - fn from(v: Parallelism) -> Self { - match v { - Parallelism::Serial => jwalk::Parallelism::Serial, - Parallelism::ThreadPoolPerTraversal { thread_name } => std::thread::available_parallelism() - .map_or_else( - |_| Parallelism::Serial.into(), - |threads| { - let pool = jwalk::rayon::ThreadPoolBuilder::new() - .num_threads(threads.get().min(16)) - .stack_size(128 * 1024) - .thread_name(move |idx| format!("{thread_name} {idx}")) - .build() - .expect("we only set options that can't cause a build failure"); - jwalk::Parallelism::RayonExistingPool { - pool: pool.into(), - busy_timeout: None, - } - }, - ), - } - } - } - - /// Instantiate a new directory iterator which will not skip hidden files, with the given level of `parallelism`. - /// - /// Use `precompose_unicode` to represent the `core.precomposeUnicode` configuration option. - pub fn walkdir_new(root: &Path, parallelism: Parallelism, precompose_unicode: bool) -> WalkDir { - WalkDir { - inner: WalkDirImpl::new(root) - .skip_hidden(false) - .parallelism(parallelism.into()) - .into(), - precompose_unicode, - } - } - - /// Instantiate a new directory iterator which will not skip hidden files and is sorted - /// - /// Use `precompose_unicode` to represent the `core.precomposeUnicode` configuration option. - pub fn walkdir_sorted_new(root: &Path, parallelism: Parallelism, precompose_unicode: bool) -> WalkDir { - WalkDir { - inner: WalkDirImpl::new(root) - .skip_hidden(false) - .sort(true) - .parallelism(parallelism.into()) - .into(), - precompose_unicode, - } - } - - type DirEntryIterImpl = DirEntryIterGeneric<((), ())>; - - /// The Iterator yielding directory items - pub type DirEntryIter = super::walkdir_precompose::WalkDirIter; -} - -/// -#[cfg(all(feature = "walkdir", not(feature = "fs-walkdir-parallel")))] +#[cfg(feature = "walkdir")] pub mod walkdir { use std::borrow::Cow; use std::ffi::OsStr; @@ -338,8 +217,21 @@ pub mod walkdir { /// /// Use `precompose_unicode` to represent the `core.precomposeUnicode` configuration option. pub fn walkdir_sorted_new(root: &Path, _: Parallelism, precompose_unicode: bool) -> WalkDir { + fn ft_to_number(ft: std::fs::FileType) -> usize { + if ft.is_file() { + 1 + } else { + 2 + } + } WalkDir { - inner: WalkDirImpl::new(root).sort_by_file_name().into(), + inner: WalkDirImpl::new(root) + .sort_by(|a, b| { + ft_to_number(a.file_type()) + .cmp(&ft_to_number(b.file_type())) + .then_with(|| a.file_name().cmp(b.file_name())) + }) + .into(), precompose_unicode, } } @@ -348,7 +240,7 @@ pub mod walkdir { pub type DirEntryIter = super::walkdir_precompose::WalkDirIter; } -#[cfg(any(feature = "walkdir", feature = "fs-walkdir-parallel"))] +#[cfg(feature = "walkdir")] pub use self::walkdir::{walkdir_new, walkdir_sorted_new, WalkDir}; /// Prepare open options which won't follow symlinks when the file is opened. From 56ba8986675b6f3c3032fd48a3498a10c63d65aa Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 21 Feb 2025 09:30:57 +0100 Subject: [PATCH 3/4] adjust expectations according to changed sort-order --- gix-ref/tests/refs/file/store/iter.rs | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/gix-ref/tests/refs/file/store/iter.rs b/gix-ref/tests/refs/file/store/iter.rs index 70022f69124..1d32428f07e 100644 --- a/gix-ref/tests/refs/file/store/iter.rs +++ b/gix-ref/tests/refs/file/store/iter.rs @@ -31,8 +31,8 @@ mod with_namespace { .map(|r: gix_ref::Reference| r.name) .collect::>(); let expected_namespaced_refs = vec![ - "refs/namespaces/bar/refs/heads/multi-link-target1", "refs/namespaces/bar/refs/multi-link", + "refs/namespaces/bar/refs/heads/multi-link-target1", "refs/namespaces/bar/refs/remotes/origin/multi-link-target3", "refs/namespaces/bar/refs/tags/multi-link-target2", ]; @@ -50,8 +50,8 @@ mod with_namespace { .map(|r| r.name.into_inner()) .collect::>(), [ - "refs/namespaces/bar/refs/heads/multi-link-target1", "refs/namespaces/bar/refs/multi-link", + "refs/namespaces/bar/refs/heads/multi-link-target1", "refs/namespaces/bar/refs/tags/multi-link-target2" ] ); @@ -149,8 +149,8 @@ mod with_namespace { let packed = ns_store.open_packed_buffer()?; let expected_refs = vec![ - "refs/heads/multi-link-target1", "refs/multi-link", + "refs/heads/multi-link-target1", "refs/remotes/origin/multi-link-target3", "refs/tags/multi-link-target2", ]; @@ -198,8 +198,8 @@ mod with_namespace { .map(|r| r.name.into_inner()) .collect::>(), [ - "refs/heads/multi-link-target1", "refs/multi-link", + "refs/heads/multi-link-target1", "refs/tags/multi-link-target2", ], "loose iterators have namespace support as well" @@ -214,8 +214,8 @@ mod with_namespace { .map(|r| r.name.into_inner()) .collect::>(), [ - "refs/namespaces/bar/refs/heads/multi-link-target1", "refs/namespaces/bar/refs/multi-link", + "refs/namespaces/bar/refs/heads/multi-link-target1", "refs/namespaces/bar/refs/tags/multi-link-target2", "refs/namespaces/foo/refs/remotes/origin/HEAD" ], @@ -291,14 +291,14 @@ fn loose_iter_with_broken_refs() -> crate::Result { ref_paths, vec![ "d1", + "loop-a", + "loop-b", + "multi-link", "heads/A", "heads/d1", "heads/dt1", "heads/main", "heads/multi-link-target1", - "loop-a", - "loop-b", - "multi-link", "remotes/origin/HEAD", "remotes/origin/main", "remotes/origin/multi-link-target3", @@ -435,6 +435,12 @@ fn overlay_iter_reproduce_1850() -> crate::Result { Sha1(83a70366fcc1255d35a00102138293bac673b331), ), ), + ( + "refs/heads/ig-pr4021", + Object( + Sha1(4dec145966c546402c5a9e28b932e7c8c939e01e), + ), + ), ( "refs/heads/ig/aliases", Object( @@ -453,12 +459,6 @@ fn overlay_iter_reproduce_1850() -> crate::Result { Sha1(d22f46f3d7d2504d56c573b5fe54919bd16be48a), ), ), - ( - "refs/heads/ig-pr4021", - Object( - Sha1(4dec145966c546402c5a9e28b932e7c8c939e01e), - ), - ), ] "#); Ok(()) From 5f8bff844420a2ea1fb1f949650451d235251185 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sat, 22 Feb 2025 19:56:14 +0100 Subject: [PATCH 4/4] adapt to changes in `gix-features` --- .github/workflows/ci.yml | 2 +- Cargo.lock | 1 - gix/Cargo.toml | 5 ----- gix/tests/gix/repository/reference.rs | 2 +- justfile | 1 - 5 files changed, 2 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 852f361feb3..21db6b6374e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -391,7 +391,7 @@ jobs: - name: features of gix-features run: | set +x - for feature in progress fs-walkdir-parallel parallel io-pipe crc32 zlib zlib-rust-backend fast-sha1 rustsha1 cache-efficiency-debug; do + for feature in progress parallel io-pipe crc32 zlib zlib-rust-backend fast-sha1 rustsha1 cache-efficiency-debug; do (cd gix-features && cargo build --features "$feature" --target "$TARGET") done - name: crates with 'wasm' feature diff --git a/Cargo.lock b/Cargo.lock index 6d0431cc970..4bd5b1712ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1849,7 +1849,6 @@ dependencies = [ "gix-hash 0.16.0", "gix-trace 0.1.12", "gix-utils 0.1.14", - "jwalk", "libc", "once_cell", "parking_lot", diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 8da874b8d6f..5e0a0d2a31a 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -230,11 +230,6 @@ max-control = ["parallel", "pack-cache-lru-static", "pack-cache-lru-dynamic"] ## No C toolchain is involved. max-performance-safe = ["max-control"] -## If set, walkdir iterators will be multi-threaded which affects the listing of loose objects and references. -## Note, however, that this will use `rayon` under the hood and spawn threads for each traversal to avoid a global rayon thread pool. -## Thus this option is more interesting to one-off client applications, rather than the server. -parallel-walkdir = ["gix-features/fs-walkdir-parallel"] - ## The tempfile registry uses a better implementation of a thread-safe hashmap, relying on an external crate. ## This may be useful when tempfiles are created and accessed in a massively parallel fashion and you know that this is indeed faster than ## the simpler implementation that is the default. diff --git a/gix/tests/gix/repository/reference.rs b/gix/tests/gix/repository/reference.rs index 94e9e8574d2..acea4e016ed 100644 --- a/gix/tests/gix/repository/reference.rs +++ b/gix/tests/gix/repository/reference.rs @@ -102,10 +102,10 @@ mod iter_references { "refs/heads/d1", "refs/heads/dt1", "refs/heads/main", - "refs/heads/multi-link-target1", "refs/loop-a", "refs/loop-b", "refs/multi-link", + "refs/heads/multi-link-target1", "refs/remotes/origin/HEAD", "refs/remotes/origin/main", "refs/remotes/origin/multi-link-target3", diff --git a/justfile b/justfile index 76d209aa577..da837e97e13 100755 --- a/justfile +++ b/justfile @@ -98,7 +98,6 @@ check: cargo check -p gix-status --all-features cargo check -p gix-features --all-features cargo check -p gix-features --features parallel - cargo check -p gix-features --features fs-walkdir-parallel cargo check -p gix-features --features fs-read-dir cargo check -p gix-features --features rustsha1 cargo check -p gix-features --features fast-sha1