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

Fix compilation for GOLANG=(mipsle|mips64le) #4032

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cc_platform.mk
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ else ifeq ($(GOARCH),riscv64)
HOST := riscv64-$(PLATFORM)-
else ifeq ($(GOARCH),s390x)
HOST := s390x-$(PLATFORM)-
else ifeq ($(GOARCH),mipsle)
HOST := mipsel-$(PLATFORM)-
else ifeq ($(GOARCH),mips64le)
HOST := mips64el-$(PLATFORM)abi64-
else
$(error Unsupported GOARCH $(GOARCH))
endif
Expand Down
3 changes: 2 additions & 1 deletion libcontainer/dmz/dmz_linux.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//go:build !runc_nodmz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to add this here -- if we build runc-dmz then we can use it. I will take this along with @rata's suggestion and make a PR to add MIPS binaries to our release artifacts (presumably Debian has the necessary cross-compilers).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add MIPS binaries to our release artifacts

I don't think we have to do this, MIPS is no longer official on our side.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we merge this, and discuss your concern in a separate PR?

//go:build (386 || amd64 || arm || arm64 || loong64 || ppc64le || riscv64 || s390x) && !runc_nodmz
// +build 386 amd64 arm arm64 loong64 ppc64le riscv64 s390x
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a side note, I'm not sure why we still have old-style +build tags. Those are not needed since Go 1.18 or so.

Will open a PR to remove those.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to add the arches here. IMHO, we should leave this as it was and just make all arches compile, as this PR is doing: https://github.com/opencontainers/runc/pull/4076/files

// +build !runc_nodmz

package dmz
Expand Down
4 changes: 2 additions & 2 deletions libcontainer/dmz/dmz_unsupported.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//go:build !linux || runc_nodmz
// +build !linux runc_nodmz
//go:build !linux || (!386 && !amd64 && !arm && !arm64 && !loong64 && !ppc64le && !riscv64 && !s390x) || runc_nodmz
// +build !linux !386,!amd64,!arm,!arm64,!loong64,!ppc64le,!riscv64,!s390x runc_nodmz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem, I think we don't need to add specific arches here, just leave the build tag.


package dmz

Expand Down
6 changes: 6 additions & 0 deletions script/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ function set_cross_vars() {
s390x)
HOST=s390x-${PLATFORM}
;;
mipsle)
HOST=mipsel-${PLATFORM}
;;
mips64le)
HOST=mips64el-${PLATFORM}abi64
;;
*)
echo "set_cross_vars: unsupported architecture: $1" >&2
exit 1
Expand Down