buildsys: add feature to vendor go modules #2053
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| /src/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| /src/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,160 @@ | ||||||||||||||||||||
| /*! | ||||||||||||||||||||
| Packages using the Go programming language may have upstream tar archives that | ||||||||||||||||||||
| include only the source code of the project, but not the source code of any | ||||||||||||||||||||
| dependencies. The Go programming language promotes the use of "modules" for | ||||||||||||||||||||
| dependencies and projects adopting modules will provide go.mod and go.sum | ||||||||||||||||||||
| files. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| This module provides the ability to retrieve and validate the dependencies | ||||||||||||||||||||
| declared using Go modules given a tar archive containing a go.mod and go.sum. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| The location where dependencies are retrieved from are controlled by the | ||||||||||||||||||||
| standard environment variables employed by the Go tool: GOPROXY, GOSUMDB, and | ||||||||||||||||||||
| GOPRIVATE. | ||||||||||||||||||||
|
|
||||||||||||||||||||
| */ | ||||||||||||||||||||
|
|
||||||||||||||||||||
| pub(crate) mod error; | ||||||||||||||||||||
| use error::Result; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| use super::manifest; | ||||||||||||||||||||
| use duct::cmd; | ||||||||||||||||||||
| use snafu::{OptionExt, ResultExt}; | ||||||||||||||||||||
| use std::env; | ||||||||||||||||||||
| use std::path::PathBuf; | ||||||||||||||||||||
| use std::process::Output; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| pub(crate) struct GoMod; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| impl GoMod { | ||||||||||||||||||||
| pub(crate) fn vendor( | ||||||||||||||||||||
| root_dir: &PathBuf, | ||||||||||||||||||||
| package_dir: &PathBuf, | ||||||||||||||||||||
| gomods: &[manifest::GoModule], | ||||||||||||||||||||
| ) -> Result<Self> { | ||||||||||||||||||||
|
There was a problem hiding this comment. Looks like you can just return nothing in the happy case:
Suggested change
|
||||||||||||||||||||
| for g in gomods { | ||||||||||||||||||||
| let input_path_arg = g.input.as_ref().context(error::InputFileSnafu)?; | ||||||||||||||||||||
| let input_path = package_dir.join(input_path_arg); | ||||||||||||||||||||
| if !input_path.is_file() { | ||||||||||||||||||||
| return Err(error::Error::InputFileBad); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+38
to
+40
There was a problem hiding this comment. I was wondering about the lack of the "Snafu" extension - should use
Suggested change
|
||||||||||||||||||||
| let mod_dir = g.mod_dir.as_ref().context(error::ModDirSnafu)?; | ||||||||||||||||||||
| let output_dir_arg = g.output_dir.as_ref().context(error::OutputDirSnafu)?; | ||||||||||||||||||||
| let output_dir = package_dir.join(output_dir_arg); | ||||||||||||||||||||
| if output_dir.exists() && !output_dir.is_dir() { | ||||||||||||||||||||
| return Err(error::Error::OutputDirBad); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Our SDK and toolchain are picked by the external `cargo make` invocation. | ||||||||||||||||||||
| let sdk = getenv("BUILDSYS_SDK_IMAGE")?; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| // Several Go variables control proxying | ||||||||||||||||||||
| let goproxy = go_env("GOPROXY").unwrap_or("".to_string()); | ||||||||||||||||||||
| let gosumdb = go_env("GOSUMDB").unwrap_or("".to_string()); | ||||||||||||||||||||
| let goprivate = go_env("GOPRIVATE").unwrap_or("".to_string()); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| let args = DockerGoArgs { | ||||||||||||||||||||
| module_path: package_dir.clone(), | ||||||||||||||||||||
| sdk_image: sdk, | ||||||||||||||||||||
| go_mod_cache: root_dir.join(".gomodcache".to_string()), | ||||||||||||||||||||
| command: format!( | ||||||||||||||||||||
| "mkdir -p {outdir} | ||||||||||||||||||||
| tar zxf {input} -C {outdir} && | ||||||||||||||||||||
|
There was a problem hiding this comment. You can just have
Suggested change
|
||||||||||||||||||||
| cd {outdir}/{moddir} && | ||||||||||||||||||||
| export GOPROXY={goproxy} && | ||||||||||||||||||||
| export GOSUMDB={gosumdb} && | ||||||||||||||||||||
| export GOPRIVATE={goprivate} && | ||||||||||||||||||||
| go list -mod=readonly ./... >/dev/null && go mod vendor", | ||||||||||||||||||||
| input = input_path_arg.to_string_lossy(), | ||||||||||||||||||||
| outdir = output_dir_arg.to_string_lossy(), | ||||||||||||||||||||
| moddir = mod_dir.to_string_lossy(), | ||||||||||||||||||||
| goproxy = goproxy, | ||||||||||||||||||||
| gosumdb = gosumdb, | ||||||||||||||||||||
| goprivate = goprivate, | ||||||||||||||||||||
| ), | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| match docker_go(root_dir, &args) { | ||||||||||||||||||||
| Ok(_) => continue, | ||||||||||||||||||||
| Err(e) => return Err(e), | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+76
to
+79
There was a problem hiding this comment. This can be written more concisely as:
Suggested change
(Might be able to get rid of the |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return Ok(Self); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| struct DockerGoArgs { | ||||||||||||||||||||
| module_path: PathBuf, | ||||||||||||||||||||
| sdk_image: String, | ||||||||||||||||||||
| go_mod_cache: PathBuf, | ||||||||||||||||||||
| command: String, | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// Run `docker-go` with the specified arguments. | ||||||||||||||||||||
| fn docker_go(root_dir: &PathBuf, dg_args: &DockerGoArgs) -> Result<Output> { | ||||||||||||||||||||
| let args = vec![ | ||||||||||||||||||||
| "--module-path", | ||||||||||||||||||||
| dg_args | ||||||||||||||||||||
| .module_path | ||||||||||||||||||||
| .to_str() | ||||||||||||||||||||
| .context(error::InputFileSnafu) | ||||||||||||||||||||
| .unwrap(), | ||||||||||||||||||||
|
Comment on lines
+97
to
+101
There was a problem hiding this comment. Hmm why do we still need to Generally that would mean that we would add another |
||||||||||||||||||||
| "--sdk-image", | ||||||||||||||||||||
| &dg_args.sdk_image, | ||||||||||||||||||||
| "--go-mod-cache", | ||||||||||||||||||||
| dg_args | ||||||||||||||||||||
| .go_mod_cache | ||||||||||||||||||||
| .to_str() | ||||||||||||||||||||
| .context(error::InputFileSnafu) | ||||||||||||||||||||
| .unwrap(), | ||||||||||||||||||||
| "--command", | ||||||||||||||||||||
| &dg_args.command, | ||||||||||||||||||||
| ]; | ||||||||||||||||||||
| let program = root_dir.join("tools/docker-go"); | ||||||||||||||||||||
| println!("program: {}", program.to_string_lossy()); | ||||||||||||||||||||
| let output = cmd(program, args) | ||||||||||||||||||||
| .stderr_to_stdout() | ||||||||||||||||||||
| .stdout_capture() | ||||||||||||||||||||
| .unchecked() | ||||||||||||||||||||
| .run() | ||||||||||||||||||||
| .context(error::CommandStartSnafu)?; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| let stdout = String::from_utf8_lossy(&output.stdout); | ||||||||||||||||||||
| println!("{}", &stdout); | ||||||||||||||||||||
| return if output.status.success() { | ||||||||||||||||||||
| Ok(output) | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| Err(error::Error::DockerExecution { | ||||||||||||||||||||
| args: "".to_string(), | ||||||||||||||||||||
| }) | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
Comment on lines
+124
to
+130
There was a problem hiding this comment.
Suggested change
|
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// Run `go env` with the specified argument. | ||||||||||||||||||||
| fn go_env(var: &str) -> Option<String> { | ||||||||||||||||||||
| let args = vec!["env", var]; | ||||||||||||||||||||
| let output = match cmd("go", args) | ||||||||||||||||||||
| .stderr_to_stdout() | ||||||||||||||||||||
| .stdout_capture() | ||||||||||||||||||||
| .unchecked() | ||||||||||||||||||||
| .run() | ||||||||||||||||||||
| { | ||||||||||||||||||||
| Ok(v) => v, | ||||||||||||||||||||
| Err(_) => return None, | ||||||||||||||||||||
| }; | ||||||||||||||||||||
| let stdout = String::from_utf8_lossy(&output.stdout); | ||||||||||||||||||||
| println!("{}", &stdout); | ||||||||||||||||||||
| return if output.status.success() { | ||||||||||||||||||||
| Some(stdout.to_string()) | ||||||||||||||||||||
| } else { | ||||||||||||||||||||
| None | ||||||||||||||||||||
| }; | ||||||||||||||||||||
|
Comment on lines
+147
to
+151
There was a problem hiding this comment. Does this work?
Suggested change
|
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| /// Retrieve a BUILDSYS_* variable that we expect to be set in the environment, | ||||||||||||||||||||
| /// and ensure that we track it for changes, since it will directly affect the | ||||||||||||||||||||
| /// output. | ||||||||||||||||||||
| fn getenv(var: &str) -> Result<String> { | ||||||||||||||||||||
| println!("cargo:rerun-if-env-changed={}", var); | ||||||||||||||||||||
| env::var(var).context(error::EnvironmentSnafu { var }) | ||||||||||||||||||||
| } | ||||||||||||||||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| use snafu::Snafu; | ||
|
|
||
| #[derive(Debug, Snafu)] | ||
| #[snafu(visibility(pub(super)))] | ||
| pub(crate) enum Error { | ||
| #[snafu(display("Failed to start command: {}", source))] | ||
| CommandStart { source: std::io::Error }, | ||
|
|
||
| #[snafu(display("Failed to execute command: 'docker {}'", args))] | ||
| DockerExecution { args: String }, | ||
|
|
||
| #[snafu(display("input is required"))] | ||
| InputFile, | ||
|
|
||
| #[snafu(display("input must be a file and end with .tar.gz"))] | ||
| InputFileBad, | ||
|
Comment on lines
+15
to
+16
There was a problem hiding this comment. nit: we don't seem to check for the "ends with .tar.gz" part (and might not want to, since other compression formats might be used) |
||
|
|
||
| #[snafu(display("mod-dir is required"))] | ||
| ModDir, | ||
|
|
||
| #[snafu(display("output-dir is required"))] | ||
| OutputDir, | ||
|
|
||
| #[snafu(display("output_dir must not exist or be a directory"))] | ||
| OutputDirBad, | ||
|
|
||
| #[snafu(display("Missing environment variable '{}'", var))] | ||
| Environment { | ||
| var: String, | ||
| source: std::env::VarError, | ||
| }, | ||
| } | ||
|
|
||
| pub(super) type Result<T> = std::result::Result<T, Error>; | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd tend to prefer coming up with some standard contract and automating that in a way that's easy for spec files to consume, so there's less risk of each spec doing things in slightly different ways, which tends to obscure packaging mistakes.
Specifically, if instead of an output dir we created a tarball, then we could reference that archive in the spec file in the usual way - listing it as a
Source, and using the%setupor%autosetupmacro. The latter would be valuable for any patches we'd carry. There's also a modest performance benefit from not having lots of small files to copy into the build context.Then we could specify the behavior into the
external-filesdeclaration like:(Not entirely sure about what to name the output file but there's probably an automatic convention like
x.tar.*->x-bundled.tar.*that we could use.)What do you think?