Bug 2242022 - Review Request: swiftlint - Tool to enforce Swift style and conventions
Summary: Review Request: swiftlint - Tool to enforce Swift style and conventions
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Neal Gompa
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-10-03 22:41 UTC by Davide Cavalca
Modified: 2023-11-03 18:40 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-10-17 18:46:10 UTC
Type: ---
Embargoed:
ngompa13: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6493717 to 6520906 (903 bytes, patch)
2023-10-12 20:36 UTC, Fedora Review Service
no flags Details | Diff

Description Davide Cavalca 2023-10-03 22:41:28 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint-0.53.0-1.fc40.src.rpm

Description:
SwiftLint is a tool to enforce Swift style and conventions, loosely based on
the now archived GitHub Swift Style Guide. SwiftLint enforces the style guide
rules that are generally accepted by the Swift community. These rules are well
described in popular style guides like Kodeco's Swift Style Guide.

SwiftLint hooks into Clang and SourceKit to use the AST representation of your
source files for more accurate results.

Fedora Account System Username: dcavalca

Comment 1 Davide Cavalca 2023-10-03 22:42:30 UTC
This is a preliminary WIP I'm putting up for feedback, it needs more work for sure before it can be approved (see the comments in the spec for details).

Comment 2 Neal Gompa 2023-10-03 23:24:05 UTC
Taking this review.

Comment 3 Davide Cavalca 2023-10-04 01:56:49 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint-0.53.0-1.fc40.src.rpm

Changelog:
- fix %doc usage
- do a release build by default

Comment 4 Davide Cavalca 2023-10-04 16:49:44 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint-0.53.0-1.fc40.src.rpm

Changelog:
- rework bundled deps handling to autogenerate provides as well
- patch fallback library path to avoid segfault
- enable tests by default

Comment 5 Ron Olson 2023-10-04 16:53:00 UTC
Thanks for working on this; I've been wanting this tool available on Fedora since Swift became available. :)

Comment 6 Davide Cavalca 2023-10-04 17:09:41 UTC
There's three (likely all related issues remaining):
- the build isn't using the right cflags/ldflags and I can't find a sane way to pass them
- the debuginfo that's generated seems to be in dwarf 5 format and debugedit can't cope with it
- the resulting binary has some unnecessary rpath we're currently removing manually

Comment 7 Neal Gompa 2023-10-04 17:26:51 UTC
debugedit is supposed to be able to cope with DWARF 5: https://src.fedoraproject.org/rpms/rpm/c/652537b1a5ac5c0e5b1974a4b31346db18a19d59

Comment 8 Davide Cavalca 2023-10-04 17:42:38 UTC
I'm trying this to get the right flags passed

%prep
echo -n 'swift build -v -c %{config} %{?_smp_build_ncpus:-j %{_smp_build_ncpus}} ' > build.sh
for flag in %build_cflags; do
  echo -n "-Xcc ${flag} " >> build.sh
done
for flag in %build_cxxflags; do
  echo -n "-Xcxx ${flag} " >> build.sh
done
for flag in %build_ldflags; do
  echo -n "-Xlinker ${flag} " >> build.sh
done

%build
sh -x build.sh

and it does "work" but the build ultimately fails with

/usr/libexec/swift/5.8.1/bin/swiftc -g -L /builddir/build/BUILD/SwiftLint-0.53.0/.build/aarch64-unknown-linux-gnu/release -o /builddir/build/BUILD/SwiftLint-0.53.0/.build/aarch64-unknown-linux-gnu/release/swiftlint -module-name swiftlint -emit-executable -Xlinker --gc-sections -Xlinker --defsym -Xlinker main=swiftlint_main -Xlinker '-rpath=$ORIGIN' @/builddir/build/BUILD/SwiftLint-0.53.0/.build/aarch64-unknown-linux-gnu/release/swiftlint.product/Objects.LinkFileList -target aarch64-unknown-linux-gnu -Xlinker -Wl,-z,relro -Xlinker -Wl,--as-needed -Xlinker -Wl,-z,now -Xlinker -flto=thin -Xlinker -Wl,--build-id=sha1 -Xcc -O2 -Xcc -flto=thin -Xcc -fexceptions -Xcc -g -Xcc -grecord-gcc-switches -Xcc -pipe -Xcc -Wall -Xcc -Werror=format-security -Xcc -Werror=implicit-function-declaration -Xcc -Werror=implicit-int -Xcc -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Xcc -Wp,-D_GLIBCXX_ASSERTIONS -Xcc --config -Xcc /usr/lib/rpm/redhat/redhat-hardened-clang.cfg -Xcc -fstack-protector-strong -Xcc -mbranch-protection=standard -Xcc -fasynchronous-unwind-tables -Xcc -fno-omit-frame-pointer -Xcc -mno-omit-leaf-frame-pointer -v -L /usr/libexec/swift/5.8.1/lib
Swift version 5.8.1 (swift-5.8.1-RELEASE)
Target: aarch64-unknown-linux-gnu
/usr/libexec/swift/5.8.1/bin/swift-autolink-extract @/tmp/TemporaryDirectory.1jtY4M/arguments-4662626656583931728.resp
error: autolink-extract command failed with exit code 1 (use -v to see invocation)
<unknown>:0: error: error opening input file '/builddir/build/BUILD/SwiftLint-0.53.0/.build/aarch64-unknown-linux-gnu/release/CYaml.build/src/api.c.o' (The file was not recognized as a valid object file
)
error: Bad exit status from /var/tmp/rpm-tmp.55d6Iz (%build)
    Bad exit status from /var/tmp/rpm-tmp.55d6Iz (%build)

Comment 9 Mark Wielaard 2023-10-04 17:46:12 UTC
(In reply to Neal Gompa from comment #7)
> debugedit is supposed to be able to cope with DWARF 5:
> https://src.fedoraproject.org/rpms/rpm/c/
> 652537b1a5ac5c0e5b1974a4b31346db18a19d59

For GCC generated DWARF5, llvm generates something debugedit cannot handle yet:
https://sourceware.org/bugzilla/show_bug.cgi?id=28728
Workaround is to use -gdwarf-4 till this is fixed.

Comment 10 Davide Cavalca 2023-10-04 21:00:19 UTC
For the record, the "right" way to pass the linker flags is

for flag in %(echo %build_ldflags | tr ',' ' '); do
  # These are passed directly to the linker, not to the compiler
  if [ "$flag" != '-Wl' ]; then
    echo -n "-Xlinker ${flag} " >> build.sh
  fi
done

but this still doesn't build successfully

Comment 11 Davide Cavalca 2023-10-05 17:40:56 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/swiftlint/swiftlint-0.53.0-1.fc40.src.rpm

Changelog:
- pass build id ldflags and force DRAWF4
- enable debug package generation
- enable parallel build

Comment 12 Davide Cavalca 2023-10-05 17:41:22 UTC
With the latest update I think this is ready for review.

Comment 13 Neal Gompa 2023-10-12 17:47:22 UTC
Spec review:

> Source:         %{forgeurl}/archive/%{version}/%{srcname}-%{version}.tar.gz
> # Build cache of the bundled dependencies so we can build offline
> Source:         %{srcname}-%{version}-bundled-deps.tar.gz
> Source:         %{srcname}-%{version}-bundled-provides.txt
> Source:         swiftlint-get-bundled-deps.sh

As you use the numbered Source variables in the spec, please use numbered Sources too.

Comment 15 Fedora Review Service 2023-10-12 20:36:06 UTC
Created attachment 1993655 [details]
The .spec file difference from Copr build 6493717 to 6520906

Comment 16 Neal Gompa 2023-10-17 17:35:11 UTC
> # Swift only supports these arches
> ExclusiveArch:  x86_64 aarch64

Future note: when designing macros for packaging swift stuff, don't forget to macroize this.

Comment 17 Neal Gompa 2023-10-17 17:45:51 UTC
Review notes:

* Package follows Fedora Packaging Guidelines (well, as well as it can given the lack of Swift guidelines so far)
* Package builds and installs
* Package licensing is correctly handled
* No serious issues from rpmlint (modulo weirdness caused by... well this package)

PACKAGE APPROVED.

Comment 18 Fedora Admin user for bugzilla script actions 2023-10-17 17:47:35 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/swiftlint

Comment 19 Fedora Update System 2023-10-17 18:45:02 UTC
FEDORA-2023-5b1548897b has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2023-5b1548897b

Comment 20 Fedora Update System 2023-10-17 18:46:10 UTC
FEDORA-2023-5b1548897b has been pushed to the Fedora 40 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 21 Fedora Update System 2023-10-17 21:38:48 UTC
FEDORA-2023-db294cb0c6 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-db294cb0c6

Comment 22 Fedora Update System 2023-10-17 22:12:14 UTC
FEDORA-2023-9db2651150 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-9db2651150

Comment 23 Fedora Update System 2023-10-18 01:23:52 UTC
FEDORA-2023-9db2651150 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-9db2651150 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-9db2651150

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 24 Fedora Update System 2023-10-18 02:35:26 UTC
FEDORA-2023-db294cb0c6 has been pushed to the Fedora 39 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-db294cb0c6 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-db294cb0c6

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 25 Fedora Update System 2023-10-18 21:24:34 UTC
FEDORA-EPEL-2023-21db6471b3 has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-21db6471b3

Comment 26 Fedora Update System 2023-10-19 02:38:18 UTC
FEDORA-EPEL-2023-21db6471b3 has been pushed to the Fedora EPEL 9 testing repository.

You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-21db6471b3

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 27 Fedora Update System 2023-10-24 01:02:38 UTC
FEDORA-EPEL-2023-21db6471b3 has been pushed to the Fedora EPEL 9 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 28 Fedora Update System 2023-10-24 01:21:58 UTC
FEDORA-2023-9db2651150 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 29 Fedora Update System 2023-10-24 02:24:00 UTC
FEDORA-2023-db294cb0c6 has been pushed to the Fedora 39 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-db294cb0c6 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-db294cb0c6

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 30 Fedora Update System 2023-11-03 18:40:17 UTC
FEDORA-2023-db294cb0c6 has been pushed to the Fedora 39 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.