Bug 1323186 - Review Request: opa-fmgui - Intel OPA Fabric GUI
Summary: Review Request: opa-fmgui - Intel OPA Fabric GUI
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Neil Horman
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1348668 (view as bug list)
Depends On:
Blocks: 1173314
TreeView+ depends on / blocked
 
Reported: 2016-04-01 13:10 UTC by robert.amato
Modified: 2016-07-28 15:30 UTC (History)
11 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-07-28 15:30:52 UTC
Type: ---
nhorman: fedora-review+


Attachments (Terms of Use)
opa-fmgui spec file (4.71 KB, text/plain)
2016-05-26 13:42 UTC, Rick Tierney
no flags Details
opa-fmgui spec file (5.36 KB, text/plain)
2016-05-26 21:01 UTC, Rick Tierney
no flags Details
FMGUI Spec File (6.93 KB, text/plain)
2016-06-07 21:00 UTC, Rick Tierney
no flags Details

Description robert.amato 2016-04-01 13:10:49 UTC
Spec URL: https://github.com/01org/opa-fmgui/blob/Fedora_Review/opa-fmgui.spec

SRPM URL: https://github.com/01org/opa-fmgui/releases/tag/v1.1

Description: The opa-fmgui is a Java-based graphical user interface for the Intel Omni-Path Architecture computing fabric.

Fedora Account System Username: robertja

Comment 1 Neil Horman 2016-04-08 14:17:50 UTC
Please fix your SRPM url to point directly to the downloadable link.

Comment 2 robert.amato 2016-04-08 14:37:07 UTC
(In reply to robert.amato from comment #0)
> Spec URL:
> https://github.com/01org/opa-fmgui/blob/Fedora_Review/opa-fmgui.spec
> 
> SRPM URL: https://github.com/01org/opa-fmgui/releases/download/v1.1/opa-fmgui-10.0.0.0.3-1.fc25.src.rpm> 
>
> Description: The opa-fmgui is a Java-based graphical user interface for the
> Intel Omni-Path Architecture computing fabric.
> 
> Fedora Account System Username: robertja


SRPM URL updated to point directly to SRPM file.

Comment 3 Neil Horman 2016-04-11 00:21:42 UTC
Ok, now you need to do the same thing with the spec file, you have to point to the file itself, not a github page that displays a version of it in some embedded html.  Please format the bz update as found in the update template located here:
https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora&format=fedora-review

so that the fedora review tool can find it.

Comment 4 robert.amato 2016-04-11 11:15:23 UTC
Spec URL: https://github.com/01org/opa-fmgui/releases/download/v1.1/opa-fmgui.spec

SRPM URL: https://github.com/01org/opa-fmgui/releases/download/v1.1/opa-fmgui-10.0.0.0.3-1.fc25.src.rpm> 

Description: The opa-fmgui is a Java-based graphical user interface for the
Intel Omni-Path Architecture computing fabric.
 
Fedora Account System Username: robertja

Comment 5 Neil Horman 2016-04-11 11:45:02 UTC
perfect, thank you! I'll run the review now.

Comment 6 Neil Horman 2016-04-11 14:36:42 UTC
 
This is a review *template*. Besides handling the [ ]-marked tests you are
also supposed to fix the template before pasting into bugzilla:
- Add issues you find to the list of issues on top. If there isn't such
  a list, create one.
- Add your own remarks to the template checks.
- Add new lines marked [!] or [?] when you discover new things not
  listed by fedora-review.
- Change or remove any text in the template which is plain wrong. In this
  case you could also file a bug against fedora-review
- Remove the "[ ] Manual check required", you will not have any such lines
  in what you paste.
- Remove attachments which you deem not really useful (the rpmlint
  ones are mandatory, though)
- Remove this text



Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- This seems like a Java package, please install fedora-review-plugin-java
  to get additional checks


===== MUST items =====

Generic:
[x]: 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: "BSD (3 clause)", "LGPL (v2 or later) (with incorrect FSF
     address)", "Unknown or generated". 50 files have unknown license.
     Detailed output of licensecheck in /home/nhorman/1323186-opa-
     fmgui/licensecheck.txt
<NH>
Package includes License files Third_Party_Copyright_Notices_and_Licenses.docx
and THIRD-PARTY-README which seem to relate to code which is not packaged in
this srpm.  If that is the case, then these files should not be packaged.  If it
is the case, then the license needs to change in the spec file, the docx files
needs to be converted to text and the binaries need to have thier licensing
ennumerated.

[x]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/java,
     /etc/xdg/menus, /etc/profile.d, /etc/xdg/menus/applications-merged
[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/icons/hicolor
     (hicolor-icon-theme, fedora-logos)
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[!]: %config files are marked noreplace or the reason is justified.
     Note: No (noreplace) in %config /etc/xdg/menus/applications-
     merged/Fabric.menu %config /etc/profile.d/fmguivars.sh
<NH>
See rpmlint below
[-]: Development files must be in a -devel package
[!]: Package uses nothing in %doc for runtime.
You put documents in the App folder, but they are not marked as such, if they
need to be included at all (THIRD PARTY docs mentioned above)

[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[!]: Package is named according to the Package Naming Guidelines.
<NH>
Package should likely be named fmgui to correspond to fmgui.jar file

[x]: Package does not generate any conflict.
[s]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in opa-fmgui
[x]: Package is not known to require an ExcludeArch tag.
[x]: 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]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[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]: No %config files under /usr.
[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]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Bundled jar/class files should be removed before build

===== SHOULD items =====

Generic:
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[!]: Package functions as described.
<NH>
Attempted to run fmgui against openjdk 1.8 and it seems to fail indicating that
java was not at a sufficient vm level

[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[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]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: opa-fmgui-10.0.0.0.3-1.fc25.noarch.rpm
          opa-fmgui-10.0.0.0.3-1.fc25.src.rpm
opa-fmgui.noarch: W: invalid-url URL www.intel.com
opa-fmgui.noarch: W: conffile-without-noreplace-flag /etc/profile.d/fmguivars.sh
opa-fmgui.noarch: W: conffile-without-noreplace-flag /etc/xdg/menus/applications-merged/Fabric.menu
opa-fmgui.noarch: W: no-documentation
opa-fmgui.noarch: W: no-manual-page-for-binary fmgui
opa-fmgui.noarch: W: class-path-in-manifest /usr/share/java/fmgui/fmgui.jar
opa-fmgui.src: W: invalid-url URL www.intel.com
opa-fmgui.src: W: invalid-url Source0: opa-fmgui-10.0.0.0.3.tar.gz
2 packages and 0 specfiles checked; 0 errors, 8 warnings.




Rpmlint (installed packages)
----------------------------
sh: /usr/bin/python: No such file or directory
opa-fmgui.noarch: W: invalid-url URL www.intel.com
opa-fmgui.noarch: W: conffile-without-noreplace-flag /etc/xdg/menus/applications-merged/Fabric.menu
opa-fmgui.noarch: W: conffile-without-noreplace-flag /etc/profile.d/fmguivars.sh
opa-fmgui.noarch: W: no-documentation
opa-fmgui.noarch: W: no-manual-page-for-binary fmgui
opa-fmgui.noarch: W: class-path-in-manifest /usr/share/java/fmgui/fmgui.jar
1 packages and 0 specfiles checked; 0 errors, 6 warnings.



Requires
--------
opa-fmgui (rpmlib, GLIBC filtered):
    /bin/bash
    /bin/sh
    config(opa-fmgui)
    jre



Provides
--------
opa-fmgui:
    application()
    application(fmgui.desktop)
    config(opa-fmgui)
    opa-fmgui



Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
Command line :/usr/bin/fedora-review -b 1323186
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, Java
Disabled plugins: C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 7 Honggang LI 2016-04-11 14:59:12 UTC
(In reply to Neil Horman from comment #6)
> [x]: Package consistently uses macros (instead of hard-coded directory
>      names).
> [!]: Package is named according to the Package Naming Guidelines.
> <NH>
> Package should likely be named fmgui to correspond to fmgui.jar file
> 

I could suggest to keep the name "opa-fmgui", as other Intel OPA stack packages' name start with "opa-". Those include opa-ff, opa-fm.

Comment 8 Honggang LI 2016-04-20 11:33:28 UTC
Hi Robert
 Ping? Any update? thanks

Comment 9 robert.amato 2016-04-21 09:30:35 UTC
(In reply to Honggang LI from comment #8)
> Hi Robert
>  Ping? Any update? thanks

Honli,

We will use the name opa-fmgui as you suggest.  I should have a revised package by COB Friday (4/22).

Robert

Comment 10 Honggang LI 2016-04-21 09:33:14 UTC
Please fix those issues pointed out by Neil.

Comment 11 robert.amato 2016-04-21 09:49:30 UTC
(In reply to Honggang LI from comment #10)
> Please fix those issues pointed out by Neil.

Will do.

Comment 12 robert.amato 2016-04-22 12:59:08 UTC
(In reply to Neil Horman from comment #6)
>  
> This is a review *template*. Besides handling the [ ]-marked tests you are
> also supposed to fix the template before pasting into bugzilla:
> - Add issues you find to the list of issues on top. If there isn't such
>   a list, create one.
> - Add your own remarks to the template checks.
> - Add new lines marked [!] or [?] when you discover new things not
>   listed by fedora-review.
> - Change or remove any text in the template which is plain wrong. In this
>   case you could also file a bug against fedora-review
> - Remove the "[ ] Manual check required", you will not have any such lines
>   in what you paste.
> - Remove attachments which you deem not really useful (the rpmlint
>   ones are mandatory, though)
> - Remove this text
> 
> 
> 
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> Issues:
> =======
> - This seems like a Java package, please install fedora-review-plugin-java
>   to get additional checks
> 
> 
> ===== MUST items =====
> 
> Generic:
> [x]: 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: "BSD (3 clause)", "LGPL (v2 or later) (with incorrect FSF
>      address)", "Unknown or generated". 50 files have unknown license.
>      Detailed output of licensecheck in /home/nhorman/1323186-opa-
>      fmgui/licensecheck.txt
> <NH>
> Package includes License files
> Third_Party_Copyright_Notices_and_Licenses.docx
> and THIRD-PARTY-README which seem to relate to code which is not packaged in
> this srpm.  If that is the case, then these files should not be packaged. 
> If it
> is the case, then the license needs to change in the spec file, the docx
> files
> needs to be converted to text and the binaries need to have thier licensing
> ennumerated.
> 
> [x]: Package must own all directories that it creates.
>      Note: Directories without known owners: /usr/share/java,
>      /etc/xdg/menus, /etc/profile.d, /etc/xdg/menus/applications-merged
> [x]: Package does not own files or directories owned by other packages.
>      Note: Dirs in package are owned also by: /usr/share/icons/hicolor
>      (hicolor-icon-theme, fedora-logos)
> [x]: Package contains no bundled libraries without FPC exception.
> [x]: Changelog in prescribed format.
> [x]: Sources contain only permissible code or content.
> [!]: %config files are marked noreplace or the reason is justified.
>      Note: No (noreplace) in %config /etc/xdg/menus/applications-
>      merged/Fabric.menu %config /etc/profile.d/fmguivars.sh
> <NH>
> See rpmlint below
> [-]: Development files must be in a -devel package
> [!]: Package uses nothing in %doc for runtime.
> You put documents in the App folder, but they are not marked as such, if they
> need to be included at all (THIRD PARTY docs mentioned above)
> 
> [x]: Package consistently uses macros (instead of hard-coded directory
>      names).
> [!]: Package is named according to the Package Naming Guidelines.
> <NH>
> Package should likely be named fmgui to correspond to fmgui.jar file
> 
> [x]: Package does not generate any conflict.
> [s]: Package obeys FHS, except libexecdir and /usr/target.
> [-]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [x]: Requires correct, justified where necessary.
> [x]: Spec file is legible and written in American English.
> [-]: Package contains systemd file(s) if in need.
> [x]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
>      contains icons.
>      Note: icons in opa-fmgui
> [x]: Package is not known to require an ExcludeArch tag.
> [x]: 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]: Package contains desktop file if it is a GUI application.
> [x]: Package installs a %{name}.desktop using desktop-file-install or
>      desktop-file-validate if there is such a file.
> [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]: No %config files under /usr.
> [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]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 0 bytes in 0 files.
> [x]: Packages must not store files under /srv, /opt or /usr/local
> 
> Java:
> [x]: Bundled jar/class files should be removed before build
> 
> ===== SHOULD items =====
> 
> Generic:
> [x]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.
> [x]: Final provides and requires are sane (see attachments).
> [!]: Package functions as described.
> <NH>
> Attempted to run fmgui against openjdk 1.8 and it seems to fail indicating
> that
> java was not at a sufficient vm level
> 
> [x]: Latest version is packaged.
> [x]: Package does not include license text files separate from upstream.
> [x]: SourceX tarball generation or download is documented.
>      Note: Package contains tarball without URL, check comments
> [-]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.
> [-]: Package should compile and build into binary rpms on all supported
>      architectures.
> [-]: %check is present and all tests pass.
> [x]: Packages should try to preserve timestamps of original installed
>      files.
> [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
> [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]: SourceX is a working URL.
> [x]: Spec use %global instead of %define unless justified.
> 
> ===== EXTRA items =====
> 
> Generic:
> [x]: Rpmlint is run on all installed packages.
>      Note: There are rpmlint messages (see attachment).
> [x]: Spec file according to URL is the same as in SRPM.
> 
> 
> Rpmlint
> -------
> Checking: opa-fmgui-10.0.0.0.3-1.fc25.noarch.rpm
>           opa-fmgui-10.0.0.0.3-1.fc25.src.rpm
> opa-fmgui.noarch: W: invalid-url URL www.intel.com
> opa-fmgui.noarch: W: conffile-without-noreplace-flag
> /etc/profile.d/fmguivars.sh
> opa-fmgui.noarch: W: conffile-without-noreplace-flag
> /etc/xdg/menus/applications-merged/Fabric.menu
> opa-fmgui.noarch: W: no-documentation
> opa-fmgui.noarch: W: no-manual-page-for-binary fmgui
> opa-fmgui.noarch: W: class-path-in-manifest /usr/share/java/fmgui/fmgui.jar
> opa-fmgui.src: W: invalid-url URL www.intel.com
> opa-fmgui.src: W: invalid-url Source0: opa-fmgui-10.0.0.0.3.tar.gz
> 2 packages and 0 specfiles checked; 0 errors, 8 warnings.
> 
> 
> 
> 
> Rpmlint (installed packages)
> ----------------------------
> sh: /usr/bin/python: No such file or directory
> opa-fmgui.noarch: W: invalid-url URL www.intel.com
> opa-fmgui.noarch: W: conffile-without-noreplace-flag
> /etc/xdg/menus/applications-merged/Fabric.menu
> opa-fmgui.noarch: W: conffile-without-noreplace-flag
> /etc/profile.d/fmguivars.sh
> opa-fmgui.noarch: W: no-documentation
> opa-fmgui.noarch: W: no-manual-page-for-binary fmgui
> opa-fmgui.noarch: W: class-path-in-manifest /usr/share/java/fmgui/fmgui.jar
> 1 packages and 0 specfiles checked; 0 errors, 6 warnings.
> 
> 
> 
> Requires
> --------
> opa-fmgui (rpmlib, GLIBC filtered):
>     /bin/bash
>     /bin/sh
>     config(opa-fmgui)
>     jre
> 
> 
> 
> Provides
> --------
> opa-fmgui:
>     application()
>     application(fmgui.desktop)
>     config(opa-fmgui)
>     opa-fmgui
> 
> 
> 
> Generated by fedora-review 0.6.0 (3c5c9d7) last change: 2015-05-20
> Command line :/usr/bin/fedora-review -b 1323186
> Buildroot used: fedora-rawhide-x86_64
> Active plugins: Generic, Shell-api, Java
> Disabled plugins: C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell,
> R, PHP, Ruby
> Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6


Neil,

I need clarification on your comment below:

Generic:
[x]: 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: "BSD (3 clause)", "LGPL (v2 or later) (with incorrect FSF
     address)", "Unknown or generated". 50 files have unknown license.
     Detailed output of licensecheck in /home/nhorman/1323186-opa-
     fmgui/licensecheck.txt
<NH>
Package includes License files Third_Party_Copyright_Notices_and_Licenses.docx
and THIRD-PARTY-README which seem to relate to code which is not packaged in
this srpm.  If that is the case, then these files should not be packaged.  If it is the case, then the license needs to change in the spec file, the docx files needs to be converted to text and the binaries need to have thier licensing ennumerated.


Do I need to include a separate text file containing licensing information for  each third-party jar used by opa-fmgui?  If so, is it correct to place all of them in a documentation folder that resides in the installation directory?

Regards,
Robert

Comment 13 Neil Horman 2016-04-22 15:25:12 UTC
What you need to do is go through the code and determine which code is licensed in which way.  Your spec file indicates its all BSD, but the docs in the source tarball indicate their are multiple licenses.  You need to figure out how the code is licensed and make the spec file agree with that, following the conventions in the fedora packaging and licensing guidelines.

Comment 14 Honggang LI 2016-05-06 02:20:20 UTC
Robert, any update? thanks

Comment 15 Rick Tierney 2016-05-09 19:24:37 UTC
Hello Neil and Honggang!

My name is Rick Tierney and I will be taking over for Robert Amato as he is no longer working at Intel.

I am just ramping up on how the Fedora Packaging process works and will require some time to get up to speed.  I appreciate your patience and I will get back to you shortly.

Comment 16 Rick Tierney 2016-05-24 14:59:53 UTC
Neil or Hon:

While running 'rpmlint' against my source rpm, one of my licenses (BSD 3 clause) is not recognized.  Is it sufficient to put just BSD or do you have another suggestion?

Comment 18 Rick Tierney 2016-05-24 18:33:19 UTC
Thanks, that works!

Ok, so I have a few issues I'll need help with...

Rpmlint (4 warnings)
---------------------
1. The URL tag is supposed to point to a public web site where we have documentation, but at the moment this is unavailable. My attempts to point to the GitHub URL (https://github.com/01org/opa-fmgui/tree/Fedora_Review) have failed. Previous attempts at just using http://www.intel.com/ have also failed due to an inability to resolve these URLs.  Any suggestions or can this be ommitted?

2. The current entry for the Source0 tag is opa-fmgui-10.0.0.0.3.tar.gz, but this is supposed to be a publicly accessible URL. My predecessor (Robert Amato) has an archive at URL: https://github.com/01org/opa-fmgui/archive/v1.1.tar.gz.  When I attempted to enter this URL for the Source0 tag it couldn't be found.  Are there restrictions on the naming of this archive; i.e. does it need to match the project-version-release format? 

3. Another warning that came up was:  opa-fmgui.noarch: W: no-manual-page-for-binary fmgui
I don't have a man-page at the moment, can it be omitted?

4. The opa-fmgui is a Java application (not a library).  At run-time, the Class-Path field in MANIFEST.MF lists all of the jar-files needed to run the application.  This is the traditional way of providing path information.  This apparently goes against the guidelines set forth by Fedora and is explained by rpmlint as a problem because "these entries don't work with older Java versions and even if they do work, they are inflexible and usually cause nasty surprises".  According to the Java packaging guidelines at

           https://fedoraproject.org/wiki/Packaging:Java#No_class-path_in_MANIFEST.MF

the Class-Path can't be used, and so the "packager SHOULD use one of tools designed to locating JAR files in the system."  I'm not really sure how to handle this; i.e. Class-Path was always to goto for solving dependency issues at run-time.  Do you have a standard way of dealing with this that complies with Fedora guidelines?

Thanks!

Comment 19 Neil Horman 2016-05-24 19:08:33 UTC
The URL tag is a bit arbitrary.  rpmlint flags it for various conditions, but just pointing it to a wiki page for the project should be sufficient.


For the source URL you should be able to tag a version on github, and provide the url for it:
https://fedoraproject.org/wiki/Packaging:SourceURL

Alternatively, if you can also package an arbitrary git revision and name the package accordingly:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

Regarding the man page, yes, I can waive that when I do the review, though a man page would be nice :)

regarding classpath, I think their intent is for you to rely on the common java build tools like maven or ant.

Comment 20 Rick Tierney 2016-05-24 20:29:24 UTC
The classpath is needed for dependency resolution at run-time, whereas maven and ant are build tools only used at compile-time.  I don't currently have a solution for the removal of the Class-Path field from our Manifest file without further research. 

This issue comes up as a warning, will it be considered a failure in the review?

Comment 21 Neil Horman 2016-05-25 13:01:01 UTC
Rick, I'm not an expert on java packaging, but I think your mistaken.  Ant and maven pretty clearly, according to this, support setting a runtime classpath:
https://maven.apache.org/plugins/maven-antrun-plugin/examples/classpaths.html

Comment 22 Rick Tierney 2016-05-25 13:11:12 UTC
I started researching the issue this morning, and in addition to the link you sent, I believe there is some evidence that it can be done; but I will have to run some tests to see if I can get this working.

Thanks for the link!

Comment 23 Rick Tierney 2016-05-25 20:37:31 UTC
Neil:

1) I have reduced the number of rpmlint errors in my binary RPM to those shown below. As you can see the first 3 are out of my control because there are 3 third-party-libraries that have the wrong address for the Free Software Foundation. Am I supposed to elevate this issue to the upstream? 

rpmlint -i opa-fmgui-10.0.0.0.3-1.fc25.noarch.rpm

opa-fmgui.noarch: E: incorrect-fsf-address /usr/share/java/fmgui/lib/gritty_license.txt
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

opa-fmgui.noarch: E: incorrect-fsf-address /usr/share/java/fmgui/lib/jfreechart_license.txt
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

opa-fmgui.noarch: E: incorrect-fsf-address /usr/share/java/fmgui/lib/hibernate_license.txt
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

opa-fmgui.noarch: W: no-manual-page-for-binary fmgui
Each executable in standard binary directories should have a man page.

Still working on a solution for this...
opa-fmgui.noarch: W: class-path-in-manifest /usr/share/java/fmgui/fmgui.jar
The META-INF/MANIFEST.MF file in the jar contains a hardcoded Class-Path.
These entries do not work with older Java versions and even if they do work,
they are inflexible and usually cause nasty surprises.

1 packages and 0 specfiles checked; 3 errors, 2 warnings.

-------------------------------------------------------------------------------------------------------


2) For the licensecheck in the fmgui/ folder after installation I have a few files that don't include the license header (See below).  Is that required for every file?

licensecheck -r fmgui

fmgui/util/fmguiclear.sh: *No copyright* UNKNOWN
fmgui/util/postsetup.sh: *No copyright* UNKNOWN
fmgui/README: *No copyright* GENERATED FILE
fmgui/THIRD-PARTY-README: *No copyright* UNKNOWN
fmgui/help/LICENSE: BSD (3 clause)

Comment 24 Neil Horman 2016-05-26 13:11:41 UTC
I'm not worried about the license address, no, and would waive them, though you should contact upstream for those projects and provide them with patches to fix that.

What worries me  more is how you incorporated those projects to yours.  While theres no license issue with doing so, of the three packages that you incorporate:
jfreechart is already independently packaged for fedora
hibernate is already independently packaged for fedora
gritty is not packaged, but easily could be

I'm sorry I didn't notice your 3rd party incorporations previously, but as a rule, you may not do that.  separate projects need to be packaged independently so that other applications can use them and pick up updates automatically.  At a minimum you need to not build or package the jfreechart and hibernate code, instead creating a BuildRequires for the corresponding existing packages, so those are used properly.  Ideally you would also package gritty separately, but I can likely waive that as its not currently packaged.

Comment 25 Rick Tierney 2016-05-26 13:42:07 UTC
Created attachment 1162001 [details]
opa-fmgui spec file

I'm a bit unsure of your use of the term "incorporations", so let me clarify... 

The only 3rd party library for which we include the source code is gritty.  All other 3rd party libraries are being pulled in using BuildRequires in the spec file (see attached).

Binary RPM
----------
Jar-files for these libraries are being placed in the lib/ folder along with their licenses files.

Source RPM
----------
Only opa-fmgui and gritty source code is included in the SRPM

Gritty
------
It looks like the Gritty project was cancelled as shown on the GitHub website here: https://github.com/walterDurin/gritty/tree/master.  I did find it in the Google Code Archive here: https://code.google.com/archive/p/gritty/downloads, but since it hasn't been packaged for Fedora it needed to be built.

Is this an acceptable way to package our project?

Comment 26 Neil Horman 2016-05-26 13:51:20 UTC
oh, you're right, my bad.  I saw the license issues you put up and assumed you were using them direct from your project.  You're good on hibernate and jfreechart

As for gritty, I concur with your approach.  Ideally it should be packaged independently, but given that upstream development is halted, I'd be ok with your current approach, though I would recommend looking for an alternate implementation, just in case something breaks

Ok, Upload your new spec and srpm when you're ready and I'll give it a review.  Thanks!
Neil

Comment 27 Rick Tierney 2016-05-26 15:14:27 UTC
Great, I'm glad that we cleared that up! We don't have immediate plans to replace gritty, but it is likely that we will implement our own version of this functionality in the future.

Thanks!

Comment 28 Rick Tierney 2016-05-26 20:22:10 UTC
From Comment #12, you wrote:
<NH>
Package includes License files Third_Party_Copyright_Notices_and_Licenses.docx
and THIRD-PARTY-README which seem to relate to code which is not packaged in
this srpm.  If that is the case, then these files should not be packaged.  If it is the case, then the license needs to change in the spec file, the docx files needs to be converted to text and the binaries need to have thier licensing ennumerated.


(In reply to Neil Horman from comment #13)
> What you need to do is go through the code and determine which code is
> licensed in which way.  Your spec file indicates its all BSD, but the docs
> in the source tarball indicate their are multiple licenses.  You need to
> figure out how the code is licensed and make the spec file agree with that,
> following the conventions in the fedora packaging and licensing guidelines.

Neil:
The above statements come from comments 12 and 13 and were in response to Robert Amato's questions regarding license files. I'm trying to determine the correct course of action regarding whether to include certain files or not.  The driving force here is to comply with Fedora Packaging Guidelines AND satisfy criteria set forth by the Intel Legal Dept for purposes that go beyond the Fedora Packaging process.  If these two criteria conflict, then I will have to track two separate branches and I'd prefer not to do that if possible.

On the one hand, our Legal team has instructed us to:
1. Include all the 3rd Party license files in same folder as the jar files for the binary RPM. 
2. Include the following in both RPM and SRPM:
   a. Third_Party_Copyright_Notices_and_Licenses: Contains ALL applicable licenses for 3rd parties and more. This was originially a MS-Word document that I changed to a text file.
   b. LICENSE: BSD (3-clause) for opa-fmgui
   c. THIRD-PARTY-README: Listing of 3rd party libraries and the location of their license files

On the other hand, based on your answers to previous questions we should:
   a. Remove the Third_Party_Copyright_Notices_and_Licenses and THIRD-PARTY-README files because their source code isn't packaged
   b. And this part isn't clear to me... whether the license files should be in the RPM lib/ folder with jar files or not. If it is okay to do this then I would rather leave it this way to comply with Intel Legal.

I want to make sure I have everything in the right place and omit what is not permitted.  Could you weigh in on this?

Thanks!
Rick

Comment 29 Rick Tierney 2016-05-26 20:25:25 UTC
(In reply to Rick Tierney from comment #28)
> From Comment #12, you wrote:
> <NH>
> Package includes License files
> Third_Party_Copyright_Notices_and_Licenses.docx
> and THIRD-PARTY-README which seem to relate to code which is not packaged in
> this srpm.  If that is the case, then these files should not be packaged. 
> If it is the case, then the license needs to change in the spec file, the
> docx files needs to be converted to text and the binaries need to have thier
> licensing ennumerated.
> 
> 
> (In reply to Neil Horman from comment #13)
> > What you need to do is go through the code and determine which code is
> > licensed in which way.  Your spec file indicates its all BSD, but the docs
> > in the source tarball indicate their are multiple licenses.  You need to
> > figure out how the code is licensed and make the spec file agree with that,
> > following the conventions in the fedora packaging and licensing guidelines.
> 
> Neil:
> The above statements come from comments 12 and 13 and were in response to
> Robert Amato's questions regarding license files. I'm trying to determine
> the correct course of action regarding whether to include certain files or
> not.  The driving force here is to comply with Fedora Packaging Guidelines
> AND satisfy criteria set forth by the Intel Legal Dept for purposes that go
> beyond the Fedora Packaging process.  If these two criteria conflict, then I
> will have to track two separate branches and I'd prefer not to do that if
> possible.
> 
> On the one hand, our Legal team has instructed us to:
> 1. Include all the 3rd Party license files in same folder as the jar files
> for the binary RPM. 
> 2. Include the following in both RPM and SRPM:
>    a. Third_Party_Copyright_Notices_and_Licenses: Contains ALL applicable
> licenses for 3rd parties and more. This was originially a MS-Word document
> that I changed to a text file.
>    b. LICENSE: BSD (3-clause) for opa-fmgui
>    c. THIRD-PARTY-README: Listing of 3rd party libraries and the location of
> their license files
> 
> On the other hand, based on your answers to previous questions we should:
>    a. Remove the Third_Party_Copyright_Notices_and_Licenses and
> THIRD-PARTY-README files because their source code isn't packaged
>    b. And this part isn't clear to me... whether the license files should be
> in the RPM lib/ folder with jar files or not. If it is okay to do this then
> I would rather leave it this way to comply with Intel Legal.
> 
> I want to make sure I have everything in the right place and omit what is
> not permitted.  Could you weigh in on this?
> 
> Thanks!
> Rick

Addendum:
I have figured out all of the licenses that are being used and updated the License tag in the spec file. I think that part of the issue is complete.

Comment 30 Neil Horman 2016-05-26 20:46:53 UTC
Thank you.  Regarding the license files, I'm not 100% sure what to do in this case.  I did note that they needed to be updated to reflect the distibuting choice of license previously, but that was before I understood that you weren't actually using them.  Given that, it seems safe to not change them in any way.  I suppose the most sane action would be to simply not include the source for those libraries in the upstream project at all, because you'll pull them in via rpm dependencies.  Then you know you don't need to mess with their licenses at all, because you won't need to include them

Additionally a minor nit, The license files should be tagged as %license in the spec file so that they get dropped in the /usr/share/doc/license subdirectory.

Comment 31 Rick Tierney 2016-05-26 21:01:40 UTC
Created attachment 1162275 [details]
opa-fmgui spec file

So am I to understand that if we are not including the source code for a 3rd party library (except gritty) and just pulling them in via BuildRequires, then we don't need to include the license files anywhere (neither RPM nor SRPM)?

Incidentally, I tagged all the license files with %license about an hour ago.  But they reside in the lib/ folder with jar files instead of the /usr/share/doc/license subdirectory.

Comment 32 Neil Horman 2016-05-27 13:31:03 UTC
Rick, ideally it would be better if you just removed the libraries that you are pulling in via build requires from the project alltogether, but given that the license tag is supposed to apply to the binary rpm, I suppose its ok for them just to be there in the source tarball without the license included in the %files section if you have to do it that way (though I am not a lawyer)

Regarding the license location, thats odd.  Perhaps thats where java packages them, nomnially tagging a license file moves it to /usr/share/doc/licenses.  But as long as youve tagged the file I think we're good

Comment 33 Rick Tierney 2016-05-28 00:07:59 UTC
Thanks Neil!  I will review this with our Legal team but it sounds like we should remove the 3rd party jar files after we build opa-fmgui and get rid of the licenses altogether.  This surprises me a little because since we are using other people's libraries (even if it is just the binaries) I thought we would have to include the licenses in our package.  I guess I should go back and review the Fedora Packaging Licensing Guidelines again.

Comment 34 Neil Horman 2016-05-31 00:24:40 UTC
Well, just to be clear, what I'm saying is that both the binaries and the licenses should be removed, so you're not really using the 3rd party jar file at all.  The only exception is the gritty jar file, for which you will need to keep the license file, since I'm willing to waive separate packaging on that.

Comment 35 Rick Tierney 2016-05-31 13:48:18 UTC
Neil:

My bad, the last message I posted was misleading.  Library jars MUST remain present in order for the application to run!  We cannot remove the jars from the binary RPM otherwise opa-fmgui won't work at all.  

So given that critical information, the questions become:

1. Does Fedora have any rules against this type of 3rd Party jar file usage?

2. Are the 3rd Party licenses for the jar files required when using them this way?

3. Where should the 3rd Party licenses be stored? RPM, SRPM, both?

Comment 36 Rick Tierney 2016-05-31 20:07:07 UTC
(In reply to Rick Tierney from comment #35)
> Neil:
> 
> My bad, the last message I posted was misleading.  Library jars MUST remain
> present in order for the application to run!  We cannot remove the jars from
> the binary RPM otherwise opa-fmgui won't work at all.  
> 
> So given that critical information, the questions become:
> 
> 1. Does Fedora have any rules against this type of 3rd Party jar file usage?
> 
> 2. Are the 3rd Party licenses for the jar files required when using them
> this way?
> 
> 3. Where should the 3rd Party licenses be stored? RPM, SRPM, both?

Perhaps I'm not being completely clear... these jar files contain binary .class files which contribute to the proper execution of the opa-fmgui application. NONE of these jar files (including gritty.jar) contain source code, and I'm wondering if they need to be accompanied by license files.

The Gritty source code is a separate issue, and since it is not packaged separately we are building that ourselves and it is accompanied by a license file.

Comment 37 Neil Horman 2016-05-31 20:34:34 UTC
yes, Rick, I know what jar files are.  Let me try to be very clear in what I'm saying:

opa-fmgui, when built contains three categories of jar files:

1) Jar files built from the sources that are unique to opa-fmgui

2) Jar files that are built from sources that you include in the upstream opa-fmgui source repository, but are available independently as their own fedora packages (hibernate and jfreechart fall into this category)

3) Jar files that are built from sources that you include in the upstream opa-fmgui source repository, and are _not_ independently available as their own fedora packages (gritty falls into this category)



Category 1 Jar files are a no-brainer.  They should be licenced with a fedora approved license, and that license should be included in the package.  I don't think we have any disagreement here

Category 2 Jar files, are what I'm driving at. You shouldn't need those jar files at all, because the jar files can be made available via the Requires tag in the spec file.  That is to say, you can require that the hibernate and jfreechart be installed when the opa-fmgui package is installed, so that the jar files from the hibernate and jfreechart packages are used at run time when opa-fmgui is run. As such, you don't need to build those jars.  You can remove them from your build, and in so doing, you don't need to include the license with them, because you're not (or shouldn't be) packaging them.

Category 3 jar files, I'm willing to give you a waiver on.  gritty I would normally say needs to be separately packaged, but because the upstream package is not being actively developed, I don't expect there will be a great need for it, and its just as likely you will do your own internal fixes to it going forward.  You will need to include the license file for gritty of course, and it will need to be an approved open source license (like GPL), but as long as it is, that should be fine.


If there is a 4th category of jar file I'm unaware of in the project, we will definitely have to address that as well, but I don't see any at the moment

Comment 38 Rick Tierney 2016-05-31 21:10:03 UTC
Sorry Neil I didn't mean to imply you didn't know what jar files were. I was just trying to get to the bottom of the misunderstanding.

In any case, I understand your point of view and I will attempt to install the 3rd party libraries and remove the lib/ folder containing the jar files and licenses.

Sorry for the confusion!

Comment 39 Neil Horman 2016-06-01 02:04:08 UTC
I'm sorry rick, its been a day.

If thats what you're doing, I presume then my understanding is accurate.  If thats the case, great, if not, please let me know, I want to be sure we do the right thing here.

Comment 40 Rick Tierney 2016-06-01 16:10:13 UTC
Neil:

I discussed the new strategy with my team and everyone believes that this approach would be "user unfriendly".  Here are some things to consider:

1. Specifying "Requires" in the spec file only indicates what libraries are required during the run-time execution of opa-fmgui; it doesn't automatically install them.  So if they aren't already installed, an error message will be displayed when attempting to run opa-fmgui. The user will then have to manually install the missing 3rd party libraries making this a hardship on the user; and this can be as many as 35 jar files in the worst case scenario.

2. If Fedora were to ensure that the correct versions of all the 3rd party libraries that we need are already installed as part of the Fedora release, this is "more user friendly"... but if the user comes along with another application that requires a different version of a 3rd party library than what we are using, it could potentially render opa-fmgui useless since we haven't tested it under the new version of the library.

3. If we package the 3rd party jars in the lib/ folder under the binary RPM and specify the class path to point specifically to that folder, the user is guaranteed to have a working opa-fmgui regardless of what other versions of 3rd party libraries are installed on the system.

Based on what we have discussed previously about eliminating the jar files, is it safe to say this is just a preference rather than a mandate?  If it is allowable, we would like to continue packaging the jar files in the binary RPM.

Comment 41 Jarod Wilson 2016-06-01 18:01:19 UTC
"Requires" means "you cannot install this package without this/these packages already installed, unless you force it". That is 100% by design. That's what we have things like yum and dnf for -- they automatically sort out dependencies and install them as needed. 'dnf install opa-fmgui' would install all the Requires packages.

No, you can't bundle a bunch of 3rd party jar files that are already packaged separately in Fedora into your package.

#2 is valid, it's possible a required package gets updated and breaks things, but welcome to open source. :)

Comment 42 Don Dutile (Red Hat) 2016-06-01 18:12:07 UTC
+1 to what Jarod said.

Comment 43 Rick Tierney 2016-06-07 21:00:10 UTC
Created attachment 1165764 [details]
FMGUI Spec File

I have removed the jar files from the opa-fmgui packaging as advised, and added "Requires" tags for all of the 3rd party libraries that are needed.  

In the testing of the installation I am not using dnf because opa-fmgui has not yet been deployed to the Fedora Repo. So I install the RPM directly using the following command:
     'sudo rpm -i opa-fmgui-10.0.0.0.3-3.fc25.noarch.rpm'

This results in the following output now that I have removed the jar files:
      error: Failed dependencies:
	antlr = 2.7.7 is needed by opa-fmgui-10.0.0.0.3-3.fc25.noarch
	dom4j = 1.6.1 is needed by opa-fmgui-10.0.0.0.3-3.fc25.noarch
	hibernate-commons-annotations = 4.0.4.Final is needed by opa-fmgui-10.0.0.0.3-3.fc25.noarch
	hibernate-core = 4.3.5.Final is needed by opa-fmgui-10.0.0.0.3-3.fc25.noarch
	hibernate-entitymanager = 4.3.5.Final is needed by opa-fmgui-10.0.0.0.3-3.fc25.noarch
	hibernate-jpa-2.1-api = 1.0.0.Final is needed by opa-fmgui-10.0.0.0.3-3.fc25.noarch
                                           .
                                           .
                                           .
	swingx-testsupport = 1.6.5 is needed by opa-fmgui-10.0.0.0.3-3.fc25.noarch

I have been researching the documentation trying to understand how to get these 3rd party jar files to install automatically prior to the installation the opa-fmgui RPM but I am confused.  I have also been wondering how to use the %jpackage_script in the spec file to set this up (and whether this is necessary).

Any help you can provide would be most appreciated.

attached: opa-fmgui.spec

Rick

Comment 44 Jarod Wilson 2016-06-07 21:24:42 UTC
dnf localinstall opa-fmgui-10.0.0.0.3-3.fc25.noarch.rpm

Comment 45 Rick Tierney 2016-06-10 21:12:29 UTC
Got it, thank you!

Comment 46 Rick Tierney 2016-06-21 17:15:41 UTC
Spec URL: https://github.com/01org/opa-fmgui/releases/download/v1.4/opa-fmgui.spec

SRPM URL: https://github.com/01org/opa-fmgui/releases/download/v1.4/opa-fmgui-10.0.0.0.3-4.fc25.src.rpm

Description: The opa-fmgui is a Java GUI for the Intel Omni-Path Architecture computing fabric

Fedora Account System Username: rjtierne

Comment 47 Rick Tierney 2016-06-21 17:26:49 UTC
Neil:

I tried to run the fedora-review using the information above in this BZ. It reported back an error saying "Cannot find source rpm URL".

Since that didn't work, I made a new request (and consequently new BZ) using this link: 

https://bugzilla.redhat.com/enter_bug.cgi?product=Fedora&format=fedora-review

Looks like I can't run the review using the BZ from here, so the new BZ is: https://bugzilla.redhat.com/show_bug.cgi?id=1348668

Rick

Comment 48 Neil Horman 2016-06-21 17:51:00 UTC
*** Bug 1348668 has been marked as a duplicate of this bug. ***

Comment 49 Neil Horman 2016-06-21 17:53:22 UTC
I'm not sure what problem you ran into but its working fine for me.  I've closed your new bz as a dup of this one, and will continue the review here

Comment 50 Rick Tierney 2016-06-21 17:56:06 UTC
I tried to run fedora-review -b 1323186, but it couldn't find the SRPM URL. But if you can run it, that's fine!

Thanks!

Comment 51 Neil Horman 2016-06-21 18:36:37 UTC
Looks pretty good, a few minor nits and I think we're good to go.


Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package does not contain duplicates in %files.
  Note: warning: File listed twice: /usr/share/java/opa-fmgui/LICENSE
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles
- This seems like a Java package, please install fedora-review-plugin-java
  to get additional checks


===== MUST items =====

Generic:
[x]: 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: "BSD (3 clause)", "BSD (3 clause) MIT/X11 (BSD like)", "Unknown
     or generated", "LGPL (v2 or later) (with incorrect FSF address)". 49
     files have unknown license. Detailed output of licensecheck in
     /home/nhorman/1323186-opa-fmgui/licensecheck.txt
<NH> Spec file indicates that your license is:
GPLv2 and LGPLv2+ and MIT and BSD
But the license check on the code doesn't include MIT licensed bits.  Can you
either remove the MIT bit here, or point out what I'm missing? :)

[!]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
<NH> This is asking you to put a comment in the code under the files section to
ennumerate which binaries get which license. Though I think opa-fmgui is the
only relevant one here, and it has code from all three licenses, no?

[x ]: Package must own all directories that it creates.
     Note: Directories without known owners: /etc/xdg/menus,
     /etc/profile.d, /etc/xdg/menus/applications-merged
[x ]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/icons/hicolor
     (hicolor-icon-theme, fedora-logos)
[x ]: Package contains no bundled libraries without FPC exception.
[x ]: Changelog in prescribed format.
[x ]: Sources contain only permissible code or content.
[x ]: Development files must be in a -devel package
[! ]: Package uses nothing in %doc for runtime.
<NH> Spec file should mark the readme and third-party-readme as %doc

[x ]: Package consistently uses macros (instead of hard-coded directory
     names).
[x ]: Package is named according to the Package Naming Guidelines.
[x ]: Package does not generate any conflict.
[x ]: Package obeys FHS, except libexecdir and /usr/target.
[- ]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x ]: Requires correct, justified where necessary.
[x ]: Spec file is legible and written in American English.
[- ]: Package contains systemd file(s) if in need.
[x ]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
     contains icons.
     Note: icons in opa-fmgui
[x ]: Package is not known to require an ExcludeArch tag.
[x ]: 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]: %config files are marked noreplace or the reason is justified.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install or
     desktop-file-validate if there is such a file.
[x]: Dist tag is present.
[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]: No %config files under /usr.
[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]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Bundled jar/class files should be removed before build

===== SHOULD items =====

Generic:
[x ]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x ]: Final provides and requires are sane (see attachments).
[x ]: Package functions as described.
[x ]: Latest version is packaged.
[x ]: Package does not include license text files separate from upstream.
[x ]: SourceX tarball generation or download is documented.
     Note: Package contains tarball without URL, check comments
[x ]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x ]: Package should compile and build into binary rpms on all supported
     architectures.
[- ]: %check is present and all tests pass.
[x ]: 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]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: opa-fmgui-10.0.0.0.3-4.fc25.noarch.rpm
          opa-fmgui-10.0.0.0.3-4.fc25.src.rpm
opa-fmgui.noarch: W: no-documentation
opa-fmgui.noarch: E: incorrect-fsf-address /usr/share/java/opa-fmgui/gritty/gritty_license.txt
opa-fmgui.noarch: W: no-manual-page-for-binary opa-fmgui
opa-fmgui.src: W: invalid-url Source0: opa-fmgui-10.0.0.0.3.tar.gz
2 packages and 0 specfiles checked; 1 errors, 3 warnings.




Rpmlint (installed packages)
----------------------------
opa-fmgui.noarch: W: no-documentation
opa-fmgui.noarch: E: incorrect-fsf-address /usr/share/java/opa-fmgui/gritty/gritty_license.txt
opa-fmgui.noarch: W: no-manual-page-for-binary opa-fmgui
1 packages and 0 specfiles checked; 1 errors, 2 warnings.



Requires
--------
opa-fmgui (rpmlib, GLIBC filtered):
    /bin/bash
    /bin/sh
    config(opa-fmgui)
    jre
    mvn(antlr:antlr)
    mvn(ch.qos.logback:logback-classic)
    mvn(ch.qos.logback:logback-core)
    mvn(com.jcraft:jsch)
    mvn(com.mxgraph:jgraphx)
    mvn(com.sun.mail:javax.mail)
    mvn(dom4j:dom4j)
    mvn(javax.help:javahelp)
    mvn(log4j:log4j)
    mvn(net.engio:mbassador)
    mvn(org.hibernate.common:hibernate-commons-annotations)
    mvn(org.hibernate.javax.persistence:hibernate-jpa-2.1-api)
    mvn(org.hibernate:hibernate-core)
    mvn(org.hibernate:hibernate-entitymanager)
    mvn(org.hsqldb:hsqldb)
    mvn(org.javassist:javassist)
    mvn(org.jboss.logging:jboss-logging)
    mvn(org.jboss.logging:jboss-logging-annotations)
    mvn(org.jboss.spec.javax.transaction:jboss-transaction-api_1.2_spec)
    mvn(org.jboss:jandex)
    mvn(org.jfree:jcommon)
    mvn(org.jfree:jfreechart)
    mvn(org.slf4j:log4j-over-slf4j)
    mvn(org.slf4j:slf4j-api)
    mvn(org.swinglabs.swingx:swingx-all)



Provides
--------
opa-fmgui:
    application()
    application(fmgui.desktop)
    config(opa-fmgui)
    opa-fmgui



Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1323186
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, Shell-api, Java
Disabled plugins: C/C++, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP
Disabled flags: EXARCH, DISTTAG, EPEL5, BATCH, EPEL6

Comment 52 Rick Tierney 2016-06-21 19:05:10 UTC
Neil:

Now that you have run the review (which seems slightly different from the way I do), I can see where some things are wrong and some things I'm not sure why they're being flagged:

1. - Package does not contain duplicates in %files.
     Note: warning: File listed twice: /usr/share/java/opa-fmgui/LICENSE
<RT>The "LICENSE" file appears on the install line and the %license line, but I don't see that as a     duplicate when I run the review - is that wrong?

2. License field in the package spec file matches the actual license.
<RT> This one is important, and I forgot to change it!  Now that we are no longer distributing 3rd party libraries (except for Gritty, which is LGPL) shouldn't we only list the licenses for opa-fmgui and gritty?  If that's the case then the License tag should say BSD and LGPL.

3.[!]: If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.
<NH> This is asking you to put a comment in the code under the files section to ennumerate which binaries get which license. Though I think opa-fmgui is the only relevant one here, and it has code from all three licenses, no?
<RT> I put a comment right above the License: tag stating that a license break-down can be found in the file THIRD-PARTY-README. The MIT license was for SLF4J and MBassador which are no longer included in our package. So it sounds like this comment needs to move under the %file section and maybe it's only relevant to gritty since that's the only one being built and included here. Not sure! I thought it would be more complete if the THIRD-PARTY-README file contained a license breakdown of ALL the libraries we are using.

4. [! ]: Package uses nothing in %doc for runtime.
<NH> Spec file should mark the readme and third-party-readme as %doc
<RT> I played with this before and kept getting warnings about file duplication. Maybe I can mark these with the %doc macro and they will get installed without using the "install" command. I can try that.

Comment 53 Doug Ledford 2016-06-21 19:15:35 UTC
(In reply to Rick Tierney from comment #52)
> Neil:
> 
> Now that you have run the review (which seems slightly different from the
> way I do), I can see where some things are wrong and some things I'm not
> sure why they're being flagged:
> 
> 1. - Package does not contain duplicates in %files.
>      Note: warning: File listed twice: /usr/share/java/opa-fmgui/LICENSE
> <RT>The "LICENSE" file appears on the install line and the %license line,
> but I don't see that as a     duplicate when I run the review - is that
> wrong?

> 4. [! ]: Package uses nothing in %doc for runtime.
> <NH> Spec file should mark the readme and third-party-readme as %doc
> <RT> I played with this before and kept getting warnings about file
> duplication. Maybe I can mark these with the %doc macro and they will get
> installed without using the "install" command. I can try that.

This is the same issue: you have a minor misconception about the error message about files being listed more than once.  The problem is that you have the files listed individually in the RPM via the %doc or other macro like that, then you also have them getting found by glob in the %files section of the main package.  Most likely your %files includes /usr/share/java/opa-fmgui as a directory, which then pulls in all files in that directory, and as a result any files under there that are also listed in things like %doc macros will now be listed in two places and then you get the warning.

Comment 54 Rick Tierney 2016-06-21 19:19:55 UTC
I see! Yes I do have /usr/share/java/opa-fmgui under %files... so that's why the duplicates. I will revise this and try again. Thanks!

Comment 55 Rick Tierney 2016-06-22 12:50:35 UTC
Removing the folder /usr/share/java/opa-fmgui from %files does clear up the file duplication problem, but as a side-effect this folder doesn't get removed during uninstallation.  

I suspect I could remove it as part of a %postun, but is there a recommended way to remove this folder?

Comment 56 Doug Ledford 2016-06-22 13:58:57 UTC
Looking at your spec file, you are placing your license files and other non-runtime files in your runtime java directory.  I don't know if that's standard or not.  But there are a couple ways of solving things.

Option 1: Use the %doc macro.  The files you specify with the %doc macro will not be placed in your app directory, they will end up in /usr/share/doc/<package-name>-<version>/.  The %doc macro is part of your %files stanza, and it copies the files from the source build tree to the final destination, so you don't need to manually install any files that get the %doc macro.  It would look something like this:

%files
%doc README THIRD-PARTY-README Third_Party_Copyright_Notices_and_Licenses

I'm not entirely sure that the license macro doesn't work similarly...ok, I just checked, and it does.  You don't need to install the files that get the %license macro, the macro installs the files.  So, after the %doc macro, do:

%license LICENSE gritty/gritty_license.txt

and it will copy the files from the source dir to the right target location.

After you do that, you can then re-include the parent directory for the java app and it will get removed on uninstall as now all of these files will be in their proper location, and that won't be in the java app directory, so there will be no listing of the files twice.

Option 2: Use the %exclude macro in the %files section and don't fix things like I listed above.  But, as I don't think that's the right fix here, don't use it, I just list the %exclude macro so you are aware of it.  If you have 50 commands in /usr/bin, and 48 of them belonged in the base package, but only 2 belonged to the -devel subpacakge, then it's easiest in the main package to do

%files
/usr/bin/*
%exclude /usr/bin/command1
%exclude /usr/bin/command2

%files -devel
/usr/bin/command1
/usr/bin/command2

Given that the %license and %doc macros both add the files to the internal file lists, if those files were still in an otherwise managed location, %exclude can get them off of the other list.

Comment 57 Rick Tierney 2016-06-22 17:26:05 UTC
Thanks Doug, that's a great explanation!  

I followed Option 1 and saw that the documentation was copied to /usr/share/doc/opa-fmgui and the licenses were copied to /usr/share/licenses/opa-fmgui. After restoring the parent folder under %files and removing all the non-runtime files from being installed it worked much better; and un-installation completely removed everything.

I will submit this for review again shortly.

Comment 58 Rick Tierney 2016-06-23 20:02:23 UTC
Request for Review...

Spec URL: https://github.com/01org/opa-fmgui/releases/download/v1.6/opa-fmgui.spec

SRPM URL: https://github.com/01org/opa-fmgui/releases/download/v1.6/opa-fmgui-10.0.0.0.3-6.fc25.src.rpm

Description: The opa-fmgui is a Java GUI for the Intel Omni-Path Architecture computing fabric

Fedora Account System Username: rjtierne

Comment 59 Neil Horman 2016-06-24 14:01:34 UTC
Looks like you fixed everything well.  Approved.

Comment 60 Rick Tierney 2016-06-24 14:32:23 UTC
Great! It was a long road, but I'm glad it's done!
Thanks to everyone for all of your help!

Rick

Comment 61 Rick Tierney 2016-07-11 11:18:12 UTC
Neil:

I keep receiving e-mails indicating that "a user has been waiting more than 7 days for a response from me" regarding this BZ.  Is there something else I need to do for this to be closed?

Rick

Comment 62 Rick Tierney 2016-07-11 11:38:26 UTC
(In reply to Rick Tierney from comment #61)
> Neil:
> 
> I keep receiving e-mails indicating that "a user has been waiting more than
> 7 days for a response from me" regarding this BZ.  Is there something else I
> need to do for this to be closed?
> 
> Rick

In addition, I will probably have to ask for additional reviews in the near future, but I assume that could be done under a new BZ.

Comment 63 Neil Horman 2016-07-11 14:27:10 UTC
Yes, Rick, its waiting for you to do the dist-git checkin of the package, clear the needinfo and close the bz.

And yes, new reviews should be done under a new bz

Comment 64 Rick Tierney 2016-07-11 14:50:15 UTC
Ok, I wasn't aware of this step; I'm assuming you are referring to this: https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Update_Your_Branches_.28if_desired.29

If not please advise!

Comment 65 Neil Horman 2016-07-11 18:42:34 UTC
I'm referring to this:
https://fedoraproject.org/wiki/Package_Review_Process

Specifically steps 8-15 in the contributor section of that process

Comment 66 Rick Tierney 2016-07-14 16:10:15 UTC
Neil:

I'm working on getting sponsored into the Packagers Group, but have discovered that additional changes will be required before the final submission to Fedora.  

Is it okay to continue using this BZ to request additional reviews?

Comment 67 Neil Horman 2016-07-14 17:51:35 UTC
Shoot, I meant to sponsor you, I'll take care of that Now.

as for the package review,  You shouldn't need any more changes.  What is it you feel needs changing?

Comment 68 Neil Horman 2016-07-14 17:54:31 UTC
Additionally, what is your fedora account user name?  It needs to be the same as your bugzilla account, but I don't seem to find it.

Comment 69 Rick Tierney 2016-07-14 18:05:04 UTC

(In reply to Neil Horman from comment #67)
> Shoot, I meant to sponsor you, I'll take care of that Now.
> 
> as for the package review,  You shouldn't need any more changes.  What is it
> you feel needs changing?

Neil:

I had been trying to figure out sponsorship and one of my guys here at Intel contacted Michael Schmidt to get the ball rolling. That was yesterday, but I haven't heard back yet.

I have 2 changes I need to make:

1. Update the Version number to match our source version number. My predecessors were using a strange number I never understood but I continued to use it during my development. Now that we're ready to deploy this I need to use the correct number.

2. The Intel legal team wants our license files readily available in the application folder.  I updated a %postun script to create symbolic links to the license files in /usr/licenses/opa-fmgui

These changes are pretty minor, but they do need to get put in there before we push to Fedora.

Rick

Comment 70 Rick Tierney 2016-07-14 18:09:28 UTC
(In reply to Neil Horman from comment #68)
> Additionally, what is your fedora account user name?  It needs to be the
> same as your bugzilla account, but I don't seem to find it.

This is strange! My account user name is rjtierne, which I have seen in on the Koji Web server. But now when I try to log into my account at:

https://fedoraproject.org/w/index.php?title=Special:UserLogin&returnto=Join+the+package+collection+maintainers

It won't let me in... looking for a way to reset my password.

Comment 71 Rick Tierney 2016-07-14 18:13:09 UTC
(In reply to Rick Tierney from comment #70)
> (In reply to Neil Horman from comment #68)
> > Additionally, what is your fedora account user name?  It needs to be the
> > same as your bugzilla account, but I don't seem to find it.
> 
> This is strange! My account user name is rjtierne, which I have seen in on
> the Koji Web server. But now when I try to log into my account at:
> 
> https://fedoraproject.org/w/index.php?title=Special:
> UserLogin&returnto=Join+the+package+collection+maintainers
> 
> It won't let me in... looking for a way to reset my password.


Okay, it turns out I have to submitt an SSH key for some Fedora resources to become available.  I let you know when I have fixed this.

Comment 72 Rick Tierney 2016-07-14 18:32:11 UTC
Neil:

Not sure how it happened but this is what I'm seeing:
1. Fedora Account: username=rjtierne
   - I can log into the Fedora Account System at:
     https://admin.fedoraproject.org/accounts

   - I cannot log into the Package Maintainers Site at:
     https://fedoraproject.org/w/index.php?title=Special:UserLogin&returnto=Join+the+package+collection+maintainers

2. Bugzilla Account: username=rick.tierney
I was certain I deliberately made this username the same as Fedora but I guess I messed it up somewhere along the way.

Rick

Comment 73 Jarod Wilson 2016-07-18 14:41:16 UTC
Your bugzilla login doesn't need to match the FAS login, it needs to match the email address on file with FAS tied to your login. Bugzilla login usernames are generally (always) email addresses, FAS uses usernames, but has email addresses then associated with them. So as long as you have rick.tierney associated with your FAS account, you should be all set on that front.

Comment 74 Rick Tierney 2016-07-18 14:43:44 UTC
Okay Jarod, thanks!  I do have the e-mail rick.tierney set on both the Fedora and Bugzilla accounts.

Comment 75 Neil Horman 2016-07-18 15:43:41 UTC
ok, I've sponsored you into the packager group, though I still would like you to ennumerate what changes you are putting into the package prior to committing it, please

Comment 76 Rick Tierney 2016-07-18 15:52:30 UTC
Great!  But as mentioned in Comment 69 above:

I have 2 changes I need to make:

1. I Updated the Version and Release numbers in the spec file to match our source version/release numbers. My predecessors were using a strange number I never understood but I continued to use it during my development. Now that we're ready to deploy this I need to use the correct version/release number.

2. The Intel legal team wants our license files to be readily available in the application folder.  While I have left the license files where they were (i.e. /usr/share/licenses/opa-fmgui, I have updated a %post script (buildlinks) to create symbolic links to the license files in the application folder.  This way I meet the RPM standards of marking the license files as %license and I can keep our legal team satisfied that the licenses are readily accessible by the user.

These changes are pretty minor, but they do need to get put in there before we push to Fedora.

Should we do a new review?

Comment 77 Neil Horman 2016-07-18 17:37:36 UTC
No, we don't need to do a new reivew, but these would have been good changes to make during the initial review.

I assume you're changing the version to 1.7 then?  If so, thats fine.

Regarding the license file, that really doesn't make any sense to me.  Are you saying that your legal team wants the license to be co-located with the application binary?  That seems fairly silly to me.  To suggest that they are not readily available in the license area of the filesystem, but are readily available if they are in the same directory as the binary suggests that they don't really understand how rpm installation works.  Strictly speaking, adding the symlinks that you suggest violates the FHS (http://www.pathname.com/fhs/), to which fedora adheres.  Please don't add those symlinks there, as it co-mingles applications and ancillary content.

Comment 78 Rick Tierney 2016-07-19 11:38:51 UTC
I described the RPM installation process to our legal team and they are fine with the license residing only in the license folder.  I will not add symlinks in the application folder.

Comment 79 Neil Horman 2016-07-19 13:25:59 UTC
Thank you Rick.  You're approved as a sponsor now, so you should be able to complete the import section of the packaging process and close this bz

Comment 80 Michal Schmidt 2016-07-19 14:22:32 UTC
(In reply to Neil Horman from comment #79)
> Thank you Rick.  You're approved as a sponsor now

Correction: s/sponsor/packager/

Comment 81 Rick Tierney 2016-07-19 14:24:33 UTC
Got it, no problem! I'll be working on that next.
Thanks!

Comment 82 Rick Tierney 2016-07-22 11:41:00 UTC
Neil:

Yesterday, I followed the guidelines and made a request for a new package under the Fedora Package Database, but the status is "Denied: No review.", and I haven't received any e-mail notification as to why it was denied.

Would you see if you can figure out what went wrong?

Rick

Comment 83 Michal Schmidt 2016-07-22 13:49:53 UTC
Rick,

I'm only guessing what could go wrong.
Did you put the correct BZ number (1323186) in the "Ticket number or URL" field and press the Sync button?
"Namespace" should be "rpms".

Michal

Comment 84 Rick Tierney 2016-07-22 15:02:05 UTC
Hi Michal:

It wasn't clear what URL to put in that field so I put the wrong value there; I will try again!  Thanks!

Rick

Comment 85 Gwyn Ciesla 2016-07-22 16:01:21 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/opa-fmgui

Comment 86 Rick Tierney 2016-07-25 13:20:14 UTC
I'm at the last stages of this BZ, but have been unable to push my changes to pkgs.fedoraproject.org.

I followed the procedure starting here: 
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Check_out_the_module

1. fedpkg clone opa-fmgui
I was having problems with Ssh and this didn't work so I used http instead: 
git clone http://pkgs.fedoraproject.org/git/rpms/opa-fmgui.git

2. Once I had my empty package module I successfully ran the following:
cd opa-fmgui
fedpkg import opa-fmgui-10.1.0.0-112.fc25.src.rpm

3. Then I commited to the local branch
git commit -m "Initial import (#1323186)."

4. But when I tried to push this to Fedora I got the following error:
ssh: connect to host pkgs.fedoraproject.org port 22: Connection timed out
fatal: Could not read from remote repository.

Does Fedora use the default port 22 for SSH?

Notes
-----
* Created my ssh keys: ~/.ssh/id-rsa-fedora and ~/.ssh/id-rsa-fedora.pub
* Added my private key to the local ssh-agent (confirmed sucesss with "ssh-add -l")
* Added my public key to my account at: https://admin.fedoraproject.org/accounts

* Ssh Configuration
HOST *.fedoraproject.org fedorapeople.org *.fedorahosted.org
IdentityFile ~/.ssh/id-rsa-fedora

* Git Configuration - push won't work unless the "push" and "pushurl" fields are set correctly.  By default these fields are ommitted, so I took my best guess; odds are these fields are wrong.
[core]
        repositoryformatversion = 0
        filemode = true
        bare = false
        logallrefupdates = true
[remote "origin"]
        url = ssh://rjtierne.org/rpms/opa-fmgui
        fetch = +refs/heads/*:refs/remotes/origin/*
        push = HEAD:refs/for/master
        pushurl = ssh://rjtierne.org/rpms/opa-fmgui
[branch "master"]
        remote = origin
        merge = refs/heads/master

Comment 87 Rick Tierney 2016-07-27 11:32:58 UTC
Does anyone have any advice regarding the issue I described above?

Comment 88 Doug Ledford 2016-07-27 13:14:19 UTC
If you have to git clone with http, then your ssh setup is broken.  It will help you check out, but if the clone via ssh doesn't work, push won't work either because the push must be via ssh.  I would suggest trying to clone via ssh and see if it works now.  As I understand it, there is a delay between when you configure your ssh public key in your account and when that key starts working (scheduled sync from the account system to the home directory filesystem or something like that, plus time to replicate from the master server to the synced slaves).  It might just work now.

Comment 89 Rick Tierney 2016-07-27 13:27:28 UTC
I tried to clone using fedpkg again; this is the error I'm getting:

[rjtierne@phbppriv13 fedora-scm]$ fedpkg clone opa-fmgui
Cloning into 'opa-fmgui'...
ssh: connect to host pkgs.fedoraproject.org port 22: Connection timed out
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
Could not execute clone: Command '['git', 'clone', 'ssh://rjtierne.org/opa-fmgui', '--origin', 'origin']' returned non-zero exit status 128
[rjtierne@phbppriv13 fedora-scm]$

Comment 90 Doug Ledford 2016-07-27 13:49:08 UTC
OK, there's something wrong in your ssh config or else there is something blocking ssh.  Can you ssh to other machines?  What if you remove your password from your ssh key?  Is it possible your ssh-agent isn't properly asking for your password and that's preventing the connection?

Comment 91 Michal Schmidt 2016-07-27 14:22:46 UTC
That's likely due to a firewall at Intel.
I replied to Rick in private with some advice (the whole world does not need to know the details, such as internal hostnames).

Comment 92 Rick Tierney 2016-07-27 17:21:08 UTC
Ok, Michal helped out a lot - much appreciation here!
clone/import/push - All Done!

The last step though is to actually kick off the build (fedpkg build).  I had a short chat with Paul Reger (libpsm2) who has been down this path before and this is the final analysis:

* fedpkg kicks of a Koji build

* I don't normally have any problems with running a Koji build directly because I always feed it the --kojiproxy flag and provide our proxy server:port

* When fedpkg tries to kick off a Koji build, it doesn't provide our proxy and so it dies.

* I have not been able to find any kind of proxy flag for fedpkg

* Michal kicked off the build for Paul when he was trying to do this

So... Neil:
Would you please kick of the opa-fmgui build for me?

Thanks!
Rick T.

Comment 93 Michal Schmidt 2016-07-28 13:37:06 UTC
I kicked off build opa-fmgui-10.1.0.0-112.fc26 for you:
http://koji.fedoraproject.org/koji/taskinfo?taskID=15049110

But this is a poor workaround. We need to find a way for you to start builds by yourselves.
Maybe ssh accounts on fedorapeople.org could be used for that?

Comment 94 Michal Schmidt 2016-07-28 13:43:01 UTC
> Maybe ssh accounts on fedorapeople.org could be used for that?

Ah, no, because no private keys are allowed on fedorapeople.org.

Comment 95 Rick Tierney 2016-07-28 13:46:03 UTC
Ok. Is there anyway to configure Koji to permanently use our proxy regardless of whether we call koji directly or if it's called by fedpkg?

I looked at the /etc/koji.conf file but didn't see any proxy references.

Comment 96 Michal Schmidt 2016-07-28 13:52:48 UTC
I don't know. Try asking on the devel mailing list (devel.org).

Comment 97 Rick Tierney 2016-07-28 13:54:20 UTC
Ok, I guess I close this BZ.
Thanks for all of your help!
Rick


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