feat: use build_dir in esbuild workflow to support building in source #437
Conversation
| @@ -81,11 +81,10 @@ class NodejsNpmInstallAction(BaseAction): | |||
| DESCRIPTION = "Installing dependencies from NPM" | |||
| PURPOSE = Purpose.RESOLVE_DEPENDENCIES | |||
|
|
|||
| def __init__(self, artifacts_dir, subprocess_npm): | |||
| def __init__(self, install_dir, subprocess_npm): | |||
There was a problem hiding this comment.
renamed to generalize (e.g. now we can install in the source directory as well, so this should not be specific to the artifacts dir)
There was a problem hiding this comment.
Something to consider when changing the arguments, let's make a callout to not changing the RPC protocol version because we did not touch the workflow args. https://github.com/aws/aws-lambda-builders/blob/develop/aws_lambda_builders/__init__.py#L8
If not considered, we should think of adding that into PR template so that we ask ourselves that question.
There was a problem hiding this comment.
actually, when should we change the RPC protocol version? Is it when we change any of the params passed in the request here? Or is it only on breaking changes for the params there?
There was a problem hiding this comment.
It looks good to me, thanks Rup. I just have a question for the integration test case, my assumption is building in source should not be different than build in the scratch directory. If this assumption is correct, can we update our integration test cases to run in both source and scratch and make sure that it is working fine in both instead of having a new test case for build in source
|
as @moelasmar suggested, we could parameterize, but I'm not blocking on that. |
|
Hmm I see what you guys are saying, but the behaviour is slightly different when building in source (no copy operation, no dealing with |
my point is we need to make sure that all the cases we support in |
|
@moelasmar yep, I get what you're saying. It just seems weird to me that because we added a new parameter, we'll have to test all combinations of it with all of the existing tests. It seems a bit much to me 🤔 |
|
Discussed with the team, merging for now to close the task. If we feel like we need more tests, or need a refactoring of our integration tests, we can create another task/PR for this. |
* feat: support rust via cargo * make black reformat * use os.path.join in path assertion tests to address windows paths * address pylint ci errors * fix reformatted list comprehensions * try passing rust-lld linker on windows * update rust cargo design doc. windows support was added and tested * add test for cargo workspaces project * reformat test sources * build with musl on linux because glibc may differ on lambda * update linux copy and bin paths * update make test again, use the version appveyor is complaining about * update integration tests for rust cargo with latest aws rust runtime interfaces * align TestCustomMakeWorkflow integ test assumptions with appveyor reality * make x86_64-unknown-linux-musl a const for the rust cargo workflow * add integ test for failing cargo rust build * Update Rust workflow to use cargo-lambda Cargo-lambda takes care of cross compiling using Zig as linker. This works on Windows, Linux, and MacOS natively. Signed-off-by: David Calavera <[email protected]> * Fix deprecation warnings. Signed-off-by: David Calavera <[email protected]> * Install Zig on Windows manually Signed-off-by: David Calavera <[email protected]> * Print Zig version on Windows Signed-off-by: David Calavera <[email protected]> * Update version of cargo-lambda Signed-off-by: David Calavera <[email protected]> * Run windows tests in powershell Signed-off-by: David Calavera <[email protected]> * Fix powershell env notation Signed-off-by: David Calavera <[email protected]> * Print clang version on windows Signed-off-by: David Calavera <[email protected]> * Fix env variable name Signed-off-by: David Calavera <[email protected]> * Try new version of zigbuild that fixes some linker issues on Windows. Signed-off-by: David Calavera <[email protected]> * Update releases URL. Signed-off-by: David Calavera <[email protected]> * Upgrade LLVM and clang Signed-off-by: David Calavera <[email protected]> * Update visual studio image To check if that makes any difference. Signed-off-by: David Calavera <[email protected]> * Change the visual studio image everywhere. Signed-off-by: David Calavera <[email protected]> * Revert upgrade changes They didn't fix the problem Signed-off-by: David Calavera <[email protected]> * Print environment Signed-off-by: David Calavera <[email protected]> * Update to Visual Studio 2022 Signed-off-by: David Calavera <[email protected]> * Fix python variable Signed-off-by: David Calavera <[email protected]> * Fix package urls Signed-off-by: David Calavera <[email protected]> * Update missing vs 2019 reference. Signed-off-by: David Calavera <[email protected]> * Change windows package suffix Signed-off-by: David Calavera <[email protected]> * Update cargo-lambda to version 0.9.0 Signed-off-by: David Calavera <[email protected]> * Go back to the original VS version. Signed-off-by: David Calavera <[email protected]> * Cleanup options - Use architecture to setup the right build target. - Don't require `--bin` flag, the default behaviour should work for the majority of functions. - Add flags option to provide a list of additional flags for projects that need extra configuration, like projects within a workspace. Signed-off-by: David Calavera <[email protected]> * Add integration test with cargo_lambda_flags Signed-off-by: David Calavera <[email protected]> * Detect binaries when the project only includes one function. Signed-off-by: David Calavera <[email protected]> * Bring back handler as an optional argument. It saves some duplicated flags for working with workspaces. Signed-off-by: David Calavera <[email protected]> * Ignore errors if directory doesn't exist. Signed-off-by: David Calavera <[email protected]> * Add debug logs Set RUST_LOG=debug when debug is enabled. Signed-off-by: David Calavera <[email protected]> * Log the artifact destination path. Signed-off-by: David Calavera <[email protected]> * Add missing comma Signed-off-by: David Calavera <[email protected]> * Print out and err in the log when debug is enabled Signed-off-by: David Calavera <[email protected]> * Only set RUST_LOG when it's not already set Log it's value, so users know what's set at. Signed-off-by: David Calavera <[email protected]> * Add experimentalCargoLambda feature flag. Signed-off-by: David Calavera <[email protected]> * Add integration test for multi-function projects. Signed-off-by: David Calavera <[email protected]> * Remove type hints They're causing false positives in Python 3.9 with pylint. Signed-off-by: David Calavera <[email protected]> * Add new CI steps for GHA Signed-off-by: David Calavera <[email protected]> * Add build_in_source_support Signed-off-by: David Calavera <[email protected]> * Test rust logger Signed-off-by: David Calavera <[email protected]> * Fix assertion Signed-off-by: David Calavera <[email protected]> * Don't fail fast Signed-off-by: David Calavera <[email protected]> * fix: Fix failing esbuild integration tests (#423) * fix: Fix failing esbuild integration tests * Replace npm ci with npm install * Remove default shell Signed-off-by: David Calavera <[email protected]> * Revert "Remove default shell" This reverts commit 478c3b6. * Add check to ensure that Cargo Lambda is installed. Include a link to the gettings started guide that gives direct installation instructions based on the platform. Signed-off-by: David Calavera <[email protected]> * feat: Add support for mjs files with esbuild (#427) * feat: Add support for mjs files with esbuild * Black reformat * Test Cargo Lambda check Signed-off-by: David Calavera <[email protected]> * Don't redefine which Signed-off-by: David Calavera <[email protected]> * Organize code in more modules This structure follows other workflows, and provides better testeability. Signed-off-by: David Calavera <[email protected]> * Add more documentation Signed-off-by: David Calavera <[email protected]> * Format code Signed-off-by: David Calavera <[email protected]> * Capture exception Signed-off-by: David Calavera <[email protected]> * chore: Remove type/bug label for Bug Issue Template (#425) Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Mehmet Nuri Deveci <[email protected]> * Clean design doc and tests Signed-off-by: David Calavera <[email protected]> * chore: Version bump to 1.25.0 (#429) * refactor: assign each workflow a default build directory (#428) * fix: remove unused symlinking (#432) * chore: Move to ruff from pylint (#435) When we started the project we defaulted to use pylint. Pylint has served it's purpose but it pretty slow. Ruff is a newer linter in the python ecosystem but is written in Rust. This makes Ruff was faster than pylint. On my machine (while testing in SAM CLI), pylint took about 70s to lint the repo but with ruff it took .04s. Co-authored-by: Jacob Fuss <[email protected]> * chore: Enable pylint within ruff (#436) Co-authored-by: Jacob Fuss <[email protected]> * refactor: esbuild refactor for readability (#433) * feat: use build_dir in esbuild workflow to support building in source (#437) * Fix formatting Signed-off-by: David Calavera <[email protected]> * Update build in source settings Signed-off-by: David Calavera <[email protected]> * fix: remove python3.6 support (#434) * chore: bump version to 1.26.0 (#441) * Remove default value for subprocess_cargo_lambda Signed-off-by: David Calavera <[email protected]> * feat: Pin ruff version, add dependabot config (#442) * feat: Pin ruff version, add dependabot config to keep our dependencies up to date * Exlude isort and flake8 from dependabot updates * feat: Add sources content flag to supported esbuild options (#439) * Better process management Signed-off-by: David Calavera <[email protected]> * Make release mode the default Signed-off-by: David Calavera <[email protected]> * Remove already default None Signed-off-by: David Calavera <[email protected]> * Turn debug message into warning Signed-off-by: David Calavera <[email protected]> * Update documentation format Signed-off-by: David Calavera <[email protected]> * Fix formatting Signed-off-by: David Calavera <[email protected]> * Preserve binary permissions Use copy2 instead of copyfile Signed-off-by: David Calavera <[email protected]> --------- Signed-off-by: David Calavera <[email protected]> Co-authored-by: softprops <[email protected]> Co-authored-by: Daniel Mil <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Jacob Fuss <[email protected]> Co-authored-by: Mehmet Nuri Deveci <[email protected]> Co-authored-by: Ruperto Torres <[email protected]>
Issue #, if available:
Description of changes:
Now that
build_dirwill point to either the source directory, if building in source, or the workflow's default build directory, we can refactor the workflow to usebuild_dirand it will enable build-in-source support.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.