From 8249aec6622c64f868a3da83ab798bf82546e2e4 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Tue, 2 Apr 2024 18:37:11 +0200 Subject: [PATCH 01/12] hg: refactor for later evolutions Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/hg.go | 68 +++++++++++++++++++++++++------------ pkg/vendir/fetch/hg/sync.go | 6 +++- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index 0bf11e4d..9969535a 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -21,12 +21,19 @@ type Hg struct { opts ctlconf.DirectoryContentsHg infoLog io.Writer refFetcher ctlfetch.RefFetcher + authDir string + env []string } func NewHg(opts ctlconf.DirectoryContentsHg, - infoLog io.Writer, refFetcher ctlfetch.RefFetcher) *Hg { - - return &Hg{opts, infoLog, refFetcher} + infoLog io.Writer, refFetcher ctlfetch.RefFetcher, + tempArea ctlfetch.TempArea, +) (*Hg, error) { + t := Hg{opts, infoLog, refFetcher, "", nil} + if err := t.setup(tempArea); err != nil { + return nil, err + } + return &t, nil } //nolint:revive @@ -48,14 +55,14 @@ func (t *Hg) Retrieve(dstPath string, tempArea ctlfetch.TempArea) (HgInfo, error info := HgInfo{} // use hg log to retrieve full cset sha - out, _, err := t.run([]string{"log", "-r", ".", "-T", "{node}"}, nil, dstPath) + out, _, err := t.run([]string{"log", "-r", ".", "-T", "{node}"}, dstPath) if err != nil { return HgInfo{}, err } info.SHA = strings.TrimSpace(out) - out, _, err = t.run([]string{"log", "-l", "1", "-T", "{desc|firstline|strip}", "-r", info.SHA}, nil, dstPath) + out, _, err = t.run([]string{"log", "-l", "1", "-T", "{desc|firstline|strip}", "-r", info.SHA}, dstPath) if err != nil { return HgInfo{}, err } @@ -65,7 +72,14 @@ func (t *Hg) Retrieve(dstPath string, tempArea ctlfetch.TempArea) (HgInfo, error return info, nil } -func (t *Hg) fetch(dstPath string, tempArea ctlfetch.TempArea) error { +func (t *Hg) Close() { + if t.authDir != "" { + os.RemoveAll(t.authDir) + t.authDir = "" + } +} + +func (t *Hg) setup(tempArea ctlfetch.TempArea) error { authOpts, err := t.getAuthOpts() if err != nil { return err @@ -76,17 +90,12 @@ func (t *Hg) fetch(dstPath string, tempArea ctlfetch.TempArea) error { return err } - defer os.RemoveAll(authDir) + t.authDir = authDir - env := os.Environ() + t.env = os.Environ() hgURL := t.opts.URL - _, _, err = t.run([]string{"init"}, env, dstPath) - if err != nil { - return err - } - var hgRc string if t.opts.Evolve { @@ -147,27 +156,44 @@ hgauth.password = %s if err != nil { return fmt.Errorf("Writing %s: %s", hgRcPath, err) } - env = append(env, "HGRCPATH="+hgRcPath) + t.env = append(t.env, "HGRCPATH="+hgRcPath) + } + + return nil +} + +func (t *Hg) initClone(dstPath string) error { + hgURL := t.opts.URL + + if _, _, err := t.run([]string{"init"}, dstPath); err != nil { + return err } repoHgRcPath := filepath.Join(dstPath, ".hg", "hgrc") repoHgRc := fmt.Sprintf("[paths]\ndefault = %s\n", hgURL) - err = os.WriteFile(repoHgRcPath, []byte(repoHgRc), 0600) - if err != nil { + if err := os.WriteFile(repoHgRcPath, []byte(repoHgRc), 0600); err != nil { return fmt.Errorf("Writing %s: %s", repoHgRcPath, err) } + return nil +} + +func (t *Hg) fetch(dstPath string, tempArea ctlfetch.TempArea) error { + if err := t.initClone(dstPath); err != nil { + return err + } + return t.runMultiple([][]string{ {"pull"}, {"checkout", t.opts.Ref}, - }, env, dstPath) + }, dstPath) } -func (t *Hg) runMultiple(argss [][]string, env []string, dstPath string) error { +func (t *Hg) runMultiple(argss [][]string, dstPath string) error { for _, args := range argss { - _, _, err := t.run(args, env, dstPath) + _, _, err := t.run(args, dstPath) if err != nil { return err } @@ -175,11 +201,11 @@ func (t *Hg) runMultiple(argss [][]string, env []string, dstPath string) error { return nil } -func (t *Hg) run(args []string, env []string, dstPath string) (string, string, error) { +func (t *Hg) run(args []string, dstPath string) (string, string, error) { var stdoutBs, stderrBs bytes.Buffer cmd := exec.Command("hg", args...) - cmd.Env = env + cmd.Env = t.env cmd.Dir = dstPath cmd.Stdout = io.MultiWriter(t.infoLog, &stdoutBs) cmd.Stderr = io.MultiWriter(t.infoLog, &stderrBs) diff --git a/pkg/vendir/fetch/hg/sync.go b/pkg/vendir/fetch/hg/sync.go index abfb44fe..c1fefe85 100644 --- a/pkg/vendir/fetch/hg/sync.go +++ b/pkg/vendir/fetch/hg/sync.go @@ -44,7 +44,11 @@ func (d Sync) Sync(dstPath string, tempArea ctlfetch.TempArea) (ctlconf.LockDire defer os.RemoveAll(incomingTmpPath) - hg := NewHg(d.opts, d.log, d.refFetcher) + hg, err := NewHg(d.opts, d.log, d.refFetcher, tempArea) + if err != nil { + return hgLockConf, err + } + defer hg.Close() info, err := hg.Retrieve(incomingTmpPath, tempArea) if err != nil { From a606d2c5d55ff8910c59c2f25a38a146d8a7c295 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Tue, 2 Apr 2024 19:41:11 +0200 Subject: [PATCH 02/12] hg: Add a cache for mercurial repositories. The idea is to save the whole untouched clone (with no checkout) in the cache. If already present, the pull is done directly in the cache, and is faster (except on very small repos) because only new changeset are transfered. If the ref is a changeset id (not a tag, branch, topic or bookmark), and the changeset is already known in the cached clone, no pull is done which avoid any network exchange. Then we copy the cached entry and do the checkout. Signed-off-by: Christophe de Vienne --- pkg/vendir/directory/directory.go | 2 +- pkg/vendir/fetch/hg/hg.go | 61 ++++++++++++++++++++++++++++--- pkg/vendir/fetch/hg/sync.go | 34 ++++++++++++++--- 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/pkg/vendir/directory/directory.go b/pkg/vendir/directory/directory.go index be2f89cf..1a06f88d 100644 --- a/pkg/vendir/directory/directory.go +++ b/pkg/vendir/directory/directory.go @@ -119,7 +119,7 @@ func (d *Directory) Sync(syncOpts SyncOpts) (ctlconf.LockDirectory, error) { lockDirContents.Git = &lock case contents.Hg != nil: - hgSync := ctlhg.NewSync(*contents.Hg, NewInfoLog(d.ui), syncOpts.RefFetcher) + hgSync := ctlhg.NewSync(*contents.Hg, NewInfoLog(d.ui), syncOpts.RefFetcher, syncOpts.Cache) d.ui.PrintLinef("Fetching: %s + %s (hg from %s)", d.opts.Path, contents.Path, hgSync.Desc()) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index 9969535a..04c5c379 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -23,32 +23,74 @@ type Hg struct { refFetcher ctlfetch.RefFetcher authDir string env []string + cacheID string } func NewHg(opts ctlconf.DirectoryContentsHg, infoLog io.Writer, refFetcher ctlfetch.RefFetcher, tempArea ctlfetch.TempArea, ) (*Hg, error) { - t := Hg{opts, infoLog, refFetcher, "", nil} + t := Hg{opts, infoLog, refFetcher, "", nil, ""} if err := t.setup(tempArea); err != nil { return nil, err } return &t, nil } +// CacheID returns a cache id for the repository +// It doesn't include the ref because we want to reuse a cache when only the ref +// is changed +// Basically we combine all data used to write the hgrc file +func (t *Hg) CacheID() string { + return t.cacheID +} + //nolint:revive type HgInfo struct { SHA string ChangeSetTitle string } -func (t *Hg) Retrieve(dstPath string, tempArea ctlfetch.TempArea) (HgInfo, error) { - if len(t.opts.URL) == 0 { - return HgInfo{}, fmt.Errorf("Expected non-empty URL") +// CloneHasTargetRef returns true if the given clone contains the target +// ref, and this ref is a revision id (not a tag or a branch) +func (t *Hg) CloneHasTargetRef(dstPath string) bool { + out, _, err := t.run([]string{"id", "--id", "-r", t.opts.Ref}, dstPath) + if err != nil { + return false } + out = strings.TrimSpace(out) + if strings.HasPrefix(t.opts.Ref, out) { + return true + } + return false +} - err := t.fetch(dstPath, tempArea) - if err != nil { +func (t *Hg) Clone(dstPath string) error { + if err := t.initClone(dstPath); err != nil { + return err + } + return t.SyncClone(dstPath) +} + +func (t *Hg) SyncClone(dstPath string) error { + if _, _, err := t.run([]string{"pull"}, dstPath); err != nil { + return err + } + return nil +} + +func (t *Hg) LocalClone(localClone, dstPath string) error { + if err := t.initClone(dstPath); err != nil { + return err + } + if _, _, err := t.run([]string{"pull", localClone}, dstPath); err != nil { + return err + } + return nil +} + +func (t *Hg) Checkout(dstPath string) (HgInfo, error) { + if _, _, err := t.run([]string{"checkout", t.opts.Ref}, dstPath); err != nil { return HgInfo{}, err } @@ -80,6 +122,10 @@ func (t *Hg) Close() { } func (t *Hg) setup(tempArea ctlfetch.TempArea) error { + if len(t.opts.URL) == 0 { + return fmt.Errorf("Expected non-empty URL") + } + authOpts, err := t.getAuthOpts() if err != nil { return err @@ -132,6 +178,7 @@ hgauth.password = %s } sshCmd = append(sshCmd, "-i", path, "-o", "IdentitiesOnly=yes") + t.cacheID += "private-key=" + *authOpts.PrivateKey + "|" } if authOpts.KnownHosts != nil { @@ -143,6 +190,7 @@ hgauth.password = %s } sshCmd = append(sshCmd, "-o", "StrictHostKeyChecking=yes", "-o", "UserKnownHostsFile="+path) + t.cacheID += "known-hosts=" + *authOpts.KnownHosts + "|" } else { sshCmd = append(sshCmd, "-o", "StrictHostKeyChecking=no") } @@ -157,6 +205,7 @@ hgauth.password = %s return fmt.Errorf("Writing %s: %s", hgRcPath, err) } t.env = append(t.env, "HGRCPATH="+hgRcPath) + t.cacheID += hgRc } return nil diff --git a/pkg/vendir/fetch/hg/sync.go b/pkg/vendir/fetch/hg/sync.go index c1fefe85..b401c735 100644 --- a/pkg/vendir/fetch/hg/sync.go +++ b/pkg/vendir/fetch/hg/sync.go @@ -11,18 +11,20 @@ import ( ctlconf "carvel.dev/vendir/pkg/vendir/config" ctlfetch "carvel.dev/vendir/pkg/vendir/fetch" + ctlcache "carvel.dev/vendir/pkg/vendir/fetch/cache" ) type Sync struct { opts ctlconf.DirectoryContentsHg log io.Writer refFetcher ctlfetch.RefFetcher + cache ctlcache.Cache } func NewSync(opts ctlconf.DirectoryContentsHg, - log io.Writer, refFetcher ctlfetch.RefFetcher) Sync { + log io.Writer, refFetcher ctlfetch.RefFetcher, cache ctlcache.Cache) Sync { - return Sync{opts, log, refFetcher} + return Sync{opts, log, refFetcher, cache} } func (d Sync) Desc() string { @@ -46,13 +48,35 @@ func (d Sync) Sync(dstPath string, tempArea ctlfetch.TempArea) (ctlconf.LockDire hg, err := NewHg(d.opts, d.log, d.refFetcher, tempArea) if err != nil { - return hgLockConf, err + return hgLockConf, fmt.Errorf("Setting up hg: %w", err) } defer hg.Close() - info, err := hg.Retrieve(incomingTmpPath, tempArea) + if cachePath, ok := d.cache.Has("hg", hg.CacheID()); ok { + // Sync directly in the cache if needed + if !hg.CloneHasTargetRef(cachePath) { + if err := hg.SyncClone(cachePath); err != nil { + return hgLockConf, fmt.Errorf("Syncing hg cached clone: %w", err) + } + } + // fetch from cachedDir + if err := d.cache.CopyFrom("hg", hg.CacheID(), incomingTmpPath); err != nil { + return hgLockConf, fmt.Errorf("Extracting cached hg clone: %w", err) + } + } else { + // fetch in the target directory, and save it to cache + if err := hg.Clone(incomingTmpPath); err != nil { + return hgLockConf, fmt.Errorf("Cloning hg repository: %w", err) + } + if err := d.cache.Save("hg", hg.CacheID(), incomingTmpPath); err != nil { + return hgLockConf, fmt.Errorf("Saving hg repository to cache: %w", err) + } + } + + // now checkout the wanted revision + info, err := hg.Checkout(incomingTmpPath) if err != nil { - return hgLockConf, fmt.Errorf("Fetching hg repository: %s", err) + return hgLockConf, fmt.Errorf("Checking out hg repository: %s", err) } hgLockConf.SHA = info.SHA From d2b9218abfdce9ac1dae63cb8c1e547a99f694eb Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Tue, 2 Apr 2024 22:42:25 +0200 Subject: [PATCH 03/12] hg: code cleaning: drop unused function Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/hg.go | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index 04c5c379..df035cff 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -229,27 +229,6 @@ func (t *Hg) initClone(dstPath string) error { return nil } -func (t *Hg) fetch(dstPath string, tempArea ctlfetch.TempArea) error { - if err := t.initClone(dstPath); err != nil { - return err - } - - return t.runMultiple([][]string{ - {"pull"}, - {"checkout", t.opts.Ref}, - }, dstPath) -} - -func (t *Hg) runMultiple(argss [][]string, dstPath string) error { - for _, args := range argss { - _, _, err := t.run(args, dstPath) - if err != nil { - return err - } - } - return nil -} - func (t *Hg) run(args []string, dstPath string) (string, string, error) { var stdoutBs, stderrBs bytes.Buffer From 299570d52a7327e86e0150981777a9a900273200 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Tue, 9 Apr 2024 18:44:08 +0200 Subject: [PATCH 04/12] hg: fix cacheID construction The repo URL must be in the cache id. The ref is purposely not included in it because we want to reuse the cached repository when the ref moves. And finally, we use a sha256 hash to mask any authentication data because we don't want them to be readable in the cache folder name. Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/hg.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index df035cff..73953175 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -5,6 +5,8 @@ package hg import ( "bytes" + "crypto/sha256" + "encoding/hex" "fmt" "io" "net/url" @@ -126,6 +128,8 @@ func (t *Hg) setup(tempArea ctlfetch.TempArea) error { return fmt.Errorf("Expected non-empty URL") } + cacheID := t.opts.URL + authOpts, err := t.getAuthOpts() if err != nil { return err @@ -178,7 +182,7 @@ hgauth.password = %s } sshCmd = append(sshCmd, "-i", path, "-o", "IdentitiesOnly=yes") - t.cacheID += "private-key=" + *authOpts.PrivateKey + "|" + cacheID += "private-key=" + *authOpts.PrivateKey + "|" } if authOpts.KnownHosts != nil { @@ -190,7 +194,7 @@ hgauth.password = %s } sshCmd = append(sshCmd, "-o", "StrictHostKeyChecking=yes", "-o", "UserKnownHostsFile="+path) - t.cacheID += "known-hosts=" + *authOpts.KnownHosts + "|" + cacheID += "known-hosts=" + *authOpts.KnownHosts + "|" } else { sshCmd = append(sshCmd, "-o", "StrictHostKeyChecking=no") } @@ -205,9 +209,12 @@ hgauth.password = %s return fmt.Errorf("Writing %s: %s", hgRcPath, err) } t.env = append(t.env, "HGRCPATH="+hgRcPath) - t.cacheID += hgRc + cacheID += hgRc } + sha := sha256.Sum256([]byte(cacheID)) + t.cacheID = hex.EncodeToString(sha[:]) + return nil } From 235398e188a29d98b3322973db05f4e45fd63108 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Tue, 9 Apr 2024 19:50:35 +0200 Subject: [PATCH 05/12] hg-cache: add a e2e test Signed-off-by: Christophe de Vienne --- test/e2e/assets/hg-repos/README.md | 6 ++ test/e2e/assets/hg-repos/asset.tgz | Bin 0 -> 2677 bytes test/e2e/assets/hg-repos/build.sh | 44 ++++++++ test/e2e/hg_test.go | 167 +++++++++++++++++++++++++++++ 4 files changed, 217 insertions(+) create mode 100644 test/e2e/assets/hg-repos/README.md create mode 100644 test/e2e/assets/hg-repos/asset.tgz create mode 100755 test/e2e/assets/hg-repos/build.sh create mode 100644 test/e2e/hg_test.go diff --git a/test/e2e/assets/hg-repos/README.md b/test/e2e/assets/hg-repos/README.md new file mode 100644 index 00000000..ce1330fe --- /dev/null +++ b/test/e2e/assets/hg-repos/README.md @@ -0,0 +1,6 @@ +``build.sh`` assemble a mercurial repository and more, suitable for testing the hg +fetcher cache feature: + +- A base repository (repo) +- A bundle with an extra changeset +- A json file with the changeset ids we need in the test, and the bundle filename diff --git a/test/e2e/assets/hg-repos/asset.tgz b/test/e2e/assets/hg-repos/asset.tgz new file mode 100644 index 0000000000000000000000000000000000000000..0f4cdca922e8990bb27ba572bf1cae929f6cd548 GIT binary patch literal 2677 zcmV-*3X1g~iwFP!000001MQp%bQ8xJ$5$AP(Zd7?N4U2Jj0wo>y;`ms8_ZG6@d!78 ztaeA1unwbL83ReMslkscwlVa@TsB95gc5GbQ3ALL7Q|C1z1LH}!km;a{=ykG%As0P*Zzj7W<(F_g!uLb`8R~|x=&FV-pC?8BBkZW$k z<6wrNp3(oxc{t1BivF>hn^p(V;cua^HQ z=zmT4Kka|OY?Q};_SyYU;Iwi&N(4R>`2+p$<9|Evu$qA+o82~>3{vH?(i+)NYyWAC z^zuLJ@ju2AQ2$zxFq>L2_Q#JVHVNNp=9?c|^`Nk*qf$(otu}y1%c(L#5jew$z{r>| zqrfvf!|`SgDE?D^1Yl%M6vN>r8rPq{SAWdgdz&rn*>?7qkpnM+V#F8kRnYP35;P)jQ;ku%Xms@QT~|+mcMstbh19sC-kYI?PvbKU$6&jhL`?Dt4q;> z9QX>Z+W(`mC-slhQ2!8!26*Y;1*W;JE+7TmhN}K?(x?AV;}`?=uLY8YcL9UMn?dc4 zW02>=OaH-2|2{|lpCUcuKbZdwK7RgpnFND_w*$$Lf(OWkdj5~~&HpF?0Ji^HAObV* zwgn}#0;vV~>A!ryY<4(BAO)xaP}M)<+y1i{O+x)^LFrS#ZP;%c4zrzoIOEs~dC+>- z%3EE(rH9*3yy*Ld1ePk}zYC-UF#N0QpY)yoXGjKz`qzTIf1lmwIePT&IcCnuZu4UA z2%;r^`Zu|FhhQNLCSD3uRaetLPBOmwr)k*#*8~!01QH|7JVmi0U??L?P%PkZ+KgkA zNYesmG*dLip}78;Pj#XdqY9W(dg<}gKS*(cRN3G(NoCIsSj|8vLr+0IvG)@?OJZr$<7kyKZ;Nz!%&*+~pq*x`Z$rhlxUcLCwl&$|_X{diK zz|m(7>w1ne<056^c(a-0NJ=mQjK>8)nRtfaNt4LZENS8>GivrMBrqry3C91I9xwfu z_xyukLw)|&Z~n)#Bn;|b17J`6*NvC{CD~<7R>u88k~=w|)4LF= z|2thT%ltpZ!u_9`;OBp{qvi{~eNc`6l)wI282>dv5INU zNO1Afe-KuIs*e8&-}Rpi0r!7tgXoljLvV^fU(xXE6M+M`c$)}pGOu@m6q_^AAUN&G zN}Z6bPKRNpB#Ra0DY#xr=3Npff2uF5L9ofFP59{_!zwp`MOy=8Lrwpl$^Iwze=rOV z=l``K1kqLeMJW%mvbb^`&{Ot@J;MFG8>XnT-({P$D&9(0Y4<6cxvB!M_6DzjeQ>`F z{Pdq};g$B>>68Q415oY%W4`;p7@CFp*MutCf5E~#5`hxW0~G+&{O{T9?K}TN!To>Q zpnKH*rznr~%HqoQ-?Kh!>wj{GzNc?{ZNF*VLASJTxcB8zyL&u9)9oP$k@;Hdw0+@4 z4+rdUbc~qTedjt|(w4b@U~&qjzopD7S@^T<`$&{ocyz>+e_rKIruANNdi$91BM;w~ z<`o_YpMXB-GkxBFhmCDCG&ecygEULlo-K!(IEJ{l%pKQZ^oAuF`OV{+&it-f9V8|y z`?KC5kM0*i<|~i2NO|;l6hYKxv&-&!ZN0k44@GTTzqQ?s${mO=-WOB567L>Cy&oyKPfvO<&XOUl-)9Q3o;`&|NymwfoD4 zhS^!$!frbg{+xF-T5*cKq~THvSK+jpxY!QW(ttX+qWhf0qE28K3~zP#ym(ulcP1p=db& zuMJiA|8d&9C9uH8kAoxUiuHh$gdpL@jr|Ee*Xg;|7%8R zDJr_{_8I6S@1wF4O?H74Mb@ZWvO1Ik3qYMV5%ts+)W*+ny5WxDUj|YRd(^HAO+{k6(KJ>-s^vz&LlhUN(%TFKvggQUzYzN zNI3tm3DG@_tdZa_++;EvaRZ=8BWdROie;cmwFrO>760Qn>$m@tg6qGvVbDOr^D#Gy zd%VJh?pBw zaQBts{_!JU-KNW1v^(SVtghGFZ&A+c)O~AO6bsn@mOBZ1cr8_D}w4{@3p}n({&Dyao0n8I$YG@0-7@&UM?W?QN`u zIeX(f4H~omRyyBG*5B2&{P~N|a#wH0N8g`AA9~N)I|^%&gH4J|e7I)V0(^3>uA_2J zB@f!sYuBRjTP-VgcB{AKlZHb#7QP-8I+8q+H}2@!0ck`ZGHa5nvzXa>t}#BvXxrd+ z?cr}+-*F(Xe%v?nZ1MH;uY|3*x2)&KjniY>yjnawf6=A%MdVXL*V^BW#o*+|Z4>kyqvt@`|TO9!N{7}FsH?!S4w*_GIsU1JT6Uv%wre+FiVdrL|7eG*~;4t file1.txt + +hg add file1.txt +hg commit -m "Added file1" +CSET1_ID=$(hg id --id) + +hg tag first-tag +hg phase -p + +hg topic "wip" +echo "content2" > file1.txt +hg commit -m "extra cset" +CSETX_ID=$(hg id --id) + +hg strip -r . + +BUNDLE=$(basename .hg/strip-backup/*-backup.hg) +mv .hg/strip-backup/$BUNDLE .. + +hg checkout 00000 + +cd .. + +cat > info.json < Date: Wed, 8 May 2024 22:31:43 +0200 Subject: [PATCH 06/12] Add copyright header Signed-off-by: Christophe de Vienne --- test/e2e/hg_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/e2e/hg_test.go b/test/e2e/hg_test.go index 2d5b1bad..db499030 100644 --- a/test/e2e/hg_test.go +++ b/test/e2e/hg_test.go @@ -1,3 +1,6 @@ +// Copyright 2024 The Carvel Authors. +// SPDX-License-Identifier: Apache-2.0 + package e2e import ( From 868e6d0b9174162f24dd667aca068833b772ad7b Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Fri, 10 May 2024 11:39:40 +0200 Subject: [PATCH 07/12] hg_test: use require.NoError, code cleanup Signed-off-by: Christophe de Vienne --- test/e2e/hg_test.go | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/test/e2e/hg_test.go b/test/e2e/hg_test.go index db499030..2fc69078 100644 --- a/test/e2e/hg_test.go +++ b/test/e2e/hg_test.go @@ -34,13 +34,9 @@ type hgVendirOutput struct { func loadHgAssetInfo(t *testing.T, filename string) HgAssetInfo { t.Helper() f, err := os.Open(filename) - if err != nil { - t.Fatalf("could not open file %s: %s", filename, err) - } + require.NoError(t, err, "could not open file %s", filename) var info HgAssetInfo - if err := json.NewDecoder(f).Decode(&info); err != nil { - t.Fatalf("could not parse file %s: %s", filename, err) - } + require.NoError(t, json.NewDecoder(f).Decode(&info), "could not parse file %s", filename) return info } @@ -52,15 +48,11 @@ func TestHgCache(t *testing.T) { _ = Vendir{t, env.BinaryPath, logger} hgSrcPath, err := os.MkdirTemp("", "vendir-e2e-hg-cache") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) defer os.RemoveAll(hgSrcPath) out, err := exec.Command("tar", "xzvf", "assets/hg-repos/asset.tgz", "-C", hgSrcPath).CombinedOutput() - if err != nil { - t.Fatalf("Unpacking hg-repos asset: %s (output: '%s')", err, out) - } + require.NoError(t, err, "Unpacking hg-repos asset, output: '%s'", out) info := loadHgAssetInfo(t, path.Join(hgSrcPath, "info.json")) @@ -88,7 +80,7 @@ directories: var stdout bytes.Buffer stdoutDec := json.NewDecoder(&stdout) - //defer os.RemoveAll(dstPath) + logger.Section("initial clone", func() { stdout.Truncate(0) vendir.RunWithOpts( @@ -140,9 +132,7 @@ directories: "--repository", filepath.Join(hgSrcPath, "/repo"), filepath.Join(hgSrcPath, info.ExtraBundle), ).CombinedOutput() - if err != nil { - t.Fatalf("Unpacking hg-repos asset: %s (output: '%s')", err, out) - } + require.NoError(t, err, "Unpacking hg-repos asset, output: '%s'", out) logger.Section("sync from cache + pull", func() { stdout.Truncate(0) From de55c1b9ea9c07442082fcd79ada7e92c6ffa560 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Fri, 10 May 2024 11:41:25 +0200 Subject: [PATCH 08/12] hg: simplify cacheID Use only the repository URL as a cacheID Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/hg.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index 73953175..6d378ac3 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -182,7 +182,6 @@ hgauth.password = %s } sshCmd = append(sshCmd, "-i", path, "-o", "IdentitiesOnly=yes") - cacheID += "private-key=" + *authOpts.PrivateKey + "|" } if authOpts.KnownHosts != nil { @@ -194,7 +193,6 @@ hgauth.password = %s } sshCmd = append(sshCmd, "-o", "StrictHostKeyChecking=yes", "-o", "UserKnownHostsFile="+path) - cacheID += "known-hosts=" + *authOpts.KnownHosts + "|" } else { sshCmd = append(sshCmd, "-o", "StrictHostKeyChecking=no") } @@ -209,7 +207,6 @@ hgauth.password = %s return fmt.Errorf("Writing %s: %s", hgRcPath, err) } t.env = append(t.env, "HGRCPATH="+hgRcPath) - cacheID += hgRc } sha := sha256.Sum256([]byte(cacheID)) From 195a66faedeff704d20ab26294f6ab1a8df454a8 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Mon, 13 May 2024 11:36:13 +0200 Subject: [PATCH 09/12] hg: fix a possible unbounded growth of the cache Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/sync.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/vendir/fetch/hg/sync.go b/pkg/vendir/fetch/hg/sync.go index b401c735..654ac609 100644 --- a/pkg/vendir/fetch/hg/sync.go +++ b/pkg/vendir/fetch/hg/sync.go @@ -53,18 +53,21 @@ func (d Sync) Sync(dstPath string, tempArea ctlfetch.TempArea) (ctlconf.LockDire defer hg.Close() if cachePath, ok := d.cache.Has("hg", hg.CacheID()); ok { - // Sync directly in the cache if needed - if !hg.CloneHasTargetRef(cachePath) { - if err := hg.SyncClone(cachePath); err != nil { - return hgLockConf, fmt.Errorf("Syncing hg cached clone: %w", err) - } - } // fetch from cachedDir if err := d.cache.CopyFrom("hg", hg.CacheID(), incomingTmpPath); err != nil { return hgLockConf, fmt.Errorf("Extracting cached hg clone: %w", err) } + // Sync if needed + if !hg.CloneHasTargetRef(cachePath) { + if err := hg.SyncClone(incomingTmpPath); err != nil { + return hgLockConf, fmt.Errorf("Syncing hg repository: %w", err) + } + if err := d.cache.Save("hg", hg.CacheID(), incomingTmpPath); err != nil { + return hgLockConf, fmt.Errorf("Saving hg repository to cache: %w", err) + } + } } else { - // fetch in the target directory, and save it to cache + // fetch in the target directory if err := hg.Clone(incomingTmpPath); err != nil { return hgLockConf, fmt.Errorf("Cloning hg repository: %w", err) } From 255e8046f11cd4011d3456e7f88dfee08cd491d5 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Tue, 21 May 2024 09:48:37 +0200 Subject: [PATCH 10/12] hg: unexport completely the Hg api The 'hg' type is strictly an internal tool of the 'hg' fetcher. The only meaningful public API of this package is the 'Sync' type. Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/hg.go | 46 ++++++++++++++++++------------------- pkg/vendir/fetch/hg/sync.go | 18 +++++++-------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index 6d378ac3..15a9b0ca 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -19,7 +19,7 @@ import ( ctlfetch "carvel.dev/vendir/pkg/vendir/fetch" ) -type Hg struct { +type hg struct { opts ctlconf.DirectoryContentsHg infoLog io.Writer refFetcher ctlfetch.RefFetcher @@ -28,34 +28,34 @@ type Hg struct { cacheID string } -func NewHg(opts ctlconf.DirectoryContentsHg, +func newHg(opts ctlconf.DirectoryContentsHg, infoLog io.Writer, refFetcher ctlfetch.RefFetcher, tempArea ctlfetch.TempArea, -) (*Hg, error) { - t := Hg{opts, infoLog, refFetcher, "", nil, ""} +) (*hg, error) { + t := hg{opts, infoLog, refFetcher, "", nil, ""} if err := t.setup(tempArea); err != nil { return nil, err } return &t, nil } -// CacheID returns a cache id for the repository +// getCacheID returns a cache id for the repository // It doesn't include the ref because we want to reuse a cache when only the ref // is changed // Basically we combine all data used to write the hgrc file -func (t *Hg) CacheID() string { +func (t *hg) getCacheID() string { return t.cacheID } //nolint:revive -type HgInfo struct { +type hgInfo struct { SHA string ChangeSetTitle string } -// CloneHasTargetRef returns true if the given clone contains the target +// cloneHasTargetRef returns true if the given clone contains the target // ref, and this ref is a revision id (not a tag or a branch) -func (t *Hg) CloneHasTargetRef(dstPath string) bool { +func (t *hg) cloneHasTargetRef(dstPath string) bool { out, _, err := t.run([]string{"id", "--id", "-r", t.opts.Ref}, dstPath) if err != nil { return false @@ -67,21 +67,21 @@ func (t *Hg) CloneHasTargetRef(dstPath string) bool { return false } -func (t *Hg) Clone(dstPath string) error { +func (t *hg) clone(dstPath string) error { if err := t.initClone(dstPath); err != nil { return err } - return t.SyncClone(dstPath) + return t.syncClone(dstPath) } -func (t *Hg) SyncClone(dstPath string) error { +func (t *hg) syncClone(dstPath string) error { if _, _, err := t.run([]string{"pull"}, dstPath); err != nil { return err } return nil } -func (t *Hg) LocalClone(localClone, dstPath string) error { +func (t *hg) localClone(localClone, dstPath string) error { if err := t.initClone(dstPath); err != nil { return err } @@ -91,24 +91,24 @@ func (t *Hg) LocalClone(localClone, dstPath string) error { return nil } -func (t *Hg) Checkout(dstPath string) (HgInfo, error) { +func (t *hg) checkout(dstPath string) (hgInfo, error) { if _, _, err := t.run([]string{"checkout", t.opts.Ref}, dstPath); err != nil { - return HgInfo{}, err + return hgInfo{}, err } - info := HgInfo{} + info := hgInfo{} // use hg log to retrieve full cset sha out, _, err := t.run([]string{"log", "-r", ".", "-T", "{node}"}, dstPath) if err != nil { - return HgInfo{}, err + return hgInfo{}, err } info.SHA = strings.TrimSpace(out) out, _, err = t.run([]string{"log", "-l", "1", "-T", "{desc|firstline|strip}", "-r", info.SHA}, dstPath) if err != nil { - return HgInfo{}, err + return hgInfo{}, err } info.ChangeSetTitle = strings.TrimSpace(out) @@ -116,14 +116,14 @@ func (t *Hg) Checkout(dstPath string) (HgInfo, error) { return info, nil } -func (t *Hg) Close() { +func (t *hg) Close() { if t.authDir != "" { os.RemoveAll(t.authDir) t.authDir = "" } } -func (t *Hg) setup(tempArea ctlfetch.TempArea) error { +func (t *hg) setup(tempArea ctlfetch.TempArea) error { if len(t.opts.URL) == 0 { return fmt.Errorf("Expected non-empty URL") } @@ -215,7 +215,7 @@ hgauth.password = %s return nil } -func (t *Hg) initClone(dstPath string) error { +func (t *hg) initClone(dstPath string) error { hgURL := t.opts.URL if _, _, err := t.run([]string{"init"}, dstPath); err != nil { @@ -233,7 +233,7 @@ func (t *Hg) initClone(dstPath string) error { return nil } -func (t *Hg) run(args []string, dstPath string) (string, string, error) { +func (t *hg) run(args []string, dstPath string) (string, string, error) { var stdoutBs, stderrBs bytes.Buffer cmd := exec.Command("hg", args...) @@ -263,7 +263,7 @@ func (o hgAuthOpts) IsPresent() bool { return o.PrivateKey != nil || o.KnownHosts != nil || o.Username != nil || o.Password != nil } -func (t *Hg) getAuthOpts() (hgAuthOpts, error) { +func (t *hg) getAuthOpts() (hgAuthOpts, error) { var opts hgAuthOpts if t.opts.SecretRef != nil { diff --git a/pkg/vendir/fetch/hg/sync.go b/pkg/vendir/fetch/hg/sync.go index 654ac609..c227a6ce 100644 --- a/pkg/vendir/fetch/hg/sync.go +++ b/pkg/vendir/fetch/hg/sync.go @@ -46,38 +46,38 @@ func (d Sync) Sync(dstPath string, tempArea ctlfetch.TempArea) (ctlconf.LockDire defer os.RemoveAll(incomingTmpPath) - hg, err := NewHg(d.opts, d.log, d.refFetcher, tempArea) + hg, err := newHg(d.opts, d.log, d.refFetcher, tempArea) if err != nil { return hgLockConf, fmt.Errorf("Setting up hg: %w", err) } defer hg.Close() - if cachePath, ok := d.cache.Has("hg", hg.CacheID()); ok { + if cachePath, ok := d.cache.Has("hg", hg.getCacheID()); ok { // fetch from cachedDir - if err := d.cache.CopyFrom("hg", hg.CacheID(), incomingTmpPath); err != nil { + if err := d.cache.CopyFrom("hg", hg.getCacheID(), incomingTmpPath); err != nil { return hgLockConf, fmt.Errorf("Extracting cached hg clone: %w", err) } // Sync if needed - if !hg.CloneHasTargetRef(cachePath) { - if err := hg.SyncClone(incomingTmpPath); err != nil { + if !hg.cloneHasTargetRef(cachePath) { + if err := hg.syncClone(incomingTmpPath); err != nil { return hgLockConf, fmt.Errorf("Syncing hg repository: %w", err) } - if err := d.cache.Save("hg", hg.CacheID(), incomingTmpPath); err != nil { + if err := d.cache.Save("hg", hg.getCacheID(), incomingTmpPath); err != nil { return hgLockConf, fmt.Errorf("Saving hg repository to cache: %w", err) } } } else { // fetch in the target directory - if err := hg.Clone(incomingTmpPath); err != nil { + if err := hg.clone(incomingTmpPath); err != nil { return hgLockConf, fmt.Errorf("Cloning hg repository: %w", err) } - if err := d.cache.Save("hg", hg.CacheID(), incomingTmpPath); err != nil { + if err := d.cache.Save("hg", hg.getCacheID(), incomingTmpPath); err != nil { return hgLockConf, fmt.Errorf("Saving hg repository to cache: %w", err) } } // now checkout the wanted revision - info, err := hg.Checkout(incomingTmpPath) + info, err := hg.checkout(incomingTmpPath) if err != nil { return hgLockConf, fmt.Errorf("Checking out hg repository: %s", err) } From ffd1bd9a8874606626198f8df39b196b5f134dca Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Mon, 27 May 2024 11:12:56 +0200 Subject: [PATCH 11/12] hg: cleanup dead code Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/hg.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index 15a9b0ca..7fd105e2 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -81,16 +81,6 @@ func (t *hg) syncClone(dstPath string) error { return nil } -func (t *hg) localClone(localClone, dstPath string) error { - if err := t.initClone(dstPath); err != nil { - return err - } - if _, _, err := t.run([]string{"pull", localClone}, dstPath); err != nil { - return err - } - return nil -} - func (t *hg) checkout(dstPath string) (hgInfo, error) { if _, _, err := t.run([]string{"checkout", t.opts.Ref}, dstPath); err != nil { return hgInfo{}, err From cc864a7f29d83f200ce60103cc26c0758615a9f9 Mon Sep 17 00:00:00 2001 From: Christophe de Vienne Date: Fri, 7 Jun 2024 19:05:00 +0200 Subject: [PATCH 12/12] hg: expose the Hg type again Signed-off-by: Christophe de Vienne --- pkg/vendir/fetch/hg/hg.go | 38 ++++++++++++++++++------------------- pkg/vendir/fetch/hg/sync.go | 2 +- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/vendir/fetch/hg/hg.go b/pkg/vendir/fetch/hg/hg.go index 7fd105e2..aa24ff40 100644 --- a/pkg/vendir/fetch/hg/hg.go +++ b/pkg/vendir/fetch/hg/hg.go @@ -19,7 +19,7 @@ import ( ctlfetch "carvel.dev/vendir/pkg/vendir/fetch" ) -type hg struct { +type Hg struct { opts ctlconf.DirectoryContentsHg infoLog io.Writer refFetcher ctlfetch.RefFetcher @@ -28,11 +28,11 @@ type hg struct { cacheID string } -func newHg(opts ctlconf.DirectoryContentsHg, +func NewHg(opts ctlconf.DirectoryContentsHg, infoLog io.Writer, refFetcher ctlfetch.RefFetcher, tempArea ctlfetch.TempArea, -) (*hg, error) { - t := hg{opts, infoLog, refFetcher, "", nil, ""} +) (*Hg, error) { + t := Hg{opts, infoLog, refFetcher, "", nil, ""} if err := t.setup(tempArea); err != nil { return nil, err } @@ -43,19 +43,19 @@ func newHg(opts ctlconf.DirectoryContentsHg, // It doesn't include the ref because we want to reuse a cache when only the ref // is changed // Basically we combine all data used to write the hgrc file -func (t *hg) getCacheID() string { +func (t *Hg) getCacheID() string { return t.cacheID } //nolint:revive -type hgInfo struct { +type HgInfo struct { SHA string ChangeSetTitle string } // cloneHasTargetRef returns true if the given clone contains the target // ref, and this ref is a revision id (not a tag or a branch) -func (t *hg) cloneHasTargetRef(dstPath string) bool { +func (t *Hg) cloneHasTargetRef(dstPath string) bool { out, _, err := t.run([]string{"id", "--id", "-r", t.opts.Ref}, dstPath) if err != nil { return false @@ -67,38 +67,38 @@ func (t *hg) cloneHasTargetRef(dstPath string) bool { return false } -func (t *hg) clone(dstPath string) error { +func (t *Hg) clone(dstPath string) error { if err := t.initClone(dstPath); err != nil { return err } return t.syncClone(dstPath) } -func (t *hg) syncClone(dstPath string) error { +func (t *Hg) syncClone(dstPath string) error { if _, _, err := t.run([]string{"pull"}, dstPath); err != nil { return err } return nil } -func (t *hg) checkout(dstPath string) (hgInfo, error) { +func (t *Hg) checkout(dstPath string) (HgInfo, error) { if _, _, err := t.run([]string{"checkout", t.opts.Ref}, dstPath); err != nil { - return hgInfo{}, err + return HgInfo{}, err } - info := hgInfo{} + info := HgInfo{} // use hg log to retrieve full cset sha out, _, err := t.run([]string{"log", "-r", ".", "-T", "{node}"}, dstPath) if err != nil { - return hgInfo{}, err + return HgInfo{}, err } info.SHA = strings.TrimSpace(out) out, _, err = t.run([]string{"log", "-l", "1", "-T", "{desc|firstline|strip}", "-r", info.SHA}, dstPath) if err != nil { - return hgInfo{}, err + return HgInfo{}, err } info.ChangeSetTitle = strings.TrimSpace(out) @@ -106,14 +106,14 @@ func (t *hg) checkout(dstPath string) (hgInfo, error) { return info, nil } -func (t *hg) Close() { +func (t *Hg) Close() { if t.authDir != "" { os.RemoveAll(t.authDir) t.authDir = "" } } -func (t *hg) setup(tempArea ctlfetch.TempArea) error { +func (t *Hg) setup(tempArea ctlfetch.TempArea) error { if len(t.opts.URL) == 0 { return fmt.Errorf("Expected non-empty URL") } @@ -205,7 +205,7 @@ hgauth.password = %s return nil } -func (t *hg) initClone(dstPath string) error { +func (t *Hg) initClone(dstPath string) error { hgURL := t.opts.URL if _, _, err := t.run([]string{"init"}, dstPath); err != nil { @@ -223,7 +223,7 @@ func (t *hg) initClone(dstPath string) error { return nil } -func (t *hg) run(args []string, dstPath string) (string, string, error) { +func (t *Hg) run(args []string, dstPath string) (string, string, error) { var stdoutBs, stderrBs bytes.Buffer cmd := exec.Command("hg", args...) @@ -253,7 +253,7 @@ func (o hgAuthOpts) IsPresent() bool { return o.PrivateKey != nil || o.KnownHosts != nil || o.Username != nil || o.Password != nil } -func (t *hg) getAuthOpts() (hgAuthOpts, error) { +func (t *Hg) getAuthOpts() (hgAuthOpts, error) { var opts hgAuthOpts if t.opts.SecretRef != nil { diff --git a/pkg/vendir/fetch/hg/sync.go b/pkg/vendir/fetch/hg/sync.go index c227a6ce..59c0282c 100644 --- a/pkg/vendir/fetch/hg/sync.go +++ b/pkg/vendir/fetch/hg/sync.go @@ -46,7 +46,7 @@ func (d Sync) Sync(dstPath string, tempArea ctlfetch.TempArea) (ctlconf.LockDire defer os.RemoveAll(incomingTmpPath) - hg, err := newHg(d.opts, d.log, d.refFetcher, tempArea) + hg, err := NewHg(d.opts, d.log, d.refFetcher, tempArea) if err != nil { return hgLockConf, fmt.Errorf("Setting up hg: %w", err) }