Fix Ignore Patterns. Add .git to Ruby workflow.

[metaskills]

Ok, so this is really weird. I have been using the lambic/lambda containers to run sam build inside it vs using sam build --use-container from outside. I am using the Ruby 2.5 workflow with my open source Lamby (https://github.com/customink/lamby) project.

So... I found out when doing my sam builds in a container that I ran into a recursive .aws-sam/build/RailsFunction/.aws-sam/build/RailsFunction/... issue. After a TON of digging I found that ignored_names here https://github.com/awslabs/aws-lambda-builders/blob/develop/aws_lambda_builders/utils.py#L45 was an empty set even tho I could print the source and names and see an .aws-sam directory there.

source
/var/task

names
['Dockerfile-build', 'template.yaml', '.DS_Store', 'app', '.ruby-version', 'test', 'bin', 'config', 'Dockerfile', 'config.ru', 'node_modules', '.bundle', 'app.rb', '.env.test', 'README.md', 'Rakefile', 'public', '.gitignore', 'package.json', '.env', '.github', 'lib', 'Gemfile', 'log', 'Gemfile.lock', 'docker-compose.yml', '.git', '.aws-sam', '.env.development', 'tmp', 'vendor']

ignore(source, names)
set()

I found there were two ways to work around this issue. Neither I know not why and have included both changes. My guess is the reverse splat (is that the python name too) is the core issue and was only an issue for Ruby beaus we only had one item in our set. So adding .git fixes it too... and that in itself is a good addition. But wanted to include both in this PR.

Also note that Python is not my native language and I am hacking here. Feedback appreciated.

[metaskills]

So it appears some tests fail would you rather I keep the splat on ignore=shutil.ignore_patterns(*self.excludes)) and just add .git to the ruby bundler builder and just call it a day?

[metaskills]

OK, I reverted the ignore_patterns back with the "splat". A friend tells me that (“Hi Ken”) is an expression where (“Hi Ken”,) is a tuple. So adding .git which should be there anyway does two things but still fixes the core issue with Ruby Bundler workflow.

[awood45]

Do you have an example of a failing test, even a failing integration test, that would illustrate what is broken and the fix working? This looks fine on the surface, just want to have something we can use to avoid regressions.

[metaskills]

Good question. I suspect I can copy this test from the java maven workflow (https://github.com/awslabs/aws-lambda-builders/blob/develop/tests/unit/workflows/java_maven/test_workflow.py#L46-L53) and use it in the Ruby/Bundler one. Cause right now there is no test for the .aws-sam dir or any excluded files. Let me try real quick and see the test fail and the fix again.

[metaskills]

Hmmm... those tests seem a bit tautological and not an integration test. Just tests that a constant is set. What do you think?

[awood45]

Part of that test should also be that the build action completed successfully. Then you can check for some example files that you'd know would be generated.

Ruby has a few "example apps" that turn into these types of tests - for example: https://github.com/awslabs/aws-lambda-builders/tree/develop/tests/integration/workflows/ruby_bundler/testdata/with-deps

Can we add an example app that breaks the build, and show this change fixing said build?

[metaskills]

Can we add an example app that breaks the build, and show this change fixing said build?

I do not really have the Python skills for that. Are there any tests like that already that I can copy that you can point me too? At quick glance, I did not see any integration tests that checked for excluded files. So my questions are.

1. Can you point me to those integrations?
2. If there are none, can I copy/paste the one from another lang to Ruby?

[awood45]

I think I can meet you halfway if you can provide the Ruby application which breaks the build, and I can create the test. Put it in a folder like the one I linked. Test format is here but I'm happy to write it: https://github.com/awslabs/aws-lambda-builders/blob/develop/tests/integration/workflows/ruby_bundler/test_ruby.py

[metaskills]

Test format is here but I'm happy to write it:

Thanks so much for that offer! I had some free time tonight and got this done. The latest commit adds an "excluded-files" test app which is really just a copy of "no-deps" app with two directories added, a .aws-sam and a .git. Both have a .keep file so they are committable.

From here it was easy to test since the format of the previous test was grok'able by me even tho I do not work in Python. I even verified (locally) that if the change submitted to this pull request was not done, the submitted test would fail with the following error.

E       AssertionError: Items in the second set but not the first:
E       '.git'
E       '.aws-sam'

Again, I'd like to point out why. A co-worker showed me via this site (https://repl.it/languages/python3) that the code prior was never working because it was a string, not a tuple.

> type((".aws-sam"))
<class 'str'>
> type((".aws-sam",))
<class 'tuple'>
> type((".aws-sam", ".git"))
<class 'tuple'>

[awood45]

Thanks for this! As a bonus, I think a couple of other builders have this same issue, so I'll apply this to those as well.