Spec URL: https://github.com/derekparker/delve-fedpkg/blob/v1.1.0-1/delve.spec SRPM URL: https://github.com/derekparker/delve-fedpkg/releases/download/v1.1.0-1/delve-1.1.0-1.fc29.src.rpm Description: Package for the Delve debugger for the Go programming language. Fedora Account System Username: deparker
Spec URL: https://raw.githubusercontent.com/derekparker/delve-fedpkg/v1.1.0-1/delve.spec SRPM URL: https://github.com/derekparker/delve-fedpkg/releases/download/v1.1.0-1/delve-1.1.0-1.fc29.src.rpm Description: Package for the Del
The BuildRoot tag has been deprecated, so you should remove that. Also it looks like the spec file you posted is newer than the spec file in the srpm. Can you rebuild the srpm with the latest spec file.
The spec file doesn't contain the bundled provides for all the vendored stuff https://fedoraproject.org/wiki/Packaging:Guidelines#Bundling_and_Duplication_of_system_libraries. Have you considered de-bundling?
For the record we have issue for packaging delve in the Go-SIG https://pagure.io/GoSIG/go-sig/issue/9 .
Could this be unbundled and use the new Go packaging?
It looks like that: ============================ # Run tests in check section %bcond_without check # https://github.com/derekparker/delve %global goipath github.com/derekparker/delve Version: 1.1.0 %global common_description %{expand: Delve is a debugger for the Go programming language. The goal of the project is to provide a simple, full featured debugging tool for Go. Delve should be easy to invoke and easy to use. Chances are if you're using a debugger, things aren't going your way. With that in mind, Delve should stay out of your way as much as possible.} %gometa Name: delve Release: 1%{?dist} Summary: A debugger for the Go programming language # Detected licences # - Expat License at 'LICENSE' License: MIT URL: %{gourl} Source0: %{gosource} BuildRequires: golang(github.com/cosiner/argv) BuildRequires: golang(github.com/mattn/go-colorable) BuildRequires: golang(github.com/mattn/go-isatty) BuildRequires: golang(github.com/peterh/liner) BuildRequires: golang(github.com/pkg/profile) BuildRequires: golang(github.com/sirupsen/logrus) BuildRequires: golang(github.com/spf13/cobra) BuildRequires: golang(golang.org/x/arch/x86/x86asm) BuildRequires: golang(golang.org/x/sys/unix) BuildRequires: golang(golang.org/x/sys/windows) BuildRequires: golang(gopkg.in/yaml.v2) %description %{common_description} %package -n %{goname}-devel Summary: %{summary} BuildArch: noarch %description -n %{goname}-devel %{common_description} This package contains library source intended for building other packages which use import path with %{goipath} prefix. %prep %forgesetup rm -rf vendor/ %build %gobuildroot %gobuild -o _bin/dlv %{goipath}/cmd/dlv %install %goinstall install -Dpm 0755 _bin/dlv %{buildroot}%{_bindir}/dlv %if %{with check} %check %gochecks %endif %files %license LICENSE %doc CONTRIBUTING.md CHANGELOG.md %doc Documentation/* %{_bindir}/dlv %files -n %{goname}-devel -f devel.file-list %license LICENSE %changelog * Tue Oct 30 2018 Derek Parker <deparker> - 1.1.0-1 - First package for Fedora ================================== The only missing package is golang.org/x/arch, the other might need to be updated.
BuildRequires: golang(github.com/mattn/go-colorable) is to be removed as it is Windows only.
I've updated all the dependencies and added a Review Request for golang.org/x/arch.
Thanks Robert-Andre! I'll take the updated spec you've provided and use that as well. Thanks for that, by the way.
Is there anyone reviewing the above request for `golang.org/x/arch`? Once that is in and unblocks this package I'd love to move forward.
I don't think I have permissions to sponsor Derek, if another commenter is able to sponsor and wants take over this review, that is fine with me.
I can sponsor Derek, but I'm not familiar with Go packaging. So if someone trustworthy could do the actual review, I'll do the sponsorship then.
Looks like the other packaged was reviewed: https://bugzilla.redhat.com/show_bug.cgi?id=1645681. I'll work on this more today, and update everything.
The dependency (x/arch) requires a dep to be packaged (rsc.io/pdf). I'll work on packaging that.
All dependencies have been packaged now, however because of the default setup for Go packaging (symlinking to `_build`) tests are failing due to not evaluating the symlinks in tests where the debug information generated in the binary actually _does_ eval symlinks. There will be a fix for this in the next release which should be out this week or early next week, so I'll just wait for that to finish up this package.
I've updated the package with some patches that enable the Delve test suite to run with all of the symlinkage. These patches will be removed on the next update once we release Delve 1.2.0. SPEC: https://raw.githubusercontent.com/derekparker/delve-fedpkg/v1.1.0-1/delve.spec SRPM: https://github.com/derekparker/delve-fedpkg/releases/download/v1.1.0-1/delve-1.1.0-1.fc28.src.rpm
You can change %forgesetup to %autoforgesetup -p1 and then drop the %patch lines. With all dependencies built, you should be able to run a koji scratch build now, I think.
The change to %autoforgesetup -p1 didn't work for me for some reason, got the error about job control signifying that macro isn't set. In any event, can I go ahead and request a repo for this and begin that process? Do I need a repo to scratch build?
Sorry, that was a typo; it should be %forgeautosetup. You don't need a repo to scratch build, but you can't request a repo until this review is approved.
BTW, can you also patch the "help" command so that when it says "See $GOPATH/src/github.com/derekparker/delve/Documentation/cli/locspec.md for the syntax of linespec." it will point to /usr/share/doc/delve instead?
Updated. SPEC: https://raw.githubusercontent.com/derekparker/delve-fedpkg/master/delve.spec SRPM: https://github.com/derekparker/delve-fedpkg/releases/download/v1.1.0-1/delve-1.1.0-1.fc28.src.rpm
Unfortunately, this does not build on Rawhide. Also, Tom has assigned himself to this review, so don't wait for me to approve it.
I ran a scratch build and it seems to only be failing because it's doing something weird with the remote and trying to pull from some combination of src.fedoraproject and my actual repo: https://kojipkgs.fedoraproject.org//work/tasks/9130/32579130/checkout.log. Tom if you could take a look and approve or provide feedback I can request and repo and get real builds up.
Have you tried running this via mock? I don't think you can do scratch builds without a repo. I did a mock build, and it looks like one of the test cases is failing: Error running dwz on /tmp/dwzcompression.98164ef8: exit status 1 dwz: /tmp/dwzcompression.98164ef8: .debug_info section not present exit status 1 FAIL github.com/derekparker/delve/pkg/proc 73.371s
Ah, thanks I'll look into that.
SRPM: https://github.com/derekparker/delve-fedpkg/releases/download/v1.2.0-1/delve-1.2.0-1.fc29.src.rpm SPEC: https://raw.githubusercontent.com/derekparker/delve-fedpkg/2dc549c73ba03c3877d2ec2adb635b051800aec8/delve.spec However, during `fedpkg mockbuild` I get the following error which seems related to the tooling, not my package directly: ``` Executing(%check): /bin/sh -e /var/tmp/rpm-tmp.cqOHC4 + umask 022 + cd /builddir/build/BUILD + cd delve-1.2.0 + PATH=/builddir/build/BUILD/delve-1.2.0/_bin:/builddir/.local/bin:/builddir/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin + GO_TEST_FLAGS='-buildmode pie -compiler gc' + GO_TEST_EXT_LD_FLAGS='-Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld ' + go-rpm-integration check -i github.com/go-delve/delve -p /builddir/build/BUILDROOT/delve-1.2.0-1.fc30.x86_64 -g /usr/share/gocode -b /builddir/build/BUILD/delve-1.2.0/_build -r '.*example.*' Testing: github.com/go-delve/delve panic: runtime error: invalid memory address or nil pointer dereference [recovered] panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5b522a] goroutine 1 [running]: github.com/gofed/symbols-extractor/vendor/github.com/urfave/cli.HandleAction.func1(0xc0000d9938) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/vendor/github.com/urfave/cli/app.go:472 +0x27b panic(0x97c000, 0xe60220) /usr/lib/golang/src/runtime/panic.go:522 +0x1b5 github.com/gofed/symbols-extractor/pkg/util/internal/load.ImportPaths(0xc0001dd100, 0x1, 0x1, 0x0, 0xc0002729e0, 0xb) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/pkg/util/internal/load/pkg.go:1888 +0x5a github.com/gofed/symbols-extractor/pkg/util/internal/load.PackagesAndErrors(0xc0001dd100, 0x1, 0x1, 0x6, 0xf, 0x8) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/pkg/util/internal/load/pkg.go:1839 +0xa4 github.com/gofed/symbols-extractor/pkg/util.ListPackage(0xc0000323ed, 0x19, 0x19, 0x0, 0xc0000d9248) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/pkg/util/util.go:420 +0x8f github.com/gofed/symbols-extractor/pkg/util.(*PackageInfoCollector).CollectPackageInfos.func1(0xc0000323c0, 0x47, 0xad77c0, 0xc0001971e0, 0x0, 0x0, 0x0, 0xc0001971e0) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/pkg/util/util.go:181 +0x6dd path/filepath.walk(0xc0000323c0, 0x47, 0xad77c0, 0xc0001971e0, 0xc000260060, 0x0, 0x2) /usr/lib/golang/src/path/filepath/path.go:367 +0xe3 path/filepath.Walk(0xc0000323c0, 0x47, 0xc000260060, 0xa064e6, 0x1) /usr/lib/golang/src/path/filepath/path.go:409 +0xff github.com/gofed/symbols-extractor/pkg/util.(*PackageInfoCollector).CollectPackageInfos(0xc0001cbe00, 0x7ffc0c3c79a3, 0x19, 0x7ffc0c3c79a3, 0x19) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/pkg/util/util.go:139 +0x158 main.main.func1(0xc0002448c0, 0x0, 0x0) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/cmd/golist/golist.go:73 +0x610 reflect.Value.call(0x95fc20, 0xa2cbc0, 0x13, 0xa06d83, 0x4, 0xc0000d98d8, 0x1, 0x1, 0xc000066240, 0xa0a2a4, ...) /usr/lib/golang/src/reflect/value.go:447 +0x461 reflect.Value.Call(0x95fc20, 0xa2cbc0, 0x13, 0xc0000d98d8, 0x1, 0x1, 0xa064d8, 0x1, 0xa0a2a4) /usr/lib/golang/src/reflect/value.go:308 +0xa4 github.com/gofed/symbols-extractor/vendor/github.com/urfave/cli.HandleAction(0x95fc20, 0xa2cbc0, 0xc0002448c0, 0x0, 0x0) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/vendor/github.com/urfave/cli/app.go:481 +0x1ff github.com/gofed/symbols-extractor/vendor/github.com/urfave/cli.(*App).Run(0xc0000c6480, 0xc000020070, 0x7, 0x7, 0x0, 0x0) /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/vendor/github.com/urfave/cli/app.go:240 +0x509 main.main() /builddir/build/BUILD/symbols-extractor-9f4330a0f4437ca61ba92f9f30e34424c6742ad6/src/github.com/gofed/symbols-extractor/cmd/golist/golist.go:125 +0x791 + exit 0 ```
This is a known issue in golist / the symbol extractor: https://github.com/gofed/symbols-extractor/issues/157 https://pagure.io/golist/issue/7
You should be able to pick up a fixed golist from here: https://pagure.io/golist/pull-request/17, build it and --copyin to the mock chroot before building. Though once you have that, it appears Go modules are being a pain and trying to download stuff to build _fixtures/locationsprog.go (even though the dlv binary built fine with whatever was installed.) Working around that with GO111MODULE=off, I still get: Error running dwz on /tmp/dwzcompression.47560ce6: exit status 1 dwz: /tmp/dwzcompression.47560ce6: .debug_info section not present
Is this because DWARF is compressed by default on newer Go? <mock-chroot> sh-4.4# go build -gcflags='-l -N' -o ./dwzcompression ../BUILD/delve-1.2.0/_fixtures/dwzcompression.go <mock-chroot> sh-4.4# objdump -h dwzcompression | grep -A 1 debug_info 35 .zdebug_info 0005168f 0000000000000000 0000000000000000 000dcfb4 2**0 CONTENTS, READONLY, DEBUGGING <mock-chroot> sh-4.4# dwz dwzcompression dwz: dwzcompression: .debug_info section not present <mock-chroot> sh-4.4# go build -gcflags='-l -N' -ldflags=-compressdwarf=false -o ./dwzcompression ../BUILD/delve-1.2.0/_fixtures/dwzcompression.go <mock-chroot> sh-4.4# objdump -h dwzcompression | grep -A 1 debug_info 35 .debug_info 0005168f 0000000000000000 0000000000000000 000cac16 2**0 CONTENTS, READONLY, DEBUGGING <mock-chroot> sh-4.4# dwz dwzcompression Note the 'z': .zdebug_info vs .debug_info
Yeah, that's correct regarding DWZ compression. I've been trying a few other workarounds so let me try picking up the new golist.
SPEC: https://raw.githubusercontent.com/derekparker/delve-fedpkg/v1.2.0-1/delve.spec SRPM: https://github.com/derekparker/delve-fedpkg/releases/download/v1.2.0-1/delve-1.2.0-1.fc29.src.rpm Updated with a couple patches to get all the tests running correctly in the build env. Please review.
I've gone through the review checklist and this looks good to me, but you should take a look at the rpmlint warnings before you push this package. For example, there is an empty md file that is installed and also it claims the patches aren't applied. Might be some opportunities for clean ups there. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [ ]: Package does not contain kernel modules. [ ]: Package contains no static executables. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [ ]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [ ]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "MIT/X11 (BSD like)", "Unknown or generated". 283 files have unknown license. Detailed output of licensecheck in /home/tstellar/1645294-delve/licensecheck.txt [ ]: License file installed when any subpackage combination is installed. [ ]: Package must own all directories that it creates. Note: Directories without known owners: /usr/share/gocode/src, /usr/share/gocode [ ]: Package does not own files or directories owned by other packages. [ ]: %build honors applicable compiler flags or justifies otherwise. [ ]: Package contains no bundled libraries without FPC exception. [ ]: Changelog in prescribed format. [ ]: Sources contain only permissible code or content. [ ]: Package contains desktop file if it is a GUI application. [ ]: Development files must be in a -devel package [ ]: Package uses nothing in %doc for runtime. [ ]: Package consistently uses macros (instead of hard-coded directory names). [ ]: Package is named according to the Package Naming Guidelines. [ ]: Package does not generate any conflict. [ ]: Package obeys FHS, except libexecdir and /usr/target. [ ]: If the package is a rename of another package, proper Obsoletes and Provides are present. [ ]: Requires correct, justified where necessary. [ ]: Spec file is legible and written in American English. [ ]: Package contains systemd file(s) if in need. [ ]: Useful -debuginfo package or justification otherwise. [ ]: Package is not known to require an ExcludeArch tag. [ ]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 112640 bytes in 30 files. [ ]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [ ]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [ ]: Final provides and requires are sane (see attachments). [ ]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in golang- github-delve-devel , delve-debuginfo , delve-debugsource [ ]: Package functions as described. [ ]: Latest version is packaged. [ ]: Package does not include license text files separate from upstream. [ ]: Patches link to upstream bugs/comments/lists or are otherwise justified. [ ]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [ ]: %check is present and all tests pass. [ ]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: delve-1.2.0-1.fc30.x86_64.rpm golang-github-delve-devel-1.2.0-1.fc30.noarch.rpm delve-debuginfo-1.2.0-1.fc30.x86_64.rpm delve-debugsource-1.2.0-1.fc30.x86_64.rpm delve-1.2.0-1.fc30.src.rpm delve.x86_64: E: zero-length /usr/share/doc/delve/usage/commands.md delve.x86_64: W: no-manual-page-for-binary dlv golang-github-delve-devel.noarch: W: hidden-file-or-dir /usr/share/gocode/src/github.com/go-delve/delve/.goipath delve.src: W: patch-not-applied Patch1: ./disable-default-compression-dwz-test.patch delve.src: W: patch-not-applied Patch2: ./integration-test-symlinks.patch 5 packages and 0 specfiles checked; 1 errors, 4 warnings. Rpmlint (debuginfo) ------------------- Checking: delve-debuginfo-1.2.0-1.fc30.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output:
I've sent a PR to fix the empty commands.md file: https://github.com/go-delve/delve/pull/1492 I can include the patch in this spec file to remove it. Also, the patches _are_ getting applied, it's just happening automagically through the autoforgesetup and not through explicit patch commands.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/delve
I see an ExclusiveArch there; you need to file bug reports for those: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_architecture_build_failures
Alright I'll file those, but it doesn't seem to be working. Should I instead use ExcludeArch for each individual one?
I think that's because %gometa sets ExclusiveArch since Go is not available everywhere, and you get the union of them. Probably ExcludeArch is the only way to fix (aside from getting delve to work on the other arches.)
Got it, thanks for the input. It looks like `golist` is broken during my rawhide scratch-build. Should I just roll my own `%check` section at this point? I had that implemented before during testing but switched it back as I assumed this would be the better and more approved way of doing this, but it just seems to be broken.
I have a build completed for f30. It seems f29 doesn't have the dependency I had to build previously (rsc-pdf). When I try and build for f31 I get an error during the build stage: https://kojipkgs.fedoraproject.org//work/tasks/4503/33074503/build.log. The only thing I can find in that log output is BUILDSTDERR: collect2: fatal error: cannot find 'ld'.
(In reply to Derek Parker from comment #39) > The only thing I can find in that log output is BUILDSTDERR: collect2: fatal > error: cannot find 'ld'. This is a known issue in rawhide right now: https://bugzilla.redhat.com/show_bug.cgi?id=1683408
(In reply to Derek Parker from comment #39) > It seems f29 doesn't have the dependency I > had to build previously (rsc-pdf). You need to push your golang-rsc-pdf updates to stable: https://bodhi.fedoraproject.org/updates/?search=golang-rsc-pdf Or add build overrides (but might as well push them now.)
Now create updates for delve on 29/28?
I have Delve built on 28/29.
delve-1.2.0-1.fc29 has been submitted as an update to Fedora 29. https://bodhi.fedoraproject.org/updates/FEDORA-2019-5c4f0a0da7
delve-1.2.0-1.fc28 has been submitted as an update to Fedora 28. https://bodhi.fedoraproject.org/updates/FEDORA-2019-a516e07305
delve-1.2.0-1.fc28 has been pushed to the Fedora 28 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-a516e07305
delve-1.2.0-1.fc29 has been pushed to the Fedora 29 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-5c4f0a0da7
delve-1.2.0-1.fc28 has been pushed to the Fedora 28 stable repository. If problems still persist, please make note of it in this bug report.
delve-1.2.0-1.fc29 has been pushed to the Fedora 29 stable repository. If problems still persist, please make note of it in this bug report.