Bug 1173740 - fedora-review: please move license check after %prep
Summary: fedora-review: please move license check after %prep
Keywords:
Status: CLOSED WORKSFORME
Alias: None
Product: Fedora
Classification: Fedora
Component: fedora-review
Version: 22
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Stanislav Ochotnicky
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2014-12-12 20:03 UTC by Zbigniew Jędrzejewski-Szmek
Modified: 2015-04-15 14:51 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-04-15 14:51:52 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Zbigniew Jędrzejewski-Szmek 2014-12-12 20:03:56 UTC
Description of problem:
Problematic sources are often removed in %prep. If license check was performed after %prep, spurious warnings would be avoided.

Version-Release number of selected component (if applicable):
fedora-review-0.5.2-1.fc21.noarch

Comment 1 Jaroslav Reznik 2015-03-03 16:36:52 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 22 development cycle.
Changing version to '22'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora22

Comment 2 Raphael Groner 2015-03-03 18:50:46 UTC
My opinion is that this is no good idea. The guidelines say to remove all proplematic source before %prep is even called, that means you have to provide a clean sources tarball in SRPM. It is not allowed to provide nonfree stuff even as sources in SRPM, if you mean that with "problematic sources".

The goal of the Fedora Project is to work with the Linux community to create a complete, general purpose operating system exclusively from Free and Open Source software.
All software in Fedora must be under licenses in the Fedora licensing list . This list is based on the licenses approved by the Free Software Foundation , OSI and consultation with Red Hat Legal.
If code is multiple licensed, and at least one of the licenses is approved for Fedora, that code can be included in Fedora under the approved license(s) (but only under the terms of the approved license(s)). 
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Fedora_Licensing

Besides that, I guess this must be discussed in prior by opening a ticket against the guidelines.
https://fedorahosted.org/fpc

Comment 3 Pierre-YvesChibon 2015-03-03 18:57:04 UTC
I quite agree with Raphael, the license check is better performed on the raw sources, as all the code uploaded to the lookaside should be complying with our licensing policy.

Comment 4 Stephen Gallagher 2015-03-03 19:00:50 UTC
Okay, so I partially agree with Raphael above. Any sources shipped in the tarball must be covered by the license check, because we *are* distributing it. Removal in the %prep phase does *not* constitute "not distributing" the code (I am not a lawyer, but I'm reasonably confident saying that we would be in violation if someone stuck the contents of a Pixar film in a tarball then deleted it in %post...)

However, there *is* value in running the check a second time after %prep. It's possible that patches applied in this phase may add new licenses, and if there's any diff at all between them, that should probably flag the attention of the reviewer for a closer look.

Comment 5 Zbigniew Jędrzejewski-Szmek 2015-03-03 19:12:18 UTC
Well, I really doubt that the license check is going to catch non-distributable sources. In the original report, by "problematic" I meant bundled code which has to be deleted because of packaging policies or to make sure that the bundled copy is not used by mistake, and not stuff which cannot be legally distributed. License check seems to be pretty good at detecting various open-source licenses, but stuff which is non-distributable varies a lot and is hard to detect. Checking license after %prep would reduce the noise and let license check do what it does best and make it easier to notice bundled code and/or correctly specify the license of the code the package is actually built from.

If sources *do* contain actual non-distributable code, removing them in %prep to avoid triggering the license check warning would amount to evasive action by the packager. I don't think this is something we should worry about. If license check is smart enough to warn about non-distributable content, the packager should be able to figure out the proper way to deal with it (repack the tarball) on her own.

Comment 6 Alec Leamas 2015-04-15 14:01:50 UTC
Hm....

- The idea so far has been along Zbigniew's line of reasoning i. e. we have not really been focused on licenses we cant distribute but rather to verify that  the combination of licenses after %prep valid.

- As of today, the test *is* done after %prep, so this report is odd. If there really are some conditions under which the license check is done before %prep this is definitely a bug but then we need more data so it can be reproduced.

Unless there is input showing that this test is indeed done before %prep (and how) I will close this as notabug.

Comment 7 Zbigniew Jędrzejewski-Szmek 2015-04-15 14:17:14 UTC
I reported this after running a fedora-review on some package, but I don't remember what it was now. I'll try to find it.

Comment 8 Zbigniew Jędrzejewski-Szmek 2015-04-15 14:51:52 UTC
This seems to have been operator error. I created a package which removes a file in %prep, and indeed, fedora-review does not report this file.


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