Skip to content

Commit 033c4e2

Browse files
author
Arjun Sreedharan
committed
supply: remove pre-installing mercurial
* There's no documentation that suggests that while installing from a remote url via a VCS like `hg`, the `mercurial` package should be pre-installed. [Pip documentation](https://pip.pypa.io/en/latest/topics/vcs-support/) just states that it requires a working executable to be available, which already exists on the stack. ``` ‣ docker run --init -it cloudfoundry/cflinuxfs3 bash -c "hg --version" Mercurial Distributed SCM (version 4.5.3) ``` * When `fixtures/mercurial` is built by this branch's buildpack, we can see in the log python-hglib (which was the package referred to in the testdata by hg clone url) is installed. ``` Successfully installed Flask-2.2.2 ... python-hglib-2.6.2+2.1e7a64588ab0 ... ``` * Git history suggests that pre-installing mercurial via `pip install mercurial` ([link](https://github.com/cloudfoundry/python-buildpack/blob/v1.7.58/src/python/supply/supply.go#L201-L215)) came into this buildpack from the original heroku buildpack fork. Heroku has since removed it. See heroku/heroku-buildpack-python#1111 * This change does not address why running an app with `mercurial` present in the `requirements.txt` fails with the error pointing to a non-existent include path to `Python.h` even after include location is set via CFLAGS in 028a7b6. See [CI log](https://buildpacks.ci.cf-app.com/teams/main/pipelines/python-buildpack/jobs/specs-edge-integration-develop/builds/1054#L62d6d57b:516). The timing of this failure appearing in CI suggests that it's related to the package using the new setuptools version as a transitive dependency. See #574 This has to be separately investigated.
1 parent 4b06c2d commit 033c4e2

File tree

2 files changed

+0
-65
lines changed

2 files changed

+0
-65
lines changed

src/python/supply/supply.go

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,6 @@ func RunPython(s *Supplier) error {
128128
return err
129129
}
130130

131-
if err := s.HandleMercurial(); err != nil {
132-
s.Log.Error("Could not handle pip mercurial dependencies: %v", err)
133-
return err
134-
}
135-
136131
if err := s.UninstallUnusedDependencies(); err != nil {
137132
s.Log.Error("Error uninstalling unused dependencies: %v", err)
138133
return err
@@ -197,25 +192,6 @@ func (s *Supplier) CopyRuntimeTxt() error {
197192
return nil
198193
}
199194

200-
func (s *Supplier) HandleMercurial() error {
201-
if err := s.Command.Execute(s.Stager.BuildDir(), ioutil.Discard, ioutil.Discard, "grep", "-Fiq", "hg+", "requirements.txt"); err != nil {
202-
return nil
203-
}
204-
205-
if s.Manifest.IsCached() {
206-
s.Log.Warning("Cloud Foundry does not support Pip Mercurial dependencies while in offline-mode. Vendor your dependencies if they do not work.")
207-
}
208-
209-
if err := s.runPipInstall("mercurial"); err != nil {
210-
return err
211-
}
212-
213-
if err := s.Stager.LinkDirectoryInDepDir(filepath.Join(s.Stager.DepDir(), "python", "bin"), "bin"); err != nil {
214-
return err
215-
}
216-
return nil
217-
}
218-
219195
func (s *Supplier) HandlePipfile() error {
220196
var pipfileExists, runtimeExists bool
221197
var pipfileJson pipfile.Lock

src/python/supply/supply_test.go

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -456,47 +456,6 @@ var _ = Describe("Supply", func() {
456456
})
457457
})
458458

459-
Describe("HandleMercurial", func() {
460-
Context("has mercurial dependencies", func() {
461-
BeforeEach(func() {
462-
mockCommand.EXPECT().Execute(buildDir, gomock.Any(), gomock.Any(), "grep", "-Fiq", "hg+", "requirements.txt")
463-
})
464-
465-
Context("the buildpack is not cached", func() {
466-
BeforeEach(func() {
467-
mockManifest.EXPECT().IsCached().Return(false)
468-
})
469-
It("installs mercurial", func() {
470-
mockCommand.EXPECT().Execute(buildDir, gomock.Any(), gomock.Any(), "python", "-m", "pip", "install", "mercurial")
471-
mockStager.EXPECT().LinkDirectoryInDepDir(filepath.Join(depDir, "python", "bin"), "bin")
472-
Expect(supplier.HandleMercurial()).To(Succeed())
473-
})
474-
})
475-
476-
Context("the buildpack is cached", func() {
477-
BeforeEach(func() {
478-
mockManifest.EXPECT().IsCached().Return(true)
479-
})
480-
It("installs mercurial and provides a warning", func() {
481-
mockCommand.EXPECT().Execute(buildDir, gomock.Any(), gomock.Any(), "python", "-m", "pip", "install", "mercurial")
482-
mockStager.EXPECT().LinkDirectoryInDepDir(filepath.Join(depDir, "python", "bin"), "bin")
483-
Expect(supplier.HandleMercurial()).To(Succeed())
484-
Expect(buffer.String()).To(ContainSubstring("Cloud Foundry does not support Pip Mercurial dependencies while in offline-mode. Vendor your dependencies if they do not work."))
485-
})
486-
})
487-
488-
})
489-
Context("does not have mercurial dependencies", func() {
490-
BeforeEach(func() {
491-
mockCommand.EXPECT().Execute(buildDir, gomock.Any(), gomock.Any(), "grep", "-Fiq", "hg+", "requirements.txt").Return(fmt.Errorf("Mercurial not found"))
492-
})
493-
494-
It("succeeds without installing mercurial", func() {
495-
Expect(supplier.HandleMercurial()).To(Succeed())
496-
})
497-
})
498-
})
499-
500459
Describe("RewriteShebangs", func() {
501460
BeforeEach(func() {
502461
Expect(os.MkdirAll(filepath.Join(depDir, "bin"), 0755)).To(Succeed())

0 commit comments

Comments
 (0)