Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove git dependency? #354

Open
iHiD opened this issue Jun 2, 2021 · 3 comments
Open

Remove git dependency? #354

iHiD opened this issue Jun 2, 2021 · 3 comments

Comments

@iHiD
Copy link

iHiD commented Jun 2, 2021

This line requires the git executable to be installed. Our Dockerfiles are optimised to not have unnecessary things such as git installed, which means although the .git directory is there, this workflow fails for us.

Rather than relying on a binary, it might be a nicer option to read directly from the files that git itself reads from, which (I haven't checked, but I presume) are .git/HEAD and then whatever that points to (e.g. .git/refs/heads/main for our deployed app).

Here's an example of the config I'm trying out at Exercism for this: https://github.com/exercism/website/pull/1069/files

If we changed this upstream in this repo I could remove that messiness :)

@zvkemp
Copy link
Contributor

zvkemp commented Jun 3, 2021

The content inside your ERB tag looks generally ok (though you might want to strip the last File read). The sha should be nested under a deploy key:

deploy:
  git_sha: <%= ... %>

You can also set SKYLIGHT_DEPLOY_GIT_SHA in your env.

The stderr redirection already allows for git to be missing; I'm not sure if assuming the .git directory exists is necessarily better than what we are already doing (e.g., it's commonly added to .dockerignore), but we will consider adding another Deploy fetcher for it.

@iHiD
Copy link
Author

iHiD commented Jun 3, 2021

though you might want to strip the last File read

Good point. Thanks. I'm actually setting it here now. Our security setup (very open source - no closed repos) means that we don't use environment variables and instead have everything powered internally by AWS's Secrets Manager, which is why this is a bit of a round-about way to do it in the Environment.rb file.


I'm not sure if assuming the .git directory exists is necessarily better than what we are already doing (e.g., it's commonly added to .dockerignore)

If the .git directory is added to gitignore, the existing system breaks too though? As git will fail.

Looking in the .git directory for the data, rather than relying on an executable, continues to work in the existing scenario, and also supports this extra scenario. It therefore adds functionality, removes a dependency and a system call, and has no downside that I can see, so I would argue it is better 🙂

But it's your app! 🙂

@zvkemp
Copy link
Contributor

zvkemp commented Jun 3, 2021

Valid point. We'll put it on the roadmap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants