Add stepman bin cache #950
Conversation
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
6ce76c1 to
1eec4f1
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
3d824fe to
949510f
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
4 times, most recently
from
774e28d to
57a8bfe
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
57a8bfe to
1d28f2a
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
961e22f to
bec0030
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
bec0030 to
3e2cbcc
Compare
toolkits/golang.go
Outdated
| @@ -334,6 +334,23 @@ func (toolkit GoToolkit) PrepareForStepRun(step stepmanModels.StepModel, sIDData | |||
| return goBuildStep(&defaultRunner{}, goConfig, step.Toolkit.Go.PackageName, stepAbsDirPath, fullStepBinPath) | |||
| } | |||
|
|
|||
| func GoBuildStep(stepAbsDirPath string, packageName string, fullStepBinPath string) error { | |||
There was a problem hiding this comment.
nit: I know we probably won't introduce new toolkits, but I still think this method should be part of the toolkit interface. Offline mode is now a CLI-wide concept, so every toolkit should support it
| return nil | ||
| } | ||
|
|
||
| compressCmd := command.New("zstd", "--patch-from="+targetExecutablePathLatest, targetExecutablePath, "-o", patchFilePath) |
There was a problem hiding this comment.
Not urgent for the prototyping, but we should not forget that this brings a new implicit dependency. I think bitrise setup should at least check that zstd is available in $PATH when the offline mode is enabled.
| return checkChecksum(targetExecutablePath, checkSumPath) | ||
| } | ||
|
|
||
| func writeChecksum(patchFilePath, checksumPath string) error { |
There was a problem hiding this comment.
Idea: SHA256 checksums are not that long when converted to ASCII, so we could also append them to the filenames. This could simplify the code in this file (and also make operations atomic)
There was a problem hiding this comment.
Inspired by the Nix store path format: https://nixos.org/manual/nix/stable/store/store-path
| workers = 10 | ||
| ) | ||
|
|
||
| type GoBuilder func(stepSourceAbsPath, packageName, targetExecutablePath string) error |
There was a problem hiding this comment.
This whole file is heavily coupled with the Go step toolkit while stepman should be neutral. I would rather expend the step toolkit interface with the required methods and hooks to make this code uncoupled from any single toolkit.
There was a problem hiding this comment.
Like the idea of expanding the toolkit interface.
However we can not use the toolkit interface here directly, as it would result in circular dependency.
Can look into removing the Go specificness.
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
8ecef0c to
b5dfaf9
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
eecc865 to
0245219
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
324cab6 to
95f7da7
Compare
lpusok
force-pushed
the
ACI-2038-add-stepman-bin-cache
branch
from
eeb95a4 to
dfde6d2
Compare
toolkits/golang.go
Outdated
| } | ||
|
|
||
| func (toolkit GoToolkit) PrepareForStepRun(step stepmanModels.StepModel, sIDData models.StepIDData, activatedStep stepmanModels.ActivatedStep) (stepmanModels.ActivatedStep, error) { | ||
| if activatedStep.Type == stepmanModels.ActivatedStepTypeExecutable { |
There was a problem hiding this comment.
A seperate case if the step is already precompiled.
| ActivatedStepTypeExecutable ActivatedStepType = "executable" | ||
| ) | ||
|
|
||
| type ActivatedStep struct { |
There was a problem hiding this comment.
Todo: still not ideal as can be both source and executable at the same time.
Checklist
README.mdis updated with the changes (if needed)Version
Requires a MAJOR/MINOR/PATCH version update
Context
Changes
Investigation details
Decisions