Skip to content

Commit

Permalink
Merge dotnet_build_support branch to develop (aws#1127)
Browse files Browse the repository at this point in the history
* chore(version): set 0.14.3.dev1 version (aws#1112) (aws#1113)

* Depend on development version of lambda-builders for dev builds (aws#1111)

* Depend on development version of lambda-builders for dev builds

* Adding prod.txt to manifest

* Splitting dev and tool dependencies

* fix(build): Resolve path after .aws-sam is created (aws#1110)

* fix(build): Resolve path after .aws-sam is created

* fix: build (make pr)

* Design and implementation for producing debug build artifacts (aws#1095)

* design: Initial Design for producing debug artifacts

* initial implementation

* Adding unit tests

* Integration test with debug build mode

* Adjust Design doc and add keyword arg to a call

* fix(dotnet): init template fixes (aws#1117)

* chore(version): set 0.15.0 (aws#1125)

* Revert "Depend on development version of lambda-builders for dev builds (aws#1111)" (aws#1128)

This reverts commit 7e9de790e23791ba176faff2030286db4007e503.

* Bumping to Lambda Builders 0.3.0 (aws#1129)

Bumping to Lambda Builders 0.3.0

* fix(func-tests): add dependency manager param (aws#1130)
  • Loading branch information
sanathkr authored and sriram-mv committed Apr 16, 2019
1 parent 9fcd828 commit 846dfc3
Show file tree
Hide file tree
Showing 21 changed files with 377 additions and 60 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -382,5 +382,7 @@ $RECYCLE.BIN/
tests/integration/buildcmd/scratch
tests/integration/testdata/buildcmd/Dotnetcore2.0/bin
tests/integration/testdata/buildcmd/Dotnetcore2.0/obj
tests/integration/testdata/buildcmd/Dotnetcore2.1/bin
tests/integration/testdata/buildcmd/Dotnetcore2.1/obj

# End of https://www.gitignore.io/api/osx,node,macos,linux,python,windows,pycharm,intellij,sublimetext,visualstudiocode
135 changes: 135 additions & 0 deletions designs/build_debug_artifacts.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
SAM Build producing debuggable artifacts
========================================

What is the problem?
--------------------

`sam build` produces artifacts that are built for production use. In some langauges (usually interpreted),
these production artifacts are also debuggable locally. In the case of compiled languages, you usually need to compile
the binary or artifact in a specific manner for them to be debuggable.

What will be changed?
---------------------

We will introduce a way in `sam build` to produce these debuggable artifacts for those compiled languages.

Success criteria for the change
-------------------------------

1. Artifacts generated will be debuggable for runtimes DotNetCore 2.0 and above.

Out-of-Scope
------------

1. Other languages `sam build` supports will not be changed

User Experience Walkthrough
---------------------------

Implementation
==============

CLI Changes
-----------

*Explain the changes to command line interface, including adding new
commands, modifying arguments etc*

### Breaking Change

Changes are additive and will not break any existing functionality.

Design
------

Options considered
------------------
We have a couple options to consider:

1. A command line option (`--mode`)
* Pros
* Customer can control what kind of artifacts are produced.
* There is no guessing needed by the CLI
* Cons
* Yet another option for customers to learn or know about
* Makes the customer need to think about the artifacts they need to produce (though this can be hidden
behind the AWS Toolkit in IDEs)
* Customers running in the command line need to remember what artifacts to produce or what they previously produced
2. An Environment Variable we read (`SAM_BUILD_MODE`). IDE Toolkit will set the env var when calling `sam build`
while debugging.
* Pros
* Reduces cognitive load on customers that don't care about debugging dotnet apps through command line.
* Could force customers to use AWS Toolkit by default
* Cons
* Building debug artifacts becomes a hidden feature (silent contract)
* Environment Variables tend to be more set and forget.
* Need to conditionally add Env Var instead of a more convenient flag
3. Seamless Integration: `sam build` produces debug artifacts by default and `sam package` will build
non debug artifacts by default
* Pros
* Customers should never have to think about 'when to produce debug artifacts' or forget to add a flag
* Cons
* Requires additional work on package to auto build.
* Breaks what is produced from `sam build` as build is positioned to produce artifacts that are ready for deployment

Proposal
--------

My recommendation is to follow Option #2 from above, mainly because:

- Seamless Integration (#3) is best experience but is large in scope.
- CLI Option (#1) exposes a flag for what we think will be a rarely used CLI feature. We think so because manually
setting up debugging is cumbersome. Given that .NET developers generally prefer to be within the IDE, I find it hard
to believe someone will want to go out of the way of setting it up through CLI.
- Not a one-way-door. We can always add a CLI option to pair with env var later.

`.samrc` Changes
----------------

*Explain the new configuration entries, if any, you want to add to
.samrc*

Security
--------

*Tip: How does this change impact security? Answer the following
questions to help answer this question better:*

**What new dependencies (libraries/cli) does this change require?**
No

**What other Docker container images are you using?**
N/A

**Are you creating a new HTTP endpoint? If so explain how it will be
created & used**
No

**Are you connecting to a remote API? If so explain how is this
connection secured**
No

**Are you reading/writing to a temporary folder? If so, what is this
used for and when do you clean up?**
No

**How do you validate new .samrc configuration?**
N/A

Documentation Changes
---------------------

Open Issues
-----------

Task Breakdown
--------------

- \[x\] Send a Pull Request with this design document
- \[ \] Build the command line interface
- \[ \] Build the underlying library
- \[ \] Unit tests
- \[ \] Functional Tests
- \[ \] Integration tests
- \[ \] Run all tests on Windows
- \[ \] Update documentation
4 changes: 2 additions & 2 deletions designs/sam_build_cmd.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ Success criteria for the change
8. Integrate build action with `sam local/package/deploy` commands so
the Lambda functions will be automatically built as part of the
command without explicitly running the build command.
9. Support for building the app for debugging locally with debug
symbols (ex: Golang) [see design doc](build_debug_artifacts.md)

Out-of-Scope
------------
Expand All @@ -76,8 +78,6 @@ Out-of-Scope
package manager (ex: images, css etc)
3. Support to exclude certain files from the built artifact (ex: using
.gitignore or using regex)
4. Support for building the app for debugging locally with debug
symbols (ex: Golang)
5. Support caching dependencies & re-installing them only when the
dependency manifest changes (ex: by maintaining hash of
package.json)
Expand Down
2 changes: 1 addition & 1 deletion requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ dateparser~=0.7
python-dateutil~=2.6
pathlib2~=2.3.2; python_version<"3.4"
requests==2.20.1
aws_lambda_builders==0.2.1
serverlessrepo==0.1.8
aws_lambda_builders==0.3.0
2 changes: 1 addition & 1 deletion samcli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
SAM CLI version
"""

__version__ = '0.14.3.dev1'
__version__ = '0.15.0'
25 changes: 13 additions & 12 deletions samcli/commands/build/build_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ def __init__(self,
template_file,
base_dir,
build_dir,
mode,
manifest_path=None,
clean=False,
use_container=False,
Expand All @@ -42,6 +43,7 @@ def __init__(self,
self._parameter_overrides = parameter_overrides
self._docker_network = docker_network
self._skip_pull_image = skip_pull_image
self._mode = mode

self._function_provider = None
self._template_dict = None
Expand Down Expand Up @@ -73,21 +75,16 @@ def __exit__(self, *args):

@staticmethod
def _setup_build_dir(build_dir, clean):
build_path = pathlib.Path(build_dir)

# Get absolute path
build_dir = str(pathlib.Path(build_dir).resolve())

if not pathlib.Path(build_dir).exists():
# Build directory does not exist. Create the directory and all intermediate paths
os.makedirs(build_dir, BuildContext._BUILD_DIR_PERMISSIONS)

if os.listdir(build_dir) and clean:
# Build folder contains something inside. Clear everything.
if build_path.exists() and os.listdir(build_dir) and clean:
# build folder contains something inside. Clear everything.
shutil.rmtree(build_dir)
# this would have cleared the parent folder as well. So recreate it.
os.mkdir(build_dir, BuildContext._BUILD_DIR_PERMISSIONS)

return build_dir
build_path.mkdir(mode=BuildContext._BUILD_DIR_PERMISSIONS, parents=True, exist_ok=True)

# ensure path resolving is done after creation: https://bugs.python.org/issue32434
return str(build_path.resolve())

@property
def container_manager(self):
Expand Down Expand Up @@ -127,3 +124,7 @@ def manifest_path_override(self):
return os.path.abspath(self._manifest_path)

return None

@property
def mode(self):
return self._mode
29 changes: 24 additions & 5 deletions samcli/commands/build/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,14 @@ def cli(ctx,
manifest,
docker_network,
skip_pull_image,
parameter_overrides):
parameter_overrides,
):
# All logic must be implemented in the ``do_cli`` method. This helps with easy unit testing

mode = _get_mode_value_from_envvar("SAM_BUILD_MODE", choices=["debug"])

do_cli(template, base_dir, build_dir, True, use_container, manifest, docker_network,
skip_pull_image, parameter_overrides) # pragma: no cover
skip_pull_image, parameter_overrides, mode) # pragma: no cover


def do_cli(template, # pylint: disable=too-many-locals
Expand All @@ -105,7 +108,8 @@ def do_cli(template, # pylint: disable=too-many-locals
manifest_path,
docker_network,
skip_pull_image,
parameter_overrides):
parameter_overrides,
mode):
"""
Implementation of the ``cli`` method
"""
Expand All @@ -123,13 +127,15 @@ def do_cli(template, # pylint: disable=too-many-locals
use_container=use_container,
parameter_overrides=parameter_overrides,
docker_network=docker_network,
skip_pull_image=skip_pull_image) as ctx:
skip_pull_image=skip_pull_image,
mode=mode) as ctx:

builder = ApplicationBuilder(ctx.function_provider,
ctx.build_dir,
ctx.base_dir,
manifest_path_override=ctx.manifest_path_override,
container_manager=ctx.container_manager
container_manager=ctx.container_manager,
mode=ctx.mode
)
try:
artifacts = builder.build()
Expand Down Expand Up @@ -178,3 +184,16 @@ def gen_success_msg(artifacts_dir, output_template_path, is_default_build_dir):
template=output_template_path)

return msg


def _get_mode_value_from_envvar(name, choices):

mode = os.environ.get(name, None)
if not mode:
return None

if mode not in choices:
raise click.UsageError("Invalid value for 'mode': invalid choice: {}. (choose from {})"
.format(mode, choices))

return mode
13 changes: 10 additions & 3 deletions samcli/lib/build/app_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ def __init__(self,
base_dir,
manifest_path_override=None,
container_manager=None,
parallel=False):
parallel=False,
mode=None):
"""
Initialize the class
Expand All @@ -74,6 +75,9 @@ def __init__(self,
parallel : bool
Optional. Set to True to build each function in parallel to improve performance
mode : str
Optional, name of the build mode to use ex: 'debug'
"""
self._function_provider = function_provider
self._build_dir = build_dir
Expand All @@ -82,6 +86,7 @@ def __init__(self,

self._container_manager = container_manager
self._parallel = parallel
self._mode = mode

def build(self):
"""
Expand Down Expand Up @@ -211,7 +216,8 @@ def _build_function_in_process(self,
scratch_dir,
manifest_path,
runtime=runtime,
executable_search_paths=config.executable_search_paths)
executable_search_paths=config.executable_search_paths,
mode=self._mode)
except LambdaBuilderError as ex:
raise BuildError(str(ex))

Expand Down Expand Up @@ -245,7 +251,8 @@ def _build_function_on_container(self, # pylint: disable=too-many-locals
log_level=log_level,
optimizations=None,
options=None,
executable_search_paths=config.executable_search_paths)
executable_search_paths=config.executable_search_paths,
mode=self._mode)

try:
try:
Expand Down
5 changes: 3 additions & 2 deletions samcli/lib/build/workflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,9 @@ def _key(c):
"successfully.",
}

if _key(config) in unsupported:
return False, unsupported[config]
thiskey = _key(config)
if thiskey in unsupported:
return False, unsupported[thiskey]

return True, None

Expand Down
12 changes: 8 additions & 4 deletions samcli/local/docker/lambda_build_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ def __init__(self, # pylint: disable=too-many-locals
optimizations=None,
options=None,
executable_search_paths=None,
log_level=None):
log_level=None,
mode=None):

abs_manifest_path = pathlib.Path(manifest_path).resolve()
manifest_file_name = abs_manifest_path.name
Expand Down Expand Up @@ -67,7 +68,8 @@ def __init__(self, # pylint: disable=too-many-locals
runtime,
optimizations,
options,
executable_search_paths)
executable_search_paths,
mode)

image = LambdaBuildContainer._get_image(runtime)
entry = LambdaBuildContainer._get_entrypoint(request_json)
Expand Down Expand Up @@ -111,7 +113,8 @@ def _make_request(protocol_version,
runtime,
optimizations,
options,
executable_search_paths):
executable_search_paths,
mode):

return json.dumps({
"jsonschema": "2.0",
Expand All @@ -134,7 +137,8 @@ def _make_request(protocol_version,
"runtime": runtime,
"optimizations": optimizations,
"options": options,
"executable_search_paths": executable_search_paths
"executable_search_paths": executable_search_paths,
"mode": mode
}
})

Expand Down
Loading

0 comments on commit 846dfc3

Please sign in to comment.