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

Switch all references to github.com/containers/libpod -> podman #6909

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

rhatdan
Copy link
Member

@rhatdan rhatdan commented Jul 8, 2020

Signed-off-by: Daniel J Walsh [email protected]

@mheon
Copy link
Member

mheon commented Jul 8, 2020

Is this going to break go modules? @vrothberg

@rhatdan
Copy link
Member Author

rhatdan commented Jul 8, 2020

@mheon It took a while to get this to compile. I had to rename some stuff in homedir. Mainly move
~/go/src/github.com/containers/libpod -> ~/go/src/github.com/containers/podman

@vrothberg
Copy link
Member

Is this going to break go modules? @vrothberg

The module has been renamed as well, so we're good.

module github.com/containers/podman/v2

BUT it will break @lsm5 's blog post. Once merged, @lsm5 you may need to change the references in your bindings blog post. While the redirect works, the go packages are now renamed and don't resolve anymore.

@vrothberg
Copy link
Member

Needs a rebase

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 9, 2020
@vrothberg
Copy link
Member

I just realized that this change could be a rather big disturbance in the force. As we disable go modules for compilation, we need to follow $GOPATH and as we are renaming, all existing already checked out clones won't compile anymore - only after renaming the directory it'll work.

@vrothberg
Copy link
Member

Luckily there's go modules. Let me change the Makefile a bit to make it work.

@vrothberg
Copy link
Member

vrothberg commented Jul 10, 2020

The following patch will make it compile again.

diff --git a/Makefile b/Makefile
index 0c19e9f65e8f..d88ad1908ecd 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,3 @@
-export GO111MODULE=off
 export GOPROXY=https://proxy.golang.org
 
 GO ?= go
@@ -38,11 +37,7 @@ PRE_COMMIT = $(shell command -v bin/venv/bin/pre-commit ~/.local/bin/pre-commit
 
 SOURCES = $(shell find . -path './.*' -prune -o -name "*.go")
 
-GO_BUILD ?= $(GO) build
-# Go module support: set `-mod=vendor` to use the vendored sources
-ifeq ($(shell go help mod >/dev/null 2>&1 && echo true), true)
-	GO_BUILD ?= GO111MODULE=on $(GO) build -mod=vendor
-endif
+GO_BUILD ?= $(GO) build -mod=vendor
 
 BUILDTAGS_CROSS ?= containers_image_openpgp exclude_graphdriver_btrfs exclude_graphdriver_devicemapper exclude_graphdriver_overlay
 ifneq (,$(findstring varlink,$(BUILDTAGS)))
@@ -171,11 +166,11 @@ gofmt: ## Verify the source code gofmt
 
 .PHONY: test/checkseccomp/checkseccomp
 test/checkseccomp/checkseccomp: .gopathok $(wildcard test/checkseccomp/*.go)
-	$(GO_BUILD) -ldflags '$(LDFLAGS_PODMAN)' -tags "$(BUILDTAGS)" -o $@ $(PROJECT)/test/checkseccomp
+	$(GO_BUILD) -ldflags '$(LDFLAGS_PODMAN)' -tags "$(BUILDTAGS)" -o $@ ./test/checkseccomp
 
 .PHONY: test/goecho/goechoe
 test/goecho/goecho: .gopathok $(wildcard test/goecho/*.go)
-	$(GO_BUILD) -ldflags '$(LDFLAGS_PODMAN)' -o $@ $(PROJECT)/test/goecho
+	$(GO_BUILD) -ldflags '$(LDFLAGS_PODMAN)' -o $@ ./test/goecho
 
 
 .PHONY: bin/podman
@@ -185,18 +180,18 @@ ifeq (,$(findstring systemd,$(BUILDTAGS)))
 	@echo "Podman is being compiled without the systemd build tag. Install libsystemd on \
 		Ubuntu or systemd-devel on rpm based distro for journald support."
 endif
-	$(GO_BUILD) $(BUILDFLAGS) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags "$(BUILDTAGS)" -o $@ $(PROJECT)/cmd/podman
+	$(GO_BUILD) $(BUILDFLAGS) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags "$(BUILDTAGS)" -o $@ ./cmd/podman
 
 .PHONY: podman
 podman: bin/podman
 
 .PHONY: bin/podman-remote
 bin/podman-remote: .gopathok $(SOURCES) go.mod go.sum $(PODMAN_VARLINK_DEPENDENCIES) ## Build with podman on remote environment
-	$(GO_BUILD) $(BUILDFLAGS) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags "${REMOTETAGS}" -o $@ $(PROJECT)/cmd/podman
+	$(GO_BUILD) $(BUILDFLAGS) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags "${REMOTETAGS}" -o $@ ./cmd/podman
 
 .PHONY: bin/podman-remote-static
 podman-remote-static: bin/podman-remote-static
-	CGO_ENABLED=0 $(GO_BUILD) $(BUILDFLAGS) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN_STATIC)' -tags "${REMOTETAGS}" -o bin/podman-remote-static $(PROJECT)/cmd/podman
+	CGO_ENABLED=0 $(GO_BUILD) $(BUILDFLAGS) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN_STATIC)' -tags "${REMOTETAGS}" -o bin/podman-remote-static ./cmd/podman
 
 .PHONY: podman-remote
 podman-remote: bin/podman-remote
@@ -210,7 +205,7 @@ podman.msi: podman-remote podman-remote-windows install-podman-remote-windows-do
 
 podman-remote-%: .gopathok $(PODMAN_VARLINK_DEPENDENCIES) ## Build podman for a specific GOOS
 	$(eval BINSFX := $(shell test "$*" != "windows" || echo ".exe"))
-	CGO_ENABLED=0 GOOS=$* $(GO_BUILD) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags "${REMOTETAGS}" -o bin/$@$(BINSFX) $(PROJECT)/cmd/podman
+	CGO_ENABLED=0 GOOS=$* $(GO_BUILD) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags "${REMOTETAGS}" -o bin/$@$(BINSFX) ./cmd/podman
 
 local-cross: $(CROSS_BUILD_TARGETS) ## Cross local compilation
 
@@ -218,7 +213,7 @@ bin/podman.cross.%: .gopathok
 	TARGET="$*"; \
 	GOOS="$${TARGET%%.*}" \
 	GOARCH="$${TARGET##*.}" \
-	$(GO_BUILD) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags '$(BUILDTAGS_CROSS)' -o "$@" $(PROJECT)/cmd/podman
+	$(GO_BUILD) -gcflags '$(GCFLAGS)' -asmflags '$(ASMFLAGS)' -ldflags '$(LDFLAGS_PODMAN)' -tags '$(BUILDTAGS_CROSS)' -o "$@" ./cmd/podman
 
 # Update nix/nixpkgs.json its latest master commit
 .PHONY: nixpkgs
@@ -285,11 +280,11 @@ dbuild: libpodimage
 
 .PHONY: dbuild-podman-remote
 dbuild-podman-remote: libpodimage
-	${CONTAINER_RUNTIME} run --name=${LIBPOD_INSTANCE} --privileged -v ${PWD}:/go/src/${PROJECT} --rm ${LIBPOD_IMAGE} go build -ldflags '$(LDFLAGS_PODMAN)' -tags "$(REMOTETAGS)" -o bin/podman-remote $(PROJECT)/cmd/podman
+	${CONTAINER_RUNTIME} run --name=${LIBPOD_INSTANCE} --privileged -v ${PWD}:/go/src/${PROJECT} --rm ${LIBPOD_IMAGE} $(GOBUILD) -ldflags '$(LDFLAGS_PODMAN)' -tags "$(REMOTETAGS)" -o bin/podman-remote ./cmd/podman
 
 .PHONY: dbuild-podman-remote-darwin
 dbuild-podman-remote-darwin: libpodimage
-	${CONTAINER_RUNTIME} run --name=${LIBPOD_INSTANCE} --privileged -v ${PWD}:/go/src/${PROJECT} --rm ${LIBPOD_IMAGE} env GOOS=darwin go build -ldflags '$(LDFLAGS_PODMAN)' -tags "${REMOTETAGS}" -o bin/podman-remote-darwin $(PROJECT)/cmd/podman
+	${CONTAINER_RUNTIME} run --name=${LIBPOD_INSTANCE} --privileged -v ${PWD}:/go/src/${PROJECT} --rm ${LIBPOD_IMAGE} env GOOS=darwin $(GOBUILD) -ldflags '$(LDFLAGS_PODMAN)' -tags "${REMOTETAGS}" -o bin/podman-remote-darwin ./cmd/podman
 
 .PHONY: test
 test: libpodimage ## Run tests on built image

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2020
@rhatdan
Copy link
Member Author

rhatdan commented Jul 12, 2020

@vrothberg added your patch.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2020
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 16, 2020
@rhatdan
Copy link
Member Author

rhatdan commented Jul 16, 2020

As much as I like rebasing. at some point we need to get this merged.

@vrothberg
Copy link
Member

As much as I like rebasing. at some point we need to get this merged.

We can only merge when it's green :^)

@rhatdan rhatdan force-pushed the podman branch 3 times, most recently from 65dadc6 to c309883 Compare July 17, 2020 13:39
@rhatdan
Copy link
Member Author

rhatdan commented Jul 17, 2020

Easier said then done.

@rhatdan rhatdan force-pushed the podman branch 4 times, most recently from d1677a3 to 7ad25a3 Compare July 22, 2020 14:21
@rhatdan
Copy link
Member Author

rhatdan commented Jul 23, 2020

@mheon @vrothberg @edsantiago @cevich Any ideas what is going on special test ginkgo tests are failing with permission denied?

@edsantiago
Copy link
Member

I'm going to bet this is #6822, in particular the global setting of GOPATH. I'm trying hard now to figure out how it worked before, and how to get it working again, but I think it's going to be much faster for @cevich to solve.

@edsantiago
Copy link
Member

Submitting #7064 as a possible fix. @rhatdan care to rebase on that and retry? I know how much you love rebasing.

@rhatdan
Copy link
Member Author

rhatdan commented Jul 23, 2020

@edsantiago Passes the test with your PR. Thanks
@mheon @vrothberg @giuseppe @edsantiago @ashley-cui @baude @jwhonce
BTW, once we merge this, backporting fixes to v2.0 branch is going to become a pain.

@mheon
Copy link
Member

mheon commented Jul 23, 2020

IMO, let's start cutting v2.1.0 release candidates once we have your podman image mount PR landed. We have a fair few new features (timezone support, --preserve-fds for podman run, the --userns=keepid password addition) ready.

@mheon
Copy link
Member

mheon commented Jul 23, 2020

(And then we can stop doing backports for non-critical issues to v2.0 once the v2.1 release happens)

@mheon
Copy link
Member

mheon commented Jul 23, 2020

This LGTM though

@giuseppe
Copy link
Member

needs a rebase

@edsantiago
Copy link
Member

Tests are green. I don't feel comfortable LGTM'ing this but I sure hope someone else does. How many rebases?

@vrothberg
Copy link
Member

vrothberg commented Jul 28, 2020

Note we cannot cherry-pick patches after merging the changes here any more; all references have to be changed back to "libpod" for the 2.0 branch.

LGTM but I want @mheon to ACK on the cherry-pick issue.

@mheon
Copy link
Member

mheon commented Jul 28, 2020

@vrothberg Let's do it, we need to get master switched. We'll have to start being a lot more selective about what we backport.

@mheon mheon added the lgtm Indicates that a PR is ready to be merged. label Jul 28, 2020
@openshift-merge-robot openshift-merge-robot merged commit 288ebec into containers:master Jul 28, 2020
@lsm5
Copy link
Member

lsm5 commented Jul 28, 2020

so this ain't ending up in a v2.0.x tag anytime soon, yes?

@mheon
Copy link
Member

mheon commented Jul 28, 2020

There's no chance we're backporting this, but depending on difficulty we could re-create on the 2.0 branch?

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants