From 20f1f50eac7e5680ff3038de78b0beca47d76265 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 12:11:15 +0200 Subject: [PATCH 1/8] refactor: Split out common python methods into python.go --- python/embedded_python.go | 55 ++++------------------------------ python/python.go | 62 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 50 deletions(-) create mode 100644 python/python.go diff --git a/python/embedded_python.go b/python/embedded_python.go index fe0179f..21f41af 100644 --- a/python/embedded_python.go +++ b/python/embedded_python.go @@ -4,17 +4,11 @@ import ( "fmt" "github.com/kluctl/go-embed-python/embed_util" "github.com/kluctl/go-embed-python/python/internal/data" - "os" - "os/exec" - "path/filepath" - "runtime" - "strings" ) type EmbeddedPython struct { e *embed_util.EmbeddedFiles - - pythonPath []string + *Python } // NewEmbeddedPython creates a new EmbeddedPython instance. The embedded source code and python binaries are @@ -26,7 +20,8 @@ func NewEmbeddedPython(name string) (*EmbeddedPython, error) { return nil, err } return &EmbeddedPython{ - e: e, + e: e, + Python: NewPython(e.GetExtractedPath()), }, nil } @@ -36,7 +31,8 @@ func NewEmbeddedPythonWithTmpDir(tmpDir string, withHashInDir bool) (*EmbeddedPy return nil, err } return &EmbeddedPython{ - e: e, + e: e, + Python: NewPython(e.GetExtractedPath()), }, nil } @@ -47,44 +43,3 @@ func (ep *EmbeddedPython) Cleanup() error { func (ep *EmbeddedPython) GetExtractedPath() string { return ep.e.GetExtractedPath() } - -func (ep *EmbeddedPython) GetBinPath() string { - if runtime.GOOS == "windows" { - return ep.GetExtractedPath() - } else { - return filepath.Join(ep.GetExtractedPath(), "bin") - } -} - -func (ep *EmbeddedPython) GetExePath() string { - suffix := "" - if runtime.GOOS == "windows" { - suffix = ".exe" - } else { - suffix = "3" - } - return filepath.Join(ep.GetBinPath(), "python"+suffix) -} - -func (ep *EmbeddedPython) AddPythonPath(p string) { - ep.pythonPath = append(ep.pythonPath, p) -} - -func (ep *EmbeddedPython) PythonCmd(args ...string) *exec.Cmd { - return ep.PythonCmd2(args) -} - -func (ep *EmbeddedPython) PythonCmd2(args []string) *exec.Cmd { - exePath := ep.GetExePath() - - cmd := exec.Command(exePath, args...) - cmd.Env = os.Environ() - cmd.Env = append(cmd.Env, fmt.Sprintf("PYTHONHOME=%s", ep.GetExtractedPath())) - - if len(ep.pythonPath) != 0 { - pythonPathEnv := fmt.Sprintf("PYTHONPATH=%s", strings.Join(ep.pythonPath, string(os.PathListSeparator))) - cmd.Env = append(cmd.Env, pythonPathEnv) - } - - return cmd -} diff --git a/python/python.go b/python/python.go new file mode 100644 index 0000000..6081a6d --- /dev/null +++ b/python/python.go @@ -0,0 +1,62 @@ +package python + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" +) + +type Python struct { + pythonHome string + pythonPath []string +} + +func NewPython(pythonHome string) *Python { + return &Python{ + pythonHome: pythonHome, + } +} + +func (ep *Python) GetBinPath() string { + if runtime.GOOS == "windows" { + return ep.pythonHome + } else { + return filepath.Join(ep.pythonHome, "bin") + } +} + +func (ep *Python) GetExePath() string { + suffix := "" + if runtime.GOOS == "windows" { + suffix = ".exe" + } else { + suffix = "3" + } + return filepath.Join(ep.GetBinPath(), "python"+suffix) +} + +func (ep *Python) AddPythonPath(p string) { + ep.pythonPath = append(ep.pythonPath, p) +} + +func (ep *Python) PythonCmd(args ...string) *exec.Cmd { + return ep.PythonCmd2(args) +} + +func (ep *Python) PythonCmd2(args []string) *exec.Cmd { + exePath := ep.GetExePath() + + cmd := exec.Command(exePath, args...) + cmd.Env = os.Environ() + cmd.Env = append(cmd.Env, fmt.Sprintf("PYTHONHOME=%s", ep.pythonHome)) + + if len(ep.pythonPath) != 0 { + pythonPathEnv := fmt.Sprintf("PYTHONPATH=%s", strings.Join(ep.pythonPath, string(os.PathListSeparator))) + cmd.Env = append(cmd.Env, pythonPathEnv) + } + + return cmd +} From 0d6ebbb4af3c1d5d6b02f0e5773c640880a7b211 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 12:12:51 +0200 Subject: [PATCH 2/8] chore: Upgrade python versions --- .github/workflows/release.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c7aec1f..751354b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,13 +11,13 @@ on: env: PYTHON_STANDALONE_VERSIONS: | [ - "20240224" + "20240415" ] PYTHON_VERSIONS: | [ - "3.10.13", - "3.11.8", - "3.12.2" + "3.10.14", + "3.11.9", + "3.12.3" ] jobs: From a7ca5c4d53fa6ae80f2e2e66cdf078da60b00b4d Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 12:15:47 +0200 Subject: [PATCH 3/8] fix: Use pgo+lto builds --- python/generate/main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/generate/main.go b/python/generate/main.go index 8365468..cf76f13 100644 --- a/python/generate/main.go +++ b/python/generate/main.go @@ -81,10 +81,10 @@ func main() { } jobs := []job{ - {"linux", "amd64", "unknown-linux-gnu-lto-full", keepNixPatterns}, + {"linux", "amd64", "unknown-linux-gnu-pgo+lto-full", keepNixPatterns}, {"linux", "arm64", "unknown-linux-gnu-lto-full", keepNixPatterns}, - {"darwin", "amd64", "apple-darwin-lto-full", keepNixPatterns}, - {"darwin", "arm64", "apple-darwin-lto-full", keepNixPatterns}, + {"darwin", "amd64", "apple-darwin-pgo+lto-full", keepNixPatterns}, + {"darwin", "arm64", "apple-darwin-pgo+lto-full", keepNixPatterns}, {"windows", "amd64", "pc-windows-msvc-shared-pgo-full", keepWinPatterns}, } for _, j := range jobs { From 02a205450630174a22ca317ee7d3cfc2b1244474 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 15:04:09 +0200 Subject: [PATCH 4/8] feat: Allow to use NewPython without a PYTHONHOME This causes it to use the system python installation --- example/main.go | 5 ++- pip/embed_pip_packages.go | 7 ++-- pip/generate/main.go | 7 ++-- python/embedded_python.go | 4 +-- python/embedded_python_test.go | 8 ++--- python/python.go | 61 +++++++++++++++++++++++++--------- python/python_test.go | 33 ++++++++++++++++++ 7 files changed, 99 insertions(+), 26 deletions(-) create mode 100644 python/python_test.go diff --git a/example/main.go b/example/main.go index 3130b5e..de96734 100644 --- a/example/main.go +++ b/example/main.go @@ -11,7 +11,10 @@ func main() { panic(err) } - cmd := ep.PythonCmd("-c", "print('hello')") + cmd, err := ep.PythonCmd("-c", "print('hello')") + if err != nil { + panic(err) + } cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr err = cmd.Run() diff --git a/pip/embed_pip_packages.go b/pip/embed_pip_packages.go index e9bde0c..b38814c 100644 --- a/pip/embed_pip_packages.go +++ b/pip/embed_pip_packages.go @@ -109,10 +109,13 @@ func pipInstall(ep *python.EmbeddedPython, requirementsFile string, platforms [] args = append(args, "--only-binary=:all:") } - cmd := ep.PythonCmd(args...) + cmd, err := ep.PythonCmd(args...) + if err != nil { + return err + } cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - err := cmd.Run() + err = cmd.Run() if err != nil { return err } diff --git a/pip/generate/main.go b/pip/generate/main.go index 21131f4..e003fe3 100644 --- a/pip/generate/main.go +++ b/pip/generate/main.go @@ -33,10 +33,13 @@ func bootstrapPip(ep *python.EmbeddedPython) { getPip := downloadGetPip() defer os.Remove(getPip) - cmd := ep.PythonCmd(getPip) + cmd, err := ep.PythonCmd(getPip) + if err != nil { + panic(err) + } cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - err := cmd.Run() + err = cmd.Run() if err != nil { panic(err) } diff --git a/python/embedded_python.go b/python/embedded_python.go index 21f41af..b43bb5d 100644 --- a/python/embedded_python.go +++ b/python/embedded_python.go @@ -21,7 +21,7 @@ func NewEmbeddedPython(name string) (*EmbeddedPython, error) { } return &EmbeddedPython{ e: e, - Python: NewPython(e.GetExtractedPath()), + Python: NewPython(WithPythonHome(e.GetExtractedPath())), }, nil } @@ -32,7 +32,7 @@ func NewEmbeddedPythonWithTmpDir(tmpDir string, withHashInDir bool) (*EmbeddedPy } return &EmbeddedPython{ e: e, - Python: NewPython(e.GetExtractedPath()), + Python: NewPython(WithPythonHome(e.GetExtractedPath())), }, nil } diff --git a/python/embedded_python_test.go b/python/embedded_python_test.go index a69ffe1..74ed28e 100644 --- a/python/embedded_python_test.go +++ b/python/embedded_python_test.go @@ -17,10 +17,10 @@ func TestEmbeddedPython(t *testing.T) { defer ep.Cleanup() path := ep.GetExtractedPath() assert.NotEqual(t, path, "") - pexe := ep.GetExePath() + pexe, _ := ep.GetExePath() assert.True(t, internal.Exists(pexe)) - cmd := ep.PythonCmd("-c", "print('test test')") + cmd, _ := ep.PythonCmd("-c", "print('test test')") stdout, err := cmd.StdoutPipe() assert.NoError(t, err) defer stdout.Close() @@ -61,10 +61,10 @@ print("platform.processor=" + platform.processor()) defer ep.Cleanup() path := ep.GetExtractedPath() assert.NotEqual(t, path, "") - pexe := ep.GetExePath() + pexe, _ := ep.GetExePath() assert.True(t, internal.Exists(pexe)) - cmd := ep.PythonCmd("-c", getSystemInfo) + cmd, _ := ep.PythonCmd("-c", getSystemInfo) stdout, _ := cmd.StdoutPipe() stderr, _ := cmd.StderrPipe() defer stdout.Close() diff --git a/python/python.go b/python/python.go index 6081a6d..c503ab7 100644 --- a/python/python.go +++ b/python/python.go @@ -14,49 +14,80 @@ type Python struct { pythonPath []string } -func NewPython(pythonHome string) *Python { - return &Python{ - pythonHome: pythonHome, +type PythonOpt func(o *Python) + +func WithPythonHome(home string) PythonOpt { + return func(o *Python) { + o.pythonHome = home } } -func (ep *Python) GetBinPath() string { - if runtime.GOOS == "windows" { - return ep.pythonHome - } else { - return filepath.Join(ep.pythonHome, "bin") +func NewPython(opts ...PythonOpt) *Python { + ep := &Python{} + + for _, o := range opts { + o(ep) } + + return ep } -func (ep *Python) GetExePath() string { +func (ep *Python) GetExeName() string { suffix := "" if runtime.GOOS == "windows" { suffix = ".exe" } else { suffix = "3" } - return filepath.Join(ep.GetBinPath(), "python"+suffix) + return "python" + suffix +} + +func (ep *Python) GetExePath() (string, error) { + if ep.pythonHome == "" { + p, err := exec.LookPath(ep.GetExeName()) + if err != nil { + return "", fmt.Errorf("failed to determine %s path: %w", ep.GetExeName(), err) + } + return p, nil + } else { + var p string + if runtime.GOOS == "windows" { + p = filepath.Join(ep.pythonHome, ep.GetExeName()) + } else { + p = filepath.Join(ep.pythonHome, "bin", ep.GetExeName()) + } + if _, err := os.Stat(p); err != nil { + return "", fmt.Errorf("failed to determine %s path: %w", ep.GetExeName(), err) + } + return p, nil + } } func (ep *Python) AddPythonPath(p string) { ep.pythonPath = append(ep.pythonPath, p) } -func (ep *Python) PythonCmd(args ...string) *exec.Cmd { +func (ep *Python) PythonCmd(args ...string) (*exec.Cmd, error) { return ep.PythonCmd2(args) } -func (ep *Python) PythonCmd2(args []string) *exec.Cmd { - exePath := ep.GetExePath() +func (ep *Python) PythonCmd2(args []string) (*exec.Cmd, error) { + exePath, err := ep.GetExePath() + if err != nil { + return nil, err + } cmd := exec.Command(exePath, args...) cmd.Env = os.Environ() - cmd.Env = append(cmd.Env, fmt.Sprintf("PYTHONHOME=%s", ep.pythonHome)) + + if ep.pythonHome != "" { + cmd.Env = append(cmd.Env, fmt.Sprintf("PYTHONHOME=%s", ep.pythonHome)) + } if len(ep.pythonPath) != 0 { pythonPathEnv := fmt.Sprintf("PYTHONPATH=%s", strings.Join(ep.pythonPath, string(os.PathListSeparator))) cmd.Env = append(cmd.Env, pythonPathEnv) } - return cmd + return cmd, nil } diff --git a/python/python_test.go b/python/python_test.go new file mode 100644 index 0000000..35c39dc --- /dev/null +++ b/python/python_test.go @@ -0,0 +1,33 @@ +package python + +import ( + "bytes" + "github.com/kluctl/go-embed-python/internal" + "github.com/stretchr/testify/assert" + "io" + "testing" +) + +func TestExternalPython(t *testing.T) { + ep := NewPython() + + pexe, _ := ep.GetExePath() + assert.True(t, internal.Exists(pexe)) + + cmd, _ := ep.PythonCmd("-c", "print('test test')") + stdout, err := cmd.StdoutPipe() + assert.NoError(t, err) + defer stdout.Close() + + err = cmd.Start() + assert.NoError(t, err) + + stdoutStr, err := io.ReadAll(stdout) + assert.NoError(t, err) + + err = cmd.Wait() + assert.NoError(t, err) + + stdoutStr = bytes.TrimSpace(stdoutStr) + assert.Equal(t, "test test", string(stdoutStr)) +} From db84bf314842a6dd508dc662fda1048ff118a40c Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 15:05:28 +0200 Subject: [PATCH 5/8] chore: Run go get -u ./... --- go.mod | 4 ++-- go.sum | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 9a570dd..6705450 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/gobwas/glob v0.2.3 - github.com/klauspost/compress v1.17.8 + github.com/klauspost/compress v1.17.9 github.com/rogpeppe/go-internal v1.12.0 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.9.0 @@ -14,6 +14,6 @@ require ( require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/sys v0.15.0 // indirect + golang.org/x/sys v0.21.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index e669890..aa8275b 100644 --- a/go.sum +++ b/go.sum @@ -7,6 +7,8 @@ github.com/klauspost/compress v1.17.7 h1:ehO88t2UGzQK66LMdE8tibEd1ErmzZjNEqWkjLA github.com/klauspost/compress v1.17.7/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= github.com/klauspost/compress v1.17.8 h1:YcnTYrq7MikUT7k0Yb5eceMmALQPYBW/Xltxn0NAMnU= github.com/klauspost/compress v1.17.8/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= +github.com/klauspost/compress v1.17.9 h1:6KIumPrER1LHsvBVuDa0r5xaG0Es51mhhB9BQB2qeMA= +github.com/klauspost/compress v1.17.9/go.mod h1:Di0epgTjJY877eYKx5yC51cX2A2Vl2ibi7bDH9ttBbw= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= @@ -24,6 +26,8 @@ golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= +golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From 9dd7ecb40dfe07e85df98bee76d546f0b050b6b1 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 15:52:34 +0200 Subject: [PATCH 6/8] ci: Switch to ubuntu-22.04 --- .github/workflows/release.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 751354b..ee48ecf 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -39,7 +39,7 @@ jobs: pythonStandaloneVersion: ${{ fromJSON(needs.build-matrix.outputs.PYTHON_STANDALONE_VERSIONS) }} pythonVersion: ${{ fromJSON(needs.build-matrix.outputs.PYTHON_VERSIONS) }} fail-fast: false - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 steps: - name: checkout run: | @@ -83,7 +83,7 @@ jobs: strategy: matrix: os: - - ubuntu-20.04 + - ubuntu-22.04 - macos-11 - windows-2019 pythonStandaloneVersion: ${{ fromJSON(needs.build-matrix.outputs.PYTHON_STANDALONE_VERSIONS) }} @@ -125,7 +125,7 @@ jobs: pythonStandaloneVersion: ${{ fromJSON(needs.build-matrix.outputs.PYTHON_STANDALONE_VERSIONS) }} pythonVersion: ${{ fromJSON(needs.build-matrix.outputs.PYTHON_VERSIONS) }} fail-fast: false - runs-on: ubuntu-20.04 + runs-on: ubuntu-22.04 if: ${{ github.event_name == 'push' && github.ref_name == 'main' }} permissions: contents: write From daca3d1794bebe9f9eeee8aea48df305082546c6 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 16:20:02 +0200 Subject: [PATCH 7/8] ci: Switch to macos-12 --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ee48ecf..1bbc10b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -84,7 +84,7 @@ jobs: matrix: os: - ubuntu-22.04 - - macos-11 + - macos-12 - windows-2019 pythonStandaloneVersion: ${{ fromJSON(needs.build-matrix.outputs.PYTHON_STANDALONE_VERSIONS) }} pythonVersion: ${{ fromJSON(needs.build-matrix.outputs.PYTHON_VERSIONS) }} From 421798e8b35aabde053157e4a2f3db070eac9b00 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Mon, 17 Jun 2024 15:16:40 +0200 Subject: [PATCH 8/8] fix: Checkout correct branch --- .github/workflows/release.yml | 18 ++++++++++++++---- hack/build-tag.sh | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1bbc10b..2ca23b8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -41,12 +41,22 @@ jobs: fail-fast: false runs-on: ubuntu-22.04 steps: - - name: checkout + - name: clone run: | # can't use actions/checkout here as transferring the shallow clone fails when using upload-/download-artifact - git clone https://token:$GITHUB_TOKEN@github.com/$GITHUB_REPOSITORY . --depth=1 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + git clone https://github.com/$GITHUB_REPOSITORY . --depth=1 + - name: checkout PR + if: ${{ github.event_name == 'pull_request' }} + run: | + echo fetching pull/${{ github.ref_name }} + git fetch origin pull/${{ github.ref_name }}:pr --depth=1 + git checkout pr + - name: checkout branch + if: ${{ github.event_name == 'push' }} + run: | + echo fetching ${{ github.ref_name }} + git fetch origin ${{ github.ref_name }} --depth=1 + git checkout ${{ github.ref_name }} - name: Set up Go uses: actions/setup-go@v5 with: diff --git a/hack/build-tag.sh b/hack/build-tag.sh index 94e7257..2c8cca0 100755 --- a/hack/build-tag.sh +++ b/hack/build-tag.sh @@ -35,7 +35,7 @@ go run ./pip/generate TAG=v0.0.0-$PYTHON_VERSION-$PYTHON_STANDALONE_VERSION-$BUILD_NUM echo "checking out temporary branch" -git checkout $(git rev-parse HEAD) +git checkout --detach git add -f python/internal/data git add -f pip/internal/data git commit -m "added python $PYTHON_VERSION from python-standalone $PYTHON_STANDALONE_VERSION"