refactor: assign each workflow a default build directory #428

torresxb1 · 2023-01-18T00:27:33Z

Issue #, if available:

Description of changes:

A recent change (#418) added BUILD_IN_SOURCE_BY_DEFAULT to each workflow. This PR replaces that with DEFAULT_BUILD_DIR, which helps us generalize some logic more. We can now set and use a workflow's build_dir property using a combination of:

the value of the build_in_source parameter passed in to lambda builders
- if user wants to build in source, the build directory should be set to the source directory
the workflow's default build directory (DEFAULT_BUILD_DIR)
- in case build_in_source is not set, or it's set to False, or an unsupported value is passed in, we should use the workflow's existing default build directory

Note that we're still able to find out if a workflow builds in source by default just by checking if their DEFAULT_BUILD_DIR is the source directory.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

torresxb1 · 2023-01-18T00:53:09Z

aws_lambda_builders/workflows/custom_make/workflow.py


        self.actions.append(make_action)

-    def _select_working_directory(self, source_dir: str, scratch_dir: str, build_in_source: bool):


Due to the changes in this PR, we can remove the logic of choosing a working/build directory based on the build_in_source parameter. Similarly, when we implement building in source for other workflows we won't have to define this kind of logic either because the parent Workflow class now handles setting the build_dir.

torresxb1 · 2023-01-18T18:19:57Z

aws_lambda_builders/workflow.py

+        # Actions are registered by the subclasses as they seem fit
+        self.actions = []
+        self._binaries = {}


just moved these up, didn't add these

sriram-mv · 2023-01-26T00:56:16Z

We set a different build_dir per workflow is that any change in behavior from what is present today?

torresxb1 · 2023-01-26T18:26:16Z

We set a different build_dir per workflow is that any change in behavior from what is present today?

No change in customer-facing behaviour. Today each workflow has a different default build/install dir (some already build in the source directory, some in scratch, some in artifacts).

This just explicitly (and clearly) writes what that default is for each workflow, and makes it a class attribute so that the parent class (BaseWorkflow) can generalize some logic: the build directory should be the source directory if we want to build in source, otherwise it should be the default.

sriram-mv

Approving since its a backward compatible change.

* 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 <david.calavera@gmail.com> * Fix deprecation warnings. Signed-off-by: David Calavera <david.calavera@gmail.com> * Install Zig on Windows manually Signed-off-by: David Calavera <david.calavera@gmail.com> * Print Zig version on Windows Signed-off-by: David Calavera <david.calavera@gmail.com> * Update version of cargo-lambda Signed-off-by: David Calavera <david.calavera@gmail.com> * Run windows tests in powershell Signed-off-by: David Calavera <david.calavera@gmail.com> * Fix powershell env notation Signed-off-by: David Calavera <david.calavera@gmail.com> * Print clang version on windows Signed-off-by: David Calavera <david.calavera@gmail.com> * Fix env variable name Signed-off-by: David Calavera <david.calavera@gmail.com> * Try new version of zigbuild that fixes some linker issues on Windows. Signed-off-by: David Calavera <david.calavera@gmail.com> * Update releases URL. Signed-off-by: David Calavera <david.calavera@gmail.com> * Upgrade LLVM and clang Signed-off-by: David Calavera <david.calavera@gmail.com> * Update visual studio image To check if that makes any difference. Signed-off-by: David Calavera <david.calavera@gmail.com> * Change the visual studio image everywhere. Signed-off-by: David Calavera <david.calavera@gmail.com> * Revert upgrade changes They didn't fix the problem Signed-off-by: David Calavera <david.calavera@gmail.com> * Print environment Signed-off-by: David Calavera <david.calavera@gmail.com> * Update to Visual Studio 2022 Signed-off-by: David Calavera <david.calavera@gmail.com> * Fix python variable Signed-off-by: David Calavera <david.calavera@gmail.com> * Fix package urls Signed-off-by: David Calavera <david.calavera@gmail.com> * Update missing vs 2019 reference. Signed-off-by: David Calavera <david.calavera@gmail.com> * Change windows package suffix Signed-off-by: David Calavera <david.calavera@gmail.com> * Update cargo-lambda to version 0.9.0 Signed-off-by: David Calavera <david.calavera@gmail.com> * Go back to the original VS version. Signed-off-by: David Calavera <david.calavera@gmail.com> * 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 <david.calavera@gmail.com> * Add integration test with cargo_lambda_flags Signed-off-by: David Calavera <david.calavera@gmail.com> * Detect binaries when the project only includes one function. Signed-off-by: David Calavera <david.calavera@gmail.com> * Bring back handler as an optional argument. It saves some duplicated flags for working with workspaces. Signed-off-by: David Calavera <david.calavera@gmail.com> * Ignore errors if directory doesn't exist. Signed-off-by: David Calavera <david.calavera@gmail.com> * Add debug logs Set RUST_LOG=debug when debug is enabled. Signed-off-by: David Calavera <david.calavera@gmail.com> * Log the artifact destination path. Signed-off-by: David Calavera <david.calavera@gmail.com> * Add missing comma Signed-off-by: David Calavera <david.calavera@gmail.com> * Print out and err in the log when debug is enabled Signed-off-by: David Calavera <david.calavera@gmail.com> * 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 <david.calavera@gmail.com> * Add experimentalCargoLambda feature flag. Signed-off-by: David Calavera <david.calavera@gmail.com> * Add integration test for multi-function projects. Signed-off-by: David Calavera <david.calavera@gmail.com> * Remove type hints They're causing false positives in Python 3.9 with pylint. Signed-off-by: David Calavera <david.calavera@gmail.com> * Add new CI steps for GHA Signed-off-by: David Calavera <david.calavera@gmail.com> * Add build_in_source_support Signed-off-by: David Calavera <david.calavera@gmail.com> * Test rust logger Signed-off-by: David Calavera <david.calavera@gmail.com> * Fix assertion Signed-off-by: David Calavera <david.calavera@gmail.com> * Don't fail fast Signed-off-by: David Calavera <david.calavera@gmail.com> * 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 <david.calavera@gmail.com> * 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 <david.calavera@gmail.com> * 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 <david.calavera@gmail.com> * Don't redefine which Signed-off-by: David Calavera <david.calavera@gmail.com> * Organize code in more modules This structure follows other workflows, and provides better testeability. Signed-off-by: David Calavera <david.calavera@gmail.com> * Add more documentation Signed-off-by: David Calavera <david.calavera@gmail.com> * Format code Signed-off-by: David Calavera <david.calavera@gmail.com> * Capture exception Signed-off-by: David Calavera <david.calavera@gmail.com> * chore: Remove type/bug label for Bug Issue Template (#425) Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> * Clean design doc and tests Signed-off-by: David Calavera <david.calavera@gmail.com> * 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 <jfuss@users.noreply.github.com> * chore: Enable pylint within ruff (#436) Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> * 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 <david.calavera@gmail.com> * Update build in source settings Signed-off-by: David Calavera <david.calavera@gmail.com> * 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 <david.calavera@gmail.com> * 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 <david.calavera@gmail.com> * Make release mode the default Signed-off-by: David Calavera <david.calavera@gmail.com> * Remove already default None Signed-off-by: David Calavera <david.calavera@gmail.com> * Turn debug message into warning Signed-off-by: David Calavera <david.calavera@gmail.com> * Update documentation format Signed-off-by: David Calavera <david.calavera@gmail.com> * Fix formatting Signed-off-by: David Calavera <david.calavera@gmail.com> * Preserve binary permissions Use copy2 instead of copyfile Signed-off-by: David Calavera <david.calavera@gmail.com> --------- Signed-off-by: David Calavera <david.calavera@gmail.com> Co-authored-by: softprops <d.tangren@gmail.com> Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Jacob Fuss <32497805+jfuss@users.noreply.github.com> Co-authored-by: Jacob Fuss <jfuss@users.noreply.github.com> Co-authored-by: Mehmet Nuri Deveci <5735811+mndeveci@users.noreply.github.com> Co-authored-by: Ruperto Torres <86501267+torresxb1@users.noreply.github.com>

torresxb1 added 2 commits January 17, 2023 16:19

refactor: assign each workflow a default build directory

5084944

typo

b425e38

torresxb1 requested a review from a team as a code owner January 18, 2023 00:27

torresxb1 requested review from jfuss and lucashuy January 18, 2023 00:27

Merge branch 'develop' into refactor-build-in-source-build-dir

aa22e89

github-actions bot added area/workflow/go_modules area/workflow/java_gradle area/workflow/java_maven area/workflow/node_npm area/workflow/python_pip area/workflow/ruby_bundler pr/internal labels Jan 18, 2023

torresxb1 marked this pull request as draft January 18, 2023 00:28

black

879d7d6

torresxb1 commented Jan 18, 2023

View reviewed changes

fix custom_make workflow to ensure it matches previous behaviour

6f73545

torresxb1 commented Jan 18, 2023

View reviewed changes

add comment

a5902a0

torresxb1 marked this pull request as ready for review January 18, 2023 19:51

update comment

7132cb6

torresxb1 requested review from sriram-mv and removed request for jfuss January 25, 2023 22:22

lucashuy approved these changes Jan 25, 2023

View reviewed changes

sriram-mv approved these changes Jan 27, 2023

View reviewed changes

torresxb1 enabled auto-merge (squash) January 27, 2023 18:29

Merge branch 'develop' into refactor-build-in-source-build-dir

80266dd

torresxb1 merged commit 4008bdd into aws:develop Jan 27, 2023

torresxb1 deleted the refactor-build-in-source-build-dir branch January 27, 2023 19:45

calavera pushed a commit to calavera/aws-lambda-builders that referenced this pull request Feb 8, 2023

refactor: assign each workflow a default build directory (aws#428)

5e642a8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: assign each workflow a default build directory #428

refactor: assign each workflow a default build directory #428

Uh oh!

torresxb1 commented Jan 18, 2023 •

edited

Loading

Uh oh!

torresxb1 Jan 18, 2023 •

edited

Loading

Uh oh!

torresxb1 Jan 18, 2023

Uh oh!

sriram-mv commented Jan 26, 2023

Uh oh!

torresxb1 commented Jan 26, 2023

Uh oh!

sriram-mv left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		self.actions.append(make_action)

		def _select_working_directory(self, source_dir: str, scratch_dir: str, build_in_source: bool):

refactor: assign each workflow a default build directory #428

refactor: assign each workflow a default build directory #428

Uh oh!

Conversation

torresxb1 commented Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

torresxb1 Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

torresxb1 Jan 18, 2023

Choose a reason for hiding this comment

Uh oh!

sriram-mv commented Jan 26, 2023

Uh oh!

torresxb1 commented Jan 26, 2023

Uh oh!

sriram-mv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

torresxb1 commented Jan 18, 2023 •

edited

Loading

torresxb1 Jan 18, 2023 •

edited

Loading