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

symlinked /mockery to /usr/local/bin #279

Merged
merged 8 commits into from
Jun 7, 2020

Conversation

mkatychev
Copy link
Contributor

A lot of the code I maintain depends on using go generate to create our mockery code with this pattern: //go:generate mockery -name=MockedInterface

  • Adding the symlink to /go/bin ensures that go generate will work regardless whether mockery is called from a local installation or the docker container.

@mkatychev mkatychev changed the title symlinked root mockery to /go/bin symlinked /mockery to /usr/local/bin Jun 2, 2020
@blaggacao
Copy link
Contributor

blaggacao commented Jun 2, 2020

Curiosity: Why would you call go generate from within vektor/mockery?
Or: I don't understand the use case quite right.

@mkatychev
Copy link
Contributor Author

mkatychev commented Jun 2, 2020

@blaggacao mockery -all will generate mocks for all interfaces. I require more fine-grained control as to what objects are mocked. This gives me an explicit indicator as to what interfaces are currently being used in testing.

@blaggacao
Copy link
Contributor

Got that, thanks, bu I lack behind on how RUN ln -s /mockery /usr/local/bin solves this?

@mkatychev
Copy link
Contributor Author

@blaggacao

It allows go generate to keep using //go:generate mockery -name=MockedInterface for user level installs with brew or whatnot AND from the docker image.

Otherwise I would have to give go:generate an absolute path to work from a docker container:

> //go:generate /mockery -name=MockedInterface
< //go:generate mockery -name=MockedInterface

This breaks usability for MacOS systems, having /mockery in system root is not feasible in darwin.

@blaggacao
Copy link
Contributor

blaggacao commented Jun 2, 2020

I think the part I don't understand is "AND from the docker image."

How would //go:generate mockery -name=MockedInterface be executed within the vektra/mockery?

Or rephrased, how do you execute go generate from within vektor/mockery?

I think the (somewhat) intended use is alias mockery="docker run -v $(pwd):/src -w /src --user=$(id -u):$(id -g) vektra/mockery".

Is that what you rather try to do? Or am I simply not seeing something?

@mkatychev
Copy link
Contributor Author

mkatychev commented Jun 2, 2020

This allows go generate to run on arbitary paths:

PATH_ARGS=${PATH_ARGS:-./...}
docker run --rm -v "$PWD:/src" -w "/src" \
  --entrypoint go vektra/mockery:v2 generate $PATH_ARGS

I overrode the /mockery entry point to be go and pass it the generate $PATH_ARGS arguments. It works just fine.

@blaggacao
Copy link
Contributor

Ok, got it, fair enough, one can do that. But I'd argue some go generate commands out there might assume some kind of an environment which vektra/mockery might or might not provide, with an alias, one is on the safe side.

@mkatychev
Copy link
Contributor Author

mkatychev commented Jun 2, 2020

Then I'd argue in moving the binary to a place where it is commonly found/able to be symlinked on unix systems such as usr/local/bin rather than system root. I'm afraid that would break backwards compatibility, however 😓 .

might assume some kind of an environment

I think that responsibility is on the implementer of said go generate code to curtail (me in this case). It would be my responsibility to deal with wrong assumptions from using system $PATH.

@blaggacao
Copy link
Contributor

blaggacao commented Jun 2, 2020

Then I'd argue in moving the binary to a place where it is commonly symlinked on unix systems such as usr/local/bin rather than system root

If that's currently the case, it seems like an addressable ux bug to me...

Maybe there is an option to provide appropriate symlinks?

Dockerfile Outdated Show resolved Hide resolved
@mkatychev
Copy link
Contributor Author

If that's currently the case, it seems like an addressable ux bug to me...

If you're thinking of adding symlinks to macOS root, sadly that's not feasible at the moment :(:

https://github.com/NixOS/nix/pull/3628/files

nix package manager ran into the same issue with adding /nix to system root after Catalina.

@blaggacao
Copy link
Contributor

blaggacao commented Jun 2, 2020

Oh, now we nailed it down! Cool! The proposed change of this PR than LGTM

Btw there is another issue when using docker #280

@blaggacao
Copy link
Contributor

If you're thinking of adding symlinks to macOS root, sadly that's not feasible at the moment :(:

Alternatively, it might serve you better if brew wouldn't install mockery to /mockery to root, would it?

@mkatychev
Copy link
Contributor Author

mkatychev commented Jun 2, 2020

Btw there is another issue when using docker #280

I can wait to merge those changes then.

Alternatively, it might serve you better if brew wouldn't install mockery to /mockery to root, would it?

I'm not sure if I understand the statement. On macOS, brew cannot do so even if it tried.

@LandonTClipp
Copy link
Collaborator

@mkatychev can you confirm #280 is sufficient for you? This runs a go install and then also copies to root. This should do what you need correct?

@mkatychev
Copy link
Contributor Author

mkatychev commented Jun 3, 2020

@LandonTClipp Just built it locally and confirmed that the behaviour remained the same for the final docker step. mockery is only in system path during the builder step. COPY --from=builder /go/bin/mockery / produces the same issue as before.

@LandonTClipp
Copy link
Collaborator

@mkatychev could you rebase off master and squash your commits. Once that's done I'll merge.

@LandonTClipp
Copy link
Collaborator

Never mind I'm merging this now.

@LandonTClipp LandonTClipp merged commit 1cc3837 into vektra:master Jun 7, 2020
@mkatychev
Copy link
Contributor Author

Hi @LandonTClipp do you know when the next tagged docker image will be realeased? Current :latest shows /mockery as the entrypoint.

@LandonTClipp
Copy link
Collaborator

LandonTClipp commented Jun 12, 2020

@mkatychev I tried to tag it last weekend but was having issues with Travis, I will try again this weekend. Sorry for the delay, typically very busy on weekdays.

@mkatychev
Copy link
Contributor Author

@LandonTClipp Thank you for the update. Please take your time, I appreciate the time you put into this project.

@LandonTClipp
Copy link
Collaborator

@mkatychev the docker image failed to build: https://travis-ci.org/github/vektra/mockery/jobs/697811596

Status: Downloaded newer image for golang:1.14-alpine
 ---> 3289bf11c284
Step 2/12 : COPY ./ /mockery
 ---> a40c8af26c87
Step 3/12 : RUN cd /mockery && go install ./...
 ---> Running in 01a68acd76b2
go: warning: "./..." matched no packages
Removing intermediate container 01a68acd76b2
 ---> e3e3b09ddcae
Step 4/12 : FROM golang:1.14-alpine
1.14-alpine: Pulling from library/golang
Digest: sha256:6042b9cfb4eb303f3bdcbfeaba79b45130d170939318de85ac5b9508cb6f0f7e
Status: Image is up to date for golang:1.14-alpine
 ---> 3289bf11c284
Step 5/12 : COPY --from=builder /go/bin/mockery /usr/local/bin
COPY failed: stat /var/lib/docker/overlay2/15094578edc58eaeda3555d0eb8a93b5fbf9be1d5e05d95f5f9f5b0baaefb299/merged/go/bin/mockery: no such file or directory
: exit status 1

@mkatychev
Copy link
Contributor Author

Strange, I will take a look at this as soon as I can.

@LandonTClipp
Copy link
Collaborator

I think this was an error introduced in #278, might have nothing to do with this PR. I'll take a look this weekend as well, I think it's just something simple.

@LandonTClipp
Copy link
Collaborator

I fixed the deployment, it's all published now.

@mkatychev
Copy link
Contributor Author

mkatychev commented Jun 15, 2020

@LandonTClipp Just a heads up, latest docker image still has the bin in root:
docker run --rm -it --entrypoint ls vektra/mockery:latest /

@LandonTClipp
Copy link
Collaborator

Fixed it, sorry the tagging got messed up.

$ docker run --rm -it --entrypoint ls vektra/mockery:latest /usr/local/bin
mockery
$ docker run --rm -it --entrypoint ls vektra/mockery:latest /
bin    etc    home   media  opt    root   sbin   sys    usr
dev    go     lib    mnt    proc   run    srv    tmp    var

@mkatychev
Copy link
Contributor Author

Fantastic, it works great now!

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

Successfully merging this pull request may close these issues.

3 participants