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

dev-go/act: new package #19411

Closed
wants to merge 1 commit into from
Closed

dev-go/act: new package #19411

wants to merge 1 commit into from

Conversation

waebbl
Copy link
Contributor

@waebbl waebbl commented Feb 11, 2021

Closes: https://bugs.gentoo.org/770043
Package-Manager: Portage-3.0.14, Repoman-3.0.2
Signed-off-by: Bernd Waibel waebbl-gentoo@posteo.net

Package is needed to bump media-libs/lib3mf-2.1.0. This package has pre-built binary blobs available for x86_64, win64 and darwin, but not for x86_32 and arm, hence the keywords had to be dropped temporarily. This package should be able to re-enable those keywords.

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @waebbl
Areas affected: ebuilds
Packages affected: dev-go/act

dev-go/act: @gentoo/proxy-maint (new package)

Linked bugs

Bugs linked: 770043

New packages

This Pull Request appears to be introducing new packages only. Due to limited manpower, adding new packages is considered low priority. This does not mean that your Pull Request will not receive any attention, however, it might take quite some time for it to be reviewed. In the meantime, your new ebuild might find a home in the GURU project repository: the ebuild repository maintained collaboratively by Gentoo users. GURU offers your ebuild a place to be reviewed and improved by other Gentoo users, while making it easy for Gentoo users to install it and enjoy the software it adds.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. labels Feb 11, 2021
@waebbl
Copy link
Contributor Author

waebbl commented Feb 11, 2021

The package does not use the standard go layout, hence the functions from golang-build.eclass can IMO not be used.
I'm having not much experience in building go packages. If the eclass functions can actually be used, and I'm missing something, I'd be happy for some pointers to fix the ebuild.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-02-11 07:30 UTC
Newest commit scanned: 7f2e9e2
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/fe199842ec/output.html

dev-go/act/act-1.6.0.ebuild Outdated Show resolved Hide resolved
Comment on lines 23 to 65
go_arch() {
# By chance most portage arch names match Go
local portage_arch=$(tc-arch $@)
case "${portage_arch}" in
x86) echo 386;;
x64-*) echo amd64;;
ppc64) [[ $(tc-endian $@) = big ]] && echo ppc64 || echo ppc64le ;;
s390) echo s390x ;;
*) echo "${portage_arch}";;
esac
}

go_arm() {
case "${1:-${CHOST}}" in
armv5*) echo 5;;
armv6*) echo 6;;
armv7*) echo 7;;
*)
die "unknown GOARM for ${1:-${CHOST}}"
;;
esac
}

go_os() {
case "${1:-${CHOST}}" in
*-linux*) echo linux;;
*-darwin*) echo darwin;;
*-freebsd*) echo freebsd;;
*-netbsd*) echo netbsd;;
*-openbsd*) echo openbsd;;
*-solaris*) echo solaris;;
*-cygwin*|*-interix*|*-winnt*)
echo windows
;;
*)
die "unknown GOOS for ${1:-${CHOST}}"
;;
esac
}

go-tuple() {
echo "$(go_os $@)_$(go_arch $@)"
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to care about all this? I see lib3mf is only keyworded for amd64 arm64 x86.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about what we can remove here. As far as I understand this, go needs a tuple set before being called. We can probably cleanup the go_os (all non-linux) and go_arch (ppc and s390) until lib3mf gets keyworded for any of those.
We could also simplify it by hardcoding linux for go-os in go-tuple

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about the last changes?

dev-go/act/act-1.6.0.ebuild Outdated Show resolved Hide resolved
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-03-14 11:30 UTC
Newest commit scanned: 144dd05
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/fa557a918e/output.html

Closes: https://bugs.gentoo.org/770043
Package-Manager: Portage-3.0.14, Repoman-3.0.2
Signed-off-by: Bernd Waibel <waebbl-gentoo@posteo.net>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-03-16 21:59 UTC
Newest commit scanned: 0152c7e
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/4baf11dff8/output.html

Copy link
Member

@juippis juippis left a comment

Choose a reason for hiding this comment

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

Just a status update, I tested the previous commit and it worked fine. Give me until tomorrow to think about this, but so far LGTM, even though I believe the arm checks in here are irrelevant for arm64.

@waebbl
Copy link
Contributor Author

waebbl commented Mar 17, 2021

Sadly, I don't have a arm machine to test this. I don't actually know exactly how go is working, found those functions in the dev-lang/go ebuild and thought the env vars are needed to properly run go. I can be wrong on this. Yet, those env vars control the way go is building packages in some way, see go env and go help environment.

Comment on lines +31 to +47
go_arm() {
case "${1:-${CHOST}}" in
armv5*) echo 5;;
armv6*) echo 6;;
armv7*) echo 7;;
*)
die "unknown GOARM for ${1:-${CHOST}}"
;;
esac
}

src_compile() {
export GOARCH=$(go_arch)
export GOOS=linux
if [[ ${ARCH} == arm ]]; then
export GOARM=$(go_arm)
fi
Copy link
Member

Choose a reason for hiding this comment

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

@thesamesam what do you think? Are these relevant for arm64? I don't think they are. AFAIK this doesn't need to be keyworded with arm at all, just arm64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK this doesn't need to be keyworded with arm at all, just arm64.

That's correct, lib3mf-1 has only support for arm64.

Copy link
Member

Choose a reason for hiding this comment

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

This'll do for now, but please speak to William about possibly including this in golang-base. It's fine to keep this in because there's no sense in gatekeeping future support.

@thesamesam thesamesam self-requested a review March 19, 2021 08:26
SLOT="0"
KEYWORDS="~amd64 ~x86"

RESTRICT="strip test"
Copy link
Member

Choose a reason for hiding this comment

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

No tests? :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly no. There's an Examples/UnitTest directory but it looks like it's not updated for this version.

@thesamesam
Copy link
Member

thesamesam commented Mar 20, 2021

Thanks! Added bb5b84a for a verbose build.

@waebbl waebbl deleted the act branch March 20, 2021 09:02
@waebbl
Copy link
Contributor Author

waebbl commented Mar 20, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). bug linked Bug/Closes found in footer, and cross-linked with the PR. new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
5 participants