containers / skopeo

Work with remote images registries - retrieving information, images, signing content
Apache License 2.0
7.78k stars 757 forks source link

Ubuntu Trusty (shippable.com) issue: make binary-local BUILDTAGS=containers_image_ostree_stub -> go-md2man: not found #495

Closed taqtiqa-mark closed 6 years ago

taqtiqa-mark commented 6 years ago

The front page suggests that make all builds the doc which requires go-md2man. However it seems make binary-local BUILDTAGS=containers_image_ostree_stub also tries to build the docs.

I don't think it should?

Ubuntu Trusty on Shippable.com:

make binary-local BUILDTAGS=containers_image_ostree_stub
make install

Produces:

go build "-buildmode=pie" -ldflags "-X main.gitCommit=e626fca6a7676cb0fc095f7d8d645add01bfa6b9" -gcflags "" -tags "containers_image_ostree_stub" -o skopeo ./cmd/skopeo
install -d -m 755 /usr/bin
install -m 755 skopeo /usr/bin/skopeo
go-md2man -in docs/skopeo.1.md -out docs/skopeo.1.tmp && touch docs/skopeo.1.tmp && mv docs/skopeo.1.tmp docs/skopeo.1
/bin/sh: 1: go-md2man: not found
Makefile:87: recipe for target 'docs/skopeo.1' failed
make: *** [docs/skopeo.1] Error 127
rhatdan commented 6 years ago

make install is attempting to install the man pages. make install-binary might be what you want?

taqtiqa-mark commented 6 years ago

My mistake. Apologies.

frioux commented 6 years ago

This is still a bug right? Shouldn't make install be able to install manpages?

rhatdan commented 6 years ago

Yes the problem here is that he did not have the go-md2man executable installed.

frioux commented 6 years ago

The Readme doesn't say it's required. Why not have the build container do that?

-- Sent from a rotary phone rented from Ma Bell

On Mon, Apr 30, 2018, 4:36 AM Daniel J Walsh notifications@github.com wrote:

Yes the problem here is that he did not have the go-md2man executable installed.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/projectatomic/skopeo/issues/495#issuecomment-385373692, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAf4072wBDvQxMBDCGbl4zD8BTfFOLiks5ttvdXgaJpZM4TQuhU .

rhatdan commented 6 years ago

I don't think @taqtiqa-mark used a build container. He was just building from source. But I will let him chime in.

frioux commented 6 years ago

I got the same issue doing make install which at least partially used a build container.

-- Sent from a rotary phone rented from Ma Bell

On Mon, Apr 30, 2018, 7:57 AM Daniel J Walsh notifications@github.com wrote:

I don't think @taqtiqa-mark https://github.com/taqtiqa-mark used a build container. He was just building from source. But I will let him chime in.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/projectatomic/skopeo/issues/495#issuecomment-385424529, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAf44riKdo4UCIzemkrzC_xyzqh9ETRks5ttyZ2gaJpZM4TQuhU .

mtrmac commented 6 years ago

The Readme doesn't say it's required. Why not have the build container do that?

README does say that it’s required for building documentation. The installation section is confusing, though.

Is https://github.com/projectatomic/skopeo/pull/504 better? Any other improvements are welcome.

Why not have the build container do that?

Most of the development work and packaging does not use (and in the case of building packages, can not easily use) build containers, and I’m afraid surprises like this are a consequence.

I do agree that the current default, where make uses a container for compilation, but not for everything, is problematic. (In fact the whole idea of using a container by default is fairly dubious when the container environment leaks through dynamic library dependencies in the resulting executable.)

The pattern rule currently used for building man pages does not know whether the user is using a build container or not; so actually fixing this would require a fair amount of restructuring the Makefile. At that point it starts to look a bit attractive to just drop the build container support entirely.