Bug: Windows deploys broken because CRLF not converted to LF for bash/shell scripts

[jtuliani]

Description:

I use Windows. I found that when pulling down the https://github.com/awslabs/aws-lambda-web-adapter repo, the LF characters are changed to CRLF (which is default git behaviour on Windows). This caused a problem with .sh files, since SAM build/deploy does not change CRLF back to LF, and the CRLF causes .sh scripts to fail. The same issue also occurs for bootstrap files.

This impacts all Windows users unless they take extra steps to convert CRLF to LF. It adds friction to the getting started experience, and is hard to diagnose/fix for Windows users who are not familiar with Linux.

Instead, this should 'just work', without Windows users having to think about it or do anything. I.e. SAM should convert the CRLF to LF as required.

See awslabs/aws-lambda-web-adapter#369 for more info. That PR is a workaround for that repo only; the better fix is for SAM to convert CRLF to LF on Windows, fixing the issue for all repos.

Steps to reproduce:

1. Use a Windows client
2. Sync https://github.com/awslabs/aws-lambda-web-adapter/ to your local machine
3. Navigate to the ./examples/nodejs-zip folder
4. Run sam build --use-container followed by sam deploy --guided
5. Open the deployed application endpoint in your browser

Observed result:

Build and deploy appear to work fine.
When you open the application endpoint in your browser, an 'Internal server error' is shown.
Examining the logs, you see the error /var/task/run.sh: /bin/bash^M: bad interpreter: No such file or directory.
As per https://stackoverflow.com/questions/14219092/bash-script-bin-bashm-bad-interpreter-no-such-file-or-directory this error is caused by CRLF rather than LF in the run.sh script.

Expected result:

The example should work (and it does work, if you convert the CRLF to LF using a hex editor before you build/deploy).

Additional environment details (Ex: Windows, Mac, Amazon Linux etc)

Windows 10

{
  "version": "1.108.0",
  "system": {
    "python": "3.11.7",
    "os": "Windows-10-10.0.19044-SP0"
  },
  "additional_dependencies": {
    "docker_engine": "20.10.8",
    "aws_cdk": "Not available",
    "terraform": "Not available"
  },
  "available_beta_feature_env_vars": [
    "SAM_CLI_BETA_FEATURES",
    "SAM_CLI_BETA_BUILD_PERFORMANCE",
    "SAM_CLI_BETA_TERRAFORM_SUPPORT",
    "SAM_CLI_BETA_RUST_CARGO_LAMBDA"
  ]
}

[mndeveci]

Thanks for creating this issue @jtuliani,

Your example is very specific to using a bash script which requires line endings as LF. There might be some other examples (e.g. dotnet projects) which requires (or works) with CRLF. So from SAM CLI's perspective we don't have enough information to take such action on behalf of customer. Taking such action might also break customer's working application if their application requires to be CRLF line endings.

For that reason, customers can run git config --global core.autocrlf false one time for not auto converting LF line endings to CRLF.

We can add this information either in troubleshooting or pre-requsite section of SAM CLI documentation. I will be checking with our documentation team about updating our pages.

[normj]

Would it be possible for SAM to look for CRLF in *.sh and bootstrap files and if found any emit a warning to users. That would at least give users a quicker indication that these invisible characters might cause them problems. If there is a legitimate reason they can ignore but I suspect legitimate reasons for those files are rare.

[jtuliani]

I like the idea of a warning as a last resort, but I would still prefer to see the CRLF fixed. I think we're over-indexing on preserving the existing experience, but why? That experience is broken. Let's fix it instead. We have Windows version of SAM, it should work in an idiomatic way for Windows users. If we're making them think about CRLF vs LF, that's not idiomatic.

The git config workaround doesn't really work. There's a good reason why converting LF to CRLF is the default git behavior on Windows, and we shouldn't require developers to change it just for the benefit of SAM. And what if they created the script rather than downloading from git? In that case the workaround is useless, and the experience is still broken.