Skip to content

Commit

Permalink
Adding -timeout to constrain how much time we spend building packages
Browse files Browse the repository at this point in the history
  • Loading branch information
blast-hardcheese committed Jan 24, 2024
1 parent a27837e commit 1816364
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 8 deletions.
7 changes: 5 additions & 2 deletions internal/backends/python/gen_pypi_map/gen_pypi_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"sort"
"strings"
"time"
)

/*
Expand Down Expand Up @@ -57,6 +58,7 @@ func cmd_test(args []string) {
testForce := testCommandSet.Bool("force", false, "Force re-test when cached")
testPkgsFile := testCommandSet.String("pkgsfile", "pkgs.json", "A file where to store permanent information for each module.")
testThreshold := testCommandSet.Int("threshold", 10000, "Only process packages with at least this many downloads")
testTimeout := testCommandSet.Int("timeout", 60, "The maximum number of seconds to wait for a package to install.")
if err := testCommandSet.Parse(args); err != nil {
fmt.Fprintf(os.Stderr, "Failed to parse test flags: %s\n", err)
return
Expand Down Expand Up @@ -105,7 +107,7 @@ func cmd_test(args []string) {
} else {
packages, _ = NewPackageIndex("https://pypi.org/simple/", -1)
}
TestModules(packages, *testCache, *testPkgsFile, *testDistMods, *testWorkers, *testForce)
TestModules(packages, *testCache, *testPkgsFile, *testDistMods, *testWorkers, *testForce, time.Duration(*testTimeout)*time.Second)
}

func cmd_test_one(args []string) {
Expand All @@ -119,6 +121,7 @@ func cmd_test_one(args []string) {
testOneDistMods := testOneCommandSet.Bool("distMods", false, "Determine modules by examining dists")
testOneForce := testOneCommandSet.Bool("force", false, "Force re-test when cached")
testOnePkgsFile := testOneCommandSet.String("pkgsfile", "pkgs.json", "A file where to store permanent information for each module.")
testOneTimeout := testOneCommandSet.Int("timeout", 60, "The maximum number of seconds to wait for a package to install.")
if err := testOneCommandSet.Parse(args); err != nil {
fmt.Fprintf(os.Stderr, "Failed to parse test flags: %s\n", err)
return
Expand All @@ -129,7 +132,7 @@ func cmd_test_one(args []string) {
}

cache := LoadAllPackageInfo(*testOneCache, *testOnePkgsFile)
info, err := ProcessPackage(*testOnePackage, cache, *testOneCache, *testOneDistMods, *testOneForce)
info, err := ProcessPackage(*testOnePackage, cache, *testOneCache, *testOneDistMods, *testOneForce, time.Duration(*testOneTimeout)*time.Second)
if err != nil {
fmt.Fprintf(os.Stderr, "Error processing package: %v\n", err)
return
Expand Down
29 changes: 27 additions & 2 deletions internal/backends/python/gen_pypi_map/install_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os/exec"
"path/filepath"
"strings"
"time"
)

type InstallDiffResponse struct {
Expand Down Expand Up @@ -42,12 +43,30 @@ func discoverValidSuffixes() ([]string, error) {
return res, nil
}

func InstallDiff(metadata PackageData) ([]string, error) {
func InstallDiff(metadata PackageData, timeout time.Duration) ([]string, error) {
root := "/tmp/pypi/" + metadata.Info.Name

// Run pip to install just the package, so we can statically analyze it
cmd := exec.Command("pip", "install", "--no-deps", "--target", root, metadata.Info.Name)

killed := false
installing := true
go func() {
start := time.Now()
for installing {
elapsed := time.Since(start)
if elapsed > timeout {
killed = true
err := cmd.Process.Signal(os.Interrupt)
if err != nil {
err = cmd.Process.Signal(os.Kill)
}
break
}
time.Sleep(1 * time.Second)
}
}()

_, err := cmd.StdoutPipe()

if err != nil {
Expand All @@ -56,9 +75,15 @@ func InstallDiff(metadata PackageData) ([]string, error) {

err = cmd.Run()
if err != nil {
return nil, PypiError{InstallFailure, "Failed to start installer", err}
if killed {
return nil, PypiError{InstallFailure, "Exceeded timeout", err}
} else {
return nil, PypiError{InstallFailure, "Failed to start installer", err}
}
}

installing = false

suffixes, err := discoverValidSuffixes()
if err != nil {
return nil, err
Expand Down
8 changes: 4 additions & 4 deletions internal/backends/python/gen_pypi_map/test_modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func formatSeconds(totalSeconds int) string {
return fmt.Sprintf("%02dd%02dh%02dm%02ds", days, hours, minutes, seconds)
}

func TestModules(packages PackageIndex, cacheDir string, pkgsFile string, distMods bool, workers int, force bool) {
func TestModules(packages PackageIndex, cacheDir string, pkgsFile string, distMods bool, workers int, force bool, timeout time.Duration) {

cache := LoadAllPackageInfo(cacheDir, pkgsFile)

Expand Down Expand Up @@ -52,7 +52,7 @@ func TestModules(packages PackageIndex, cacheDir string, pkgsFile string, distMo
concurrencyLimiter <- struct{}{}
defer func() { <-concurrencyLimiter }()

packageInfo, err := ProcessPackage(packageName, cache, cacheDir, distMods, force)
packageInfo, err := ProcessPackage(packageName, cache, cacheDir, distMods, force, timeout)
packageInfo.Name = packageName
if err != nil {
fmt.Fprintf(os.Stderr, "Failed to process package [%v]: %v\n", packageName, err)
Expand Down Expand Up @@ -140,7 +140,7 @@ func GetPackageMetadata(packageName string) (PackageData, error) {
}

// NOTE: cache is read only
func ProcessPackage(packageName string, cache map[string]PackageInfo, cacheDir string, distMods bool, force bool) (PackageInfo, error) {
func ProcessPackage(packageName string, cache map[string]PackageInfo, cacheDir string, distMods bool, force bool, timeout time.Duration) (PackageInfo, error) {
// Get the package metadata from pypi
metadata, err := GetPackageMetadata(packageName)
if err != nil {
Expand All @@ -162,7 +162,7 @@ func ProcessPackage(packageName string, cache map[string]PackageInfo, cacheDir s
modules, err = GetModules(metadata)
} else {
// Determine the modules by installing the package
modules, err = InstallDiff(metadata)
modules, err = InstallDiff(metadata, timeout)
}

var retval PackageInfo
Expand Down

0 comments on commit 1816364

Please sign in to comment.