Also, I am a new packager so I will need a sponsor.  Thank you!

Successful Koji Build of swift-lang: http://koji.fedoraproject.org/koji/taskinfo?taskID=12380144

jdfergason's scratch build of swift-lang-2.2-0.1.snapshot20151231.fc23.src.rpm for epel7 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12386680

jdfergason's scratch build of swift-lang-2.2-0.1.snapshot20151231.fc23.src.rpm for epel7 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12386874

jdfergason's scratch build of swift-lang-2.2-0.1.snapshot20151231.fc23.src.rpm for f24 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12386610

jdfergason's scratch build of swift-lang-2.2-0.1.snapshot20151231.fc23.src.rpm for f24-llvm failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12386918

jdfergason's scratch build of swift-lang-2.2-0.1.snapshot20151231.fc23.src.rpm for f22 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12386556

Here's my initial feedback:
 - Use an ExclusiveArch instead of such a long list of ExcludeArch.
 - Use global instead of define ( https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define )
 - Remove the defintion for _libdir and _lib64dir (should already be defined)
 - Is that Requires list really necessary? ( https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires )
 - Would be nice to get tests working on koji. Do they work in mock? If not, then I'm guessing that the BuildRequires list is missing something ( https://fedoraproject.org/wiki/Using_Mock_to_test_package_builds )
 - Are you hoping to target EL 5? rm/mkdir of %{buildroot} is done automatically in recent versions of rpmbuild (I believe starting with EL >= 6)
 - Setting %attr only required if supporting EL <= 5
 - Use the license macro ( compatibility workaround for EPEL can be found at https://fedoraproject.org/wiki/EPEL:Packaging#The_.25license_tag )
 - Use wildcards to reduce list in %files section
 - Can the .h and other such files be moved to a -devel package?
 - Use Python macros for paths ( https://fedoraproject.org/wiki/Packaging:Python#Macros )
 - Use %{_includedir} instead of hardcoded path to /usr/include
 - Don't use hard coded path to /usr/share/doc/* (%{_datarootdir} or maybe %doc)

Please learn how to use RPM macros first, your spec is too complex. Meanwhile please rename package -docs to -doc.

I've cleaned up a lot of the comments from Dave Johansen in the spec file.  Swift doesn't like to play well in the /usr/lib64 directory ... /usr/lib is hardcoded in a number of places.  I'm looking at ways to fix this as %{_libdir} is %{prefix}/lib64 on multiline systems.

I have updated the SPEC file and SRPM:

Spec URL: https://s3-us-west-2.amazonaws.com/swift-rpm/swift-lang.spec
SRPM URL: https://s3-us-west-2.amazonaws.com/swift-rpm/swift-lang-2.2-0.1.snapshot20151231.fc23.src.rpm

The one thing that I don't know how to do is deal with the /usr/lib vs /usr/lib64 issue.  I just redefined the macro; is this the way to deal with that?

(In reply to jeremy.fergason from comment #11)
> I have updated the SPEC file and SRPM:
> 
> Spec URL: https://s3-us-west-2.amazonaws.com/swift-rpm/swift-lang.spec
> SRPM URL:
> https://s3-us-west-2.amazonaws.com/swift-rpm/swift-lang-2.2-0.1.
> snapshot20151231.fc23.src.rpm

That was a big step in the right direction, but here's some more comments:
 - Release is usually just an integer (i.e. 1 instead of 0.1) and it's usually best to increment it with each change so that it's easier for reviewers to compare different versions
 - As Meng pointed out, the Source* tags should use macros so that only a single value has to be changed when a new release is made
 - Get rid of # in Source* tags and use original names in source .rpm
 - Are versions listed in BuildRequires "real" (i.e. listed in Swift documentation and/or the true minimum version required)?
 - All patches needs to have at least an issue opened upstream and documented in the .spec file what the expected resolution is ( https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment )
 - I don't believe the "cd .." is necessary at the end of %prep
 - Add version to changelog entries ( https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs )

> The one thing that I don't know how to do is deal with the /usr/lib vs
> /usr/lib64 issue.  I just redefined the macro; is this the way to deal with
> that?

Is this actually building Swift or just unpacking a pre-built binary?

If it's just a pre-built binary, then this package cannot be packaged for Fedora ( https://fedoraproject.org/wiki/Packaging:Guidelines#No_inclusion_of_pre-built_binaries_or_libraries ).

If it's actually building Swift, then you should open a ticket upstream and get the issue resolved in their build system so it can play nice with Fedora.

> Release is usually just an integer (i.e. 1 instead of 0.1)

Generally, that's not true, since you seem to forget the pre-release guidelines:

  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

And if this package really is a snapshot, there are specific guidelines for snapshots, too:

  https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages


> %setup -q -T -D -a 1
> %setup -q -T -D -a 2
> %setup -q -T -D -a 3
> %setup -q -T -D -a 4
> %setup -q -T -D -a 5
> %setup -q -T -D -a 6
> %setup -q -T -D -a 7
> %setup -q -T -D -a 8
> %setup -q -T -D -a 9
> %setup -q -T -D -a 10

Isn't it possibly anymore to specify -a multiple times in a single %setup invocation?


> %post -p /sbin/ldconfig
> 
> %postun -p /sbin/ldconfig

Why is ldconfig run for the base package, although the package does not store any libs in runtime linker's search path?


> %files lldb
> %{_libdir}/liblldb.so*

This subpackage wants to run ldconfig in its scriptlets.


> %{_libdir}/swift/pm/*

Lots of so-called "unowned directories" created by this package. Kindly review the following pages

  https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
  https://fedoraproject.org/wiki/Packaging:UnownedDirectories

and carefully examine directory ownership of all [sub-]packages.


> %doc %{_mandir}/man1/swift.1.gz
> %doc %{_datarootdir}/doc/swift/*

%_datarootdir/doc = %_datadir/doc = %_docdir

And both %_docdir as well as %_mandir is a path where files implicitly are marked as %doc, because it's in the %__docdir_path list. See: rpm -E %__docdir_path

> Is this actually building Swift or just unpacking a pre-built binary?

The package is being built from source; not just unpacking a pre-build binary.  The problem is that /usr/lib is hardcoded in a number of places throughout the build system.  I have been going through and trying to clean that up and providing a patch to upstream.

> Get rid of # in Source* tags and use original names in source .rpm

Since this is a snapshot from upstream not all repos required for building have tags yet.  This was my way to work around conflicting filenames.

> Release is usually just an integer (i.e. 1 instead of 0.1)

I thought this was the convention for snapshots.  As @Michael Schwendt pointed out: 

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

> Are versions listed in BuildRequires "real" (i.e. listed in Swift documentation and/or the true minimum version required)?

Yes.

I'll take a look at the unowned directories and other comments and clean up.  Thanks for all the feedback!

URLs should use https (not http) if possible. Both swift.org and github.com support https.


You should define a macro with the common parameters to build commands:
%global buildcmd \
  -swift/utils/build-script \
  --assertions \
  --no-swift-stdlib-assertions \
  --llbuild --swiftpm \
  --xctest ... \
   --reconfigure

and then use it instead of the common part in %build, %check, %install.


%files lldb
...
%{python2_sitelib}/readline.so

This looks very wrong. swift should be be installing a file with a name of an existing python module. It should also not be installing a binary file in a non-arched directory.

For the other .so files: usually the versioned libraries (*.so.*) are packaged in the library subpackage (swift-lang-lldb in this case), and the versionless .so link is packaged in the devel subpackage (swift-lang-lldb-devel) in this case.

(In reply to jeremy.fergason from comment #14)
> > Get rid of # in Source* tags and use original names in source .rpm
> 
> Since this is a snapshot from upstream not all repos required for building
> have tags yet.  This was my way to work around conflicting filenames.

Could these tags be requested so that everything is uniform?

> > Release is usually just an integer (i.e. 1 instead of 0.1)
> 
> I thought this was the convention for snapshots.  As @Michael Schwendt
> pointed out: 
> 
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-
> Release_packages

Sorry about that. I forgot about that policy.

> I'll take a look at the unowned directories and other comments and clean up.
> Thanks for all the feedback!

%license is now defined in EPEL, so you can get rid of the compatibility definition that you added.
https://lists.fedoraproject.org/archives/list/epel-devel%40lists.fedoraproject.org/message/DKPU2WQ3H7XD5ELGLKGB5MV3JIMM3HIV/

Just as an FYI, llvm is moving to using cmake in rawhide (F24) and is now separate packages (see bug #1300945).

Thanks!  I'm still trying to get the lib vs lib64 issue sorted with the swift developers.

You may want to look into what I did for Editorconfig to get their lib/lib64 working. Hopefully you find this useful. https://barracks510.fedorapeople.org/packaging/editorconfig-install.patch

Any news? I'm interested in this package.

I believe that I have successfully integrated all the feedback received and the RPM now installs into the appropriate lib64 directory.  It also builds in Koji, see: http://koji.fedoraproject.org/koji/taskinfo?taskID=14931923

Updated spec file:
https://raw.githubusercontent.com/jdfergason/swift-rpm/master/SPECS/swift-lang.spec

What are the next steps necessary to get this into EPEL?

The main thing is performing the actual review ( https://fedoraproject.org/wiki/Package_Review_Process ) and resolving any issues that come up.

Also, have you been sponsored as a packager?

It seems like there's still a lot of excessive bundling going on. I'm under the impression that a lot of these tarballs can be and should be split into separate packages, each with their own package review. Also, conflicting with LLDB is not good.

@DaveJohansen No I have not been sponsored as a packager yet.

@DennisChen I agree.  To some extent this is the way that the Swift development team views the world.  The package can be split into 3 different packages but the main package will still require a lot of bundling.

Possible packages:

Swift Compiler / REPL - Requires Sources 0, 1, 2, 3, 4, 7, 9, 11
Swift Pacakge Manager - Requires 5, 6 (depends on above)
XCTest - Requires 8 (depends on Swift Compiler)

It would also be nice to have the standard library be separate from the compiler package / REPL to ease distributing software built with swift.

Regarding the LLDB conflict I agree that it is not good but I see no way around it as that is how Swift's REPL works.  They have modified LLDB and it conflicts with the main package.

Updated SPEC to address unowned directories, remove swift package manager, and divide sub packages in a more sane way:

swiftlang (meta-package installs swiftlang-lib swiftlang-bin swiftlang-devel)
swiftlang-lib - Runtime libraries
swiftlang-bin - Compiler / REPL
swiftlang-xctest - Runtime libraries for XCTest framework (included because it is required for testing the compiler)
swiftlang-devel - Headers

New SPEC file:
https://raw.githubusercontent.com/jdfergason/swift-rpm/master/SPECS/swiftlang.spec

Possible Todos:

* Add versioning to shared libraries
* Confirm the subpackge naming convention ... specifically should a swiftlang-static
  be added for all the .a files
* Determine if *.h files used by compiler need to be moved into -devel package
  or someother package.  It seems to me that -devel should only include headers
  relevant to the standard library and Foundation.  However, the headers for clang
  and lldb are required for the compiler to work.

I got the Todos added into the most recent version of the spec file.  Swift is a complicated packaging scenario and I believe that it conforms to the Fedora Packaging Guidelines.

The one fly in the ointment is the conflict with LLDB.  I see no good way around this.  If we rename the swift version of lldb it will be confusing to users as all the swift documentation just says type lldb.  If we don't rename it then there is a conflict with the lldb package.

At this point I need a sponsor and a review.  I'm not sure how to go about getting that so until I hear otherwise the SPEC is ready to go.

https://raw.githubusercontent.com/jdfergason/swift-rpm/master/SPECS/swiftlang.spec

(In reply to jeremy.fergason from comment #26)
> The one fly in the ointment is the conflict with LLDB.  I see no good way
> around this.  If we rename the swift version of lldb it will be confusing to
> users as all the swift documentation just says type lldb.  If we don't
> rename it then there is a conflict with the lldb package.

My personal opinion is that having the "official" lldb binary left as is should take precendence over catering to Swift's documentation.

The Fedora policy seems to indicate the same general feeling:
https://fedoraproject.org/wiki/Packaging:Conflicts#Binary_Name_Conflicts

This message is a reminder that Fedora 23 is nearing its end of life.
Approximately 4 (four) weeks from now Fedora will stop maintaining
and issuing updates for Fedora 23. It is Fedora's policy to close all
bug reports from releases that are no longer maintained. At that time
this bug will be closed as EOL if it remains open with a Fedora  'version'
of '23'.

Package Maintainer: If you wish for this bug to remain open because you
plan to fix it in a currently maintained version, simply change the 'version' 
to a later Fedora version.

Thank you for reporting this issue and we are sorry that we were not 
able to fix it before Fedora 23 is end of life. If you would still like 
to see this bug fixed and are able to reproduce it against a later version 
of Fedora, you are encouraged  change the 'version' to a later Fedora 
version prior this bug is closed as described in the policy above.

Although we aim to fix as many bugs as possible during every release's 
lifetime, sometimes those efforts are overtaken by events. Often a 
more recent Fedora release includes newer upstream software that fixes 
bugs or makes them obsolete.

(In reply to jeremy.fergason from comment #26)
> I got the Todos added into the most recent version of the spec file.  Swift
> is a complicated packaging scenario and I believe that it conforms to the
> Fedora Packaging Guidelines.
> 
> The one fly in the ointment is the conflict with LLDB.  I see no good way
> around this.  If we rename the swift version of lldb it will be confusing to
> users as all the swift documentation just says type lldb.  If we don't
> rename it then there is a conflict with the lldb package.
> 
> At this point I need a sponsor and a review.  I'm not sure how to go about
> getting that so until I hear otherwise the SPEC is ready to go.
> 
> https://raw.githubusercontent.com/jdfergason/swift-rpm/master/SPECS/
> swiftlang.spec

I will give this a go on fedora 25 and see if your spec still works, we should change the version this bug relates to to f25. 

I seem to have built up a community around a "hack" script i built, and i want to retire it if possible, and redirect it to more "official" distro support.

https://github.com/FedoraSwift/fedora-swift

(In reply to jeremy.fergason from comment #26)
> I got the Todos added into the most recent version of the spec file.  Swift
> is a complicated packaging scenario and I believe that it conforms to the
> Fedora Packaging Guidelines.
> 
> The one fly in the ointment is the conflict with LLDB.  I see no good way
> around this.  If we rename the swift version of lldb it will be confusing to
> users as all the swift documentation just says type lldb.  If we don't
> rename it then there is a conflict with the lldb package.
> 
> At this point I need a sponsor and a review.  I'm not sure how to go about
> getting that so until I hear otherwise the SPEC is ready to go.
> 
> https://raw.githubusercontent.com/jdfergason/swift-rpm/master/SPECS/
> swiftlang.spec

@jeremy, do you have all the patchfiles that match your spec file? Im trying tro reproduce your package

(In reply to Tim Hawkins from comment #30)
> @jeremy, do you have all the patchfiles that match your spec file? Im trying
> tro reproduce your package

It appears that they're available in the github repo that Jeremy referenced:
https://github.com/jdfergason/swift-rpm/tree/master/SOURCES

Or the SRPM linked to in this review has all of the code and patches (see comment 11).

A more recent and also *great* work is available with [1]

Using patch [2], I generated a Swift 3.1 RPM for Fedora 25.
It provides REPL, LLDB, Package and Foundation:

------------------------------------------
$ swift --version
Swift version 3.1 (swift-3.1-RELEASE)
Target: x86_64-unknown-linux-gnu
------------------------------------------

------------------------------------------
$ swift
Welcome to Swift version 3.1 (swift-3.1-RELEASE). Type :help for assistance.
  1> 
------------------------------------------

------------------------------------------
$ lldb --version
lldb version 4.0.0
  Swift-3.1 (revision 6309a8a39f499549676a1ac889f01d04ac823d8f)
------------------------------------------


[1] https://github.com/corinnekrych/swift-rpm
[2] https://github.com/corinnekrych/swift-rpm/pull/6

As an update, I have updated the scripts to build Swift 4 (waiting for Corinne to pull the request):

https://github.com/tachoknight/swift-rpm

One thing that would presumably need to be worked out is that there are a bunch of pulls from Github that I presume would be frowned on in that it's not a properly self-contained project. Also Fedora does not provide libBlocksRuntime for some reason, so that is yet another explicit checkout and install that wouldn't pass muster from package reviewers. 

(In reply to Ron Olson from comment #33)
> As an update, I have updated the scripts to build Swift 4 (waiting for
> Corinne to pull the request):
> 
> https://github.com/tachoknight/swift-rpm

*** This bug has been marked as a duplicate of bug 1536780 ***

Closed -> duplicate