Attempt 2 at docker run script#207
Conversation
* Add docker run script * Break out environment setup into a common script This allows running the same set of logic for the script runs and makefile runs. It also allows non makefile users to source this and continue to run `go test ./...` or whatever they want, without duplicated logic. * Fix lint * clean up (cherry picked from commit dfe763e)
|
To try out this PR, pull down istio/istio#21997 ^ also tests it passes in CI |
For OS X
|
Noted TMPDIR was needed on MacOS and @howardjohn fixed that in the prior commit. LGTM |
| $(shell mkdir -p out) | ||
| $(shell $(PWD)/common/scripts/setup_env.sh envfile > out/.env) | ||
| include out/.env | ||
| export out/.env |
There was a problem hiding this comment.
Is this valid syntax? Nothing is exported. Should be:
- export out/.env
+ exportThere was a problem hiding this comment.
Good catch, I think this worked when I looked since istio/istio redundantly sets all of these. Testing locally what you suggested makes this work, Ill send a PR in a minute (testing it in https://github.com/istio/istio/pull/22018/files#diff-b67911656ef5d18c4ae36cb6741b7965)
There was a problem hiding this comment.
I've commented this odd difference between bash and Makefile so people don't change it around without understanding the implications. See: #211
| $(info Building with your local toolchain.) | ||
| # If we are not in build container, we need a workaround to get environment properly set | ||
| # Write to file, then include | ||
| $(shell mkdir -p out) |
There was a problem hiding this comment.
This fails check-clean-repo target. Can we change this to mktemp or add out/ to all .gitignore files?
There was a problem hiding this comment.
All repos are putting output in out/ so we should add it to .gitignore everywhere. I sent it to a couple repos but must have missed some
There was a problem hiding this comment.
An alternative is to use a common .gitignore; something like: #210 could simplify to distribution across the various istio/* repos.
WIP until we can verify on osx