-
Notifications
You must be signed in to change notification settings - Fork 35
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
Update replace existing chrome validation #116
Conversation
…download the same version installed
@marboledacci Your change here has unfortunately broken our entire Circle CI pipeline. We use this script in our Dockerfile for circle ci:
This worked fine for us right up until this PR was merged. Here is the failing output of our docker build:
Why is this failing? Since we don't lave
Fails with Unfortunately your script now fails in that case for installing Google chrome this way. Potential solution We could change line 5 to handle circleci not being installed?:
I've opened a PR #118 and I've tested this with our environment and it works ✅ Please let me know what you think |
@willzoltan This is strange, in theory CircleCI is the agent that runs the jobs, so if it was able to run a job you should have that. This usually doesn't need to be manually installed. I'm gonna check this internally to see what is happening, or if the behavior is different when using custom docker images. |
@willzoltan are you using a self hosted runner? |
@marboledacci Thanks for the quick reply! We're not using self hosted runners, but we do build our own docker images for the pipeline outside of Circle CI. But it should be possible to build the images independent of the circleci agent. The other scripts in |
@willzoltan Could you show me the output of the first step of the job, called |
@marboledacci Here is is. But we build the As a temporary work around, we are using cached docker images of
|
Sorry, I was not understanding your problem before. I see that your issue is not related to CircleCI execution.
Keep in mind that even if we merge your PR, it could break again in the future because of the same thing for different parameters or different requirements. The solution you provide is to solve a problem that is outside the scope of an Orb. I'm guessing you are doing this because of the suggestion here so we are going to look at the caching for this orb. |
Thank you for looking into this. We have other reasons why we need to build a docker image ourselves. And we are using this script entirely within the circle ci ecosystem. Our image is based off the circle ci image I can see that the feature to processed chrome ver with env subst was added in the last couple of weeks. None of the other scripts rely on It seems like a very reasonable expectation that a bash script should work with the environment variables it depends upon, without necessarily requiring them to come from At the very least, why not just default to "latest" when |
The usage of the variable |
Resolves #57
Update the conditions to remove an existing installation, so it only happens if the version installed is different to the required version.