gitlab-ci-multi-runner v1.0.4 #49562
Conversation
|
|
||
| cd "src/gitlab.com/gitlab-org/gitlab-ci-multi-runner" do | ||
| commit_sha = `git rev-parse --short HEAD` | ||
|
|
||
| # Disable vender support for go 1.5 and above |
There was a problem hiding this comment.
You could just pass:
ENV["GO15VENDOREXPERIMENT"] = "0"...
There was a problem hiding this comment.
ENV["GO15VENDOREXPERIMENT"] = "0" does not work for incoming go 1.7. I'm ok for setting ENV, your call.
Go 1.5 introduced experimental support for vendoring, enabled by setting the
GO15VENDOREXPERIMENTenvironment variable to 1. Go 1.6 keeps the vendoring support, no longer considered experimental, and enables it by default. It can be disabled explicitly by setting theGO15VENDOREXPERIMENTenvironment variable to 0. Go 1.7 will remove support for the environment variable.
There was a problem hiding this comment.
Calling Go 1.7 "incoming" is a bit of a stretch; we've only just had 1.6 and there's usually 6 months between major Go releases. We'll clear the 1.7 hurdle when we get there, let's use the ENV for now.
|
Updated according reviews |
|
👍 |
|
Just a little concern. "make executors/docker/bindata.go" behave differently with and without docker present. This formula will only build successfully when user doesn't have docker, since it will just download prebuilt docker images. But with docker installed, then "gox" will be needed to build helper binary for linux/amd64. and docker build will need either another ENV to run the real build on linux (virtual) machine, or dlite already running. I have no real solution here, but could it be made to force using prebuilt images no matter what? edit: scratch that, that was my observation from my manual building. obviously i don't know homebrew enough to understand why this formula succeeded building with docker :) |
We hide |
| # Copy from Makefile | ||
| system "go", "build", "-o", "gitlab-ci-multi-runner", "-ldflags", "-X main.NAME=gitlab-ci-multi-runner -X main.VERSION=#{version} -X main.REVISION=#{commit_sha}" | ||
| system "make", "executors/docker/bindata.go" |
There was a problem hiding this comment.
Is the https://gitlab-ci-multi-runner-downloads.s3.amazonaws.com/master/docker/prebuilt.tar.gz download it fetches here static until the next release? If so, can we vendor it into the correct directory rather than relying on the make process to fetch it for us?
There was a problem hiding this comment.
Can you check upstream? If it isn't going to move until the next release we can vendor and checksum it rather than permitting undeclared arbitrary downloads.
|
@MikeMcQuaid: Merge pliss. |
|
Updated against feedback, now using |
| url "https://gitlab-ci-multi-runner-downloads.s3.amazonaws.com/v1.0.4/docker/prebuilt.tar.gz", | ||
| :using => :nounzip | ||
| sha256 "43dedd023672990e27289e97bf74f493576956418148f6227917b8511e8aadfd" | ||
| end |
There was a problem hiding this comment.
What is this? Is it a binary?
There was a problem hiding this comment.
Yeah, it's a prebuilt version dependency.
There was a problem hiding this comment.
Of what? We need to build from source.
There was a problem hiding this comment.
It looks like docker executor stuff: https://github.com/Homebrew/homebrew/pull/49562/files#diff-71716e632d36db33cd37440d8a50c847R48
There was a problem hiding this comment.
omg, I don't even use docker, why do I have to download it?
There was a problem hiding this comment.
So if I don't use docker, I don't need prebuilt images? Is it possible to define resource optional?
if build.with? "docker"
# note, no "with" in the option name (it is added by the build.with? method)
end
There was a problem hiding this comment.
@ReDetection You need prebuilt images to build gitlab-ci-multi-runner, no matter how you use it, as the docker feature is not optional for gitlab-ci-multi-runner binary. This is about how you build gitlab-ci-multi-runner
What you can decide is whether using docker executor after installing gitlab-ci-multi-runner, to use docker executor you need docker installed. And this is about how you use gitlab-ci-multi-runner
There was a problem hiding this comment.
@cybertk even if I embed empty file?
There was a problem hiding this comment.
I'm not sure, but I think the build should be failed if you embed undesired assets
There was a problem hiding this comment.
It's now fine as-is: I just had questions about this.
|
Added |
|
Thanks for your contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock! For future reference the preferred commit message format for simple version updates is |
Closes Homebrew/legacy-homebrew#49562. Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
|
Thanks, this also close cybertk/homebrew-gitlab-runner#4 |
No description provided.