Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1873108 - Review Request: gnome-tour - GNOME Tour and Greeter
Summary: Review Request: gnome-tour - GNOME Tour and Greeter
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Felipe Borges
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1873206
TreeView+ depends on / blocked
 
Reported: 2020-08-27 12:00 UTC by Kalev Lember
Modified: 2020-08-27 16:16 UTC (History)
5 users (show)

Fixed In Version: gnome-tour-3.37.91-2.fc33 gnome-tour-3.37.91-2.fc34
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-08-27 16:14:53 UTC
Type: ---
Embargoed:
feborges: fedora-review+


Attachments (Terms of Use)

Description Kalev Lember 2020-08-27 12:00:19 UTC
Spec URL: https://kalev.fedorapeople.org/gnome-tour.spec
SRPM URL: https://kalev.fedorapeople.org/gnome-tour-3.37.91-1.fc33.src.rpm
Description: A guided tour and greeter for GNOME.
Fedora Account System Username: kalev

This is the new tour/greeter that replaces the yelp-based intro at the end of gnome-initial-setup run.

Note that we don't have all the required rust libraries packaged (in particular, libhandy-1 is missing) so we're using the bundled copies for the moment. I've prepared conditionals to switch over once we get rust libhandy-1.

Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=50242945

Comment 1 Felipe Borges 2020-08-27 12:54:44 UTC
No issues with $ sudo fedora-review -b 1873108, it build just fine. It didn't spit any errors about the package either. No problems with rpmlint.

Thank you!

Comment 2 Artur Frenszek-Iwicki 2020-08-27 13:01:39 UTC
> %global debug_package %{nil}
Why? Other rust-* packages ship binaries with debuginfo.

>BuildRequires:  /usr/bin/appstream-util
Use "libappstream-glib".
>BuildRequires:  /usr/bin/desktop-file-validate
Use "desktop-file-utils".

Also, you're missing "ExclusiveArch:  %{rust_arches}".

Comment 3 Fabio Valentini 2020-08-27 13:04:13 UTC
Not so fast :)

I see two issues:

- Since you're using bundled dependencies, you'll need to list them all:
Provides: bundled(crate(foo)) = version

- You'll need to "calculate" the effective license of everything that's getting statically linked into the built binary packages.

For example, one of my rust packages that ships a binary:
https://src.fedoraproject.org/rpms/rust-bodhi-cli/blob/master/f/rust-bodhi-cli.spec#_29
  → list all licenses of bundled dependencies
  → calculate union
  → set License tag accordingly

Comment 4 Kalev Lember 2020-08-27 13:44:07 UTC
(In reply to Artur Iwicki from comment #2)
> > %global debug_package %{nil}
> Why? Other rust-* packages ship binaries with debuginfo.

Hm. Good question -- I thought it was common to not ship debuginfo in rust packages, especially since the example in https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/ has '%global debug_package %{nil}'. Do you have any ideas what's missing that the debuginfo isn't generated? I am not familiar at all with rust packaging.


> >BuildRequires:  /usr/bin/appstream-util
> Use "libappstream-glib".
> >BuildRequires:  /usr/bin/desktop-file-validate
> Use "desktop-file-utils".

Thanks for the suggestion, but I prefer it the way it is. It's more explanatory to explicitly list the binary we need.


> Also, you're missing "ExclusiveArch:  %{rust_arches}".

Yes, I didn't use that because the list is exactly the same as regular Fedora arches. We want the package on all Fedora-supported arches as it's going to be in the default Workstation install. I'll be sure to add it in the future if it's needed to keep the package building, but in the mean time I'd like to not have the ExclusiveArch to avoid surprises.

Comment 5 Kalev Lember 2020-08-27 13:44:37 UTC
(In reply to Fabio Valentini from comment #3)
> Not so fast :)
> 
> I see two issues:
> 
> - Since you're using bundled dependencies, you'll need to list them all:
> Provides: bundled(crate(foo)) = version
> 
> - You'll need to "calculate" the effective license of everything that's
> getting statically linked into the built binary packages.
> 
> For example, one of my rust packages that ships a binary:
> https://src.fedoraproject.org/rpms/rust-bodhi-cli/blob/master/f/rust-bodhi-
> cli.spec#_29
>   → list all licenses of bundled dependencies
>   → calculate union
>   → set License tag accordingly

Ahh yes, let me do that! Thanks, Fabio.

Comment 6 Kalev Lember 2020-08-27 15:06:17 UTC
(In reply to Kalev Lember from comment #5)
> (In reply to Fabio Valentini from comment #3)
> > Not so fast :)
> > 
> > I see two issues:
> > 
> > - Since you're using bundled dependencies, you'll need to list them all:
> > Provides: bundled(crate(foo)) = version
> > 
> > - You'll need to "calculate" the effective license of everything that's
> > getting statically linked into the built binary packages.
> > 
> > For example, one of my rust packages that ships a binary:
> > https://src.fedoraproject.org/rpms/rust-bodhi-cli/blob/master/f/rust-bodhi-
> > cli.spec#_29
> >   → list all licenses of bundled dependencies
> >   → calculate union
> >   → set License tag accordingly
> 
> Ahh yes, let me do that! Thanks, Fabio.

OK, here we go:

* Thu Aug 27 2020 Kalev Lember <klember> - 3.37.91-2
- Add provides for bundled rust crates (#1873108)
- Clarify licensing for bundled rust crates (#1873108)

Spec URL: https://kalev.fedorapeople.org/gnome-tour.spec
SRPM URL: https://kalev.fedorapeople.org/gnome-tour-3.37.91-2.fc33.src.rpm

Comment 7 Fabio Valentini 2020-08-27 15:11:47 UTC
Bundled crates are listed correctly now, thanks!

However, I'm pretty sure that GPLv3+ doesn't subsume MIT, so the License should be "GPLv3+ and MIT and CC-BY-SA".

Side note: The License tag should include the "effective" license whether or not it uses the bundled deps, since it's statically linked either way.

Comment 8 Kalev Lember 2020-08-27 15:17:56 UTC
Excellent, thanks for checking! Ohh, good point for the "effective" license and statically linked binary, I didn't realize that.

Re combining MIT and GPLv3+ in one executable, I'm pretty sure the resulting binary is effectively licensed under the strictest of the two licenses, which in this case is GPLv3+. It would be 'MIT and GPLv3+' when one binary is MIT and another one is GPLv3+, but since they are compiled together into one binary, the "strictest" license wins. See https://fedoraproject.org/wiki/Licensing:FAQ?rd=Licensing/FAQ#How_should_I_handle_multiple_licensing_situations.3F and in particular, point 1 there. I'll quote:

<quote>
The source code contains some .c files which are GPLv2+ and some other .c files which are BSD. They're compiled together to form an executable. Since some of the files are licensed as GPL, the resulting executable is also GPL. The License tag should read: License: GPLv2+
Note that you do NOT need to list BSD in the License tag, the License tag reflects the resulting, packaged, items in the binary RPM.
</quote>

Comment 9 Fabio Valentini 2020-08-27 15:19:47 UTC
Good point, I stand corrected. :)

Comment 10 Kalev Lember 2020-08-27 15:22:55 UTC
OK, thanks everybody! I went ahead and requested the git repo now.

Comment 11 Gwyn Ciesla 2020-08-27 16:00:33 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/gnome-tour

Comment 12 Kalev Lember 2020-08-27 16:14:53 UTC
Package important and gnome-tour-3.37.91-2.fc33 and gnome-tour-3.37.91-2.fc34 builds are under way in koji.

Comment 13 Kalev Lember 2020-08-27 16:16:11 UTC
Err, "imported", not "important" :)


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