[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-09 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

Michal Schmidt  changed:

   What|Removed |Added

 Status|ASSIGNED|CLOSED
 Resolution|--- |NEXTRELEASE
Last Closed||2016-05-09 08:34:56



--- Comment #75 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #73)
> By the way, I tried to do the "fedpkg update" step and that did not seem to
> work.
> 
> Also, I tried to do the Bodhi step, and that too did not seem to work.

"fedpkg update" is just a CLI interface to Bodhi, the Fedora updates system.
You built libpsm2 only for Rawhide (master branch, fc25). Builds on the master
branch propagate into Rawhide composes automatically, with no need for Bodhi.
You would need Bodhi to push the package into Fedora 24 or older releases.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-06 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #74 from paul.j.re...@intel.com ---
Package:libpsm2-10.2.1-1.fc25
Status: complete
Built by:   pjreger
ID: 760647
Started:Fri, 06 May 2016 19:43:47 UTC
Finished:   Fri, 06 May 2016 19:45:49 UTC

Closed tasks:
-
Task 13951102 on arm01-builder07.arm.fedoraproject.org
Task Type: build (noarch)
Link: https://koji.fedoraproject.org/koji/taskinfo?taskID=13951102

Task 13951104 on buildvm-16.phx2.fedoraproject.org Task Type: buildSRPMFromSCM
(noarch)
Link: https://koji.fedoraproject.org/koji/taskinfo?taskID=13951104
logs:
  https://kojipkgs.fedoraproject.org/work/tasks/1104/13951104/state.log
  https://kojipkgs.fedoraproject.org/work/tasks/1104/13951104/build.log
  https://kojipkgs.fedoraproject.org/work/tasks/1104/13951104/root.log
srpm:
 
https://kojipkgs.fedoraproject.org/work/tasks/1104/13951104/libpsm2-10.2.1-1.fc25.src.rpm

Task 13951122 on buildvm-04.phx2.fedoraproject.org Task Type: buildArch
(x86_64)
Link: https://koji.fedoraproject.org/koji/taskinfo?taskID=13951122
logs:
  https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/root.log
  https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/build.log
  https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/state.log
rpms:
 
https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/libpsm2-debuginfo-10.2.1-1.fc25.x86_64.rpm
 
https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/libpsm2-devel-10.2.1-1.fc25.x86_64.rpm
 
https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/libpsm2-compat-10.2.1-1.fc25.x86_64.rpm
 
https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/libpsm2-10.2.1-1.fc25.x86_64.rpm
srpms:
 
https://kojipkgs.fedoraproject.org/work/tasks/1122/13951122/libpsm2-10.2.1-1.fc25.src.rpm

Task 13951149 on buildvm-08.phx2.fedoraproject.org Task Type: tagBuild (noarch)
Link: https://koji.fedoraproject.org/koji/taskinfo?taskID=13951149

http://koji.fedoraproject.org/koji/buildinfo?buildID=760647

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-06 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #73 from paul.j.re...@intel.com ---
Since I have successfully imported and built the psm2 library, into master for
fedora, the instructions say I should close this problem report.

Does anyone object?

By the way, I tried to do the "fedpkg update" step and that did not seem to
work.

Also, I tried to do the Bodhi step, and that too did not seem to work.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-06 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #72 from paul.j.re...@intel.com ---
For everyone's information: early in the work for getting PSM info Fedora, we
had planned on going with Intel's 10.1 release.

Sadly, we did not complete the Fedora work in time, and we missed the 10.1
deadline.

So, the Fedora work will be pushed to Intel's 10.2 release.

This will be reflected in the source located on github.  The code in the 10.2
branch is the Fedora work.

The code in the 10.1 branch on github, should match the code in Intel's 10.1
release (no longer has the Fedora changes in it).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-06 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #71 from paul.j.re...@intel.com ---
I have requested a new package to be added called libpsm2.

I was not sure what to put for the upstream url, and I may have put the wrong
thing there.

The real code should come from the 10.2 branch of our github repo.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-02 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #70 from paul.j.re...@intel.com ---
Pushed fix for lack of ownership of /usr/lib/libpsm2 dir and lack of use of
%{_prefix} macro.

Diffs:

[pjreger@Fedora23-dev opa-psm2.test]$ git diff
diff --git a/libpsm2.spec.in b/libpsm2.spec.in
index 89b7ca3..3ee4d55 100644
--- a/libpsm2.spec.in
+++ b/libpsm2.spec.in
@@ -123,7 +123,7 @@ make %{?_smp_mflags}
 %{_libdir}/psm2-compat
 %{_udevrulesdir}/40-psm-compat.rules
 @LIBPSM2_COMPAT_SYM_CONF_DIR@/modprobe.d/@rpm_n...@-compat.conf
-/usr/lib/libpsm2/@rpm_n...@-compat.cmds
+%{_prefix}/lib/libpsm2

 %changelog
 * Tue Apr 05 2016 Paul Reger  - @VERSION@

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-02 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #69 from Michal Schmidt  ---
Paul,
when you file the request for a new package in PkgDB, please list me
('michich') as a co-maintainer. Thanks!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-02 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #68 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #67)
> Paul,
> I have sponsored you into the 'packagers' group.
> You should be able to proceed with the SCM admin request step:
> https://fedoraproject.org/wiki/Package_Review_Process#Contributor

Thank you Michal!

For what it is worth, the review has not been approved here at Intel yet.  But,
given that you have approved it with that one last comment, I will be able to
complete the review soon.

Stay tuned.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-02 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #67 from Michal Schmidt  ---
Paul,
I have sponsored you into the 'packagers' group.
You should be able to proceed with the SCM admin request step:
https://fedoraproject.org/wiki/Package_Review_Process#Contributor

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-05-02 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

Michal Schmidt  changed:

   What|Removed |Added

  Flags|fedora-review?  |fedora-review+



--- Comment #66 from Michal Schmidt  ---
In the "%files compat" section you now have:
/usr/lib/libpsm2/libpsm2-compat.cmds

Since the directory /usr/lib/libpsm2 did not exist previously, you need to own
it.
And I suggest using "%{_prefix}" in place of "/usr".

I see no more issues. You can fix the /usr/lib/libpsm2 ownership before
importing the package to Fedora git.

Package APPROVED.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-29 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #65 from paul.j.re...@intel.com ---
I have just pushed changes to the 10.1 branch of github that eliminate the
40-psm.rules file from the Fedora distribution.

Can you please re-review?

If you pull the branch in a sandbox, and run the makesrpm.sh script, it should
yield a src rpm with version 10.1.9, as in:

libpsm2-10.1.9-1.fc23.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-29 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #64 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #63)
> The goal of this review should be to get libpsm2 into Fedora Rawhide (the
> 'master' branch of Fedora) and Fedora 24 (which is currently in preparation
> for a Beta release). I wouldn't bother with pushing it as an update to
> Fedora 23 (the latest stable release).

You are correct.  We are trying to get to fc24, currently by way of fc25.

The reason I am using fc23 is only due to expedience.  I do not have access to
a fc24 host, but, I do have access to a fc23 host.

> Fedora 24 currently contains kernel-4.5.2-301.fc24. It's Linus's kernel
> v4.5.2 plus some Fedora patches.
...
> 
> Are you looking for the presence of commit e116a64fab650 ("IB/hfi: Properly
> set permissions for user device files")? This one was included already in
> Linux v4.3-rc2.

In a word: yes.  :-)  Thanks for 'cutting to the chase'.

So, now that is settled.  The 40-psm.rules file will not be in the Fedora
release.  I will push new changes to github to reflect this after some internal
dust settles - which is WAY TOO COMPLICATED to discuss here.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-29 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #63 from Michal Schmidt  ---
The goal of this review should be to get libpsm2 into Fedora Rawhide (the
'master' branch of Fedora) and Fedora 24 (which is currently in preparation for
a Beta release). I wouldn't bother with pushing it as an update to Fedora 23
(the latest stable release).

Fedora 24 currently contains kernel-4.5.2-301.fc24. It's Linus's kernel v4.5.2
plus some Fedora patches.

There are several ways to get to the source code of Fedora packages.
You can get them using fedpkg:
  fedpkg clone -a kernel
  cd kernel
  git checkout f24
  fedpkg prep

You can get them from a SRPM:
  dnf install dnf-plugins-core
  dnf download --source --releasever=24 kernel
  rpm -ivh kernel-*.src.rpm
  cd ~/rpmbuild/SPECS
  rpmbuild -bp kernel.spec

In the special case of the kernel you don't even have to download anything.
You can look directly at the exploded Fedora kernel git tree published by Josh
Boyer:

https://git.kernel.org/cgit/linux/kernel/git/jwboyer/fedora.git/tree/drivers/staging/rdma/hfi1/device.c?h=kernel-4.5.2-301.fc24

Are you looking for the presence of commit e116a64fab650 ("IB/hfi: Properly set
permissions for user device files")? This one was included already in Linux
v4.3-rc2.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-28 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #62 from paul.j.re...@intel.com ---
> Is 40-psm.rules even needed with the current hfi1 kernel driver?
> The device nodes should already have the expected mode even without this rule
> file. Please remove 40-psm.rules if that's the case.

In order for us to move forward on this issue, we want to examine the driver
code for hfi1 which PSM is using.

Can you please point us to that?  That is to say, if you can point us to a
public URL of a git repo, that we can clone, and literally examine the source
code to determine if a certain commit is present?  If the commit is present,
then we can eliminate the 40-psm.rules file.  But if the commit is not present,
we will have to retain the 40-psm.rules file.

I hope this makes sense.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-28 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #61 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #55)

I am responding to the collection of problems that you cited.  One problem was
not resolved yet.  All others are resolved.

Can you please give the code another look to see if you caught all problems?

> Reviewing libpsm2-10.1.7-1 (made from commit
> faa1ef38a33b4caa64c56040a2447c1ce105b3e4).
> 
> Package Review
> ==
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> 
> 
> Issues:
> ===
> The COPYING file must be included in the package and marked as %license.

FIXED and pushed to github.

> The source tree contains header files from valgrind. Can it instead use
> valgrind headers from the system? (BuildRequires: valgrind-devel)

FIXED and pushed to github.

> Is 40-psm.rules even needed with the current hfi1 kernel driver?
> The device nodes should already have the expected mode even without this rule
> file. Please remove 40-psm.rules if that's the case.

NOT FIXED YET.  This issue is being worked internally, and will undobtedly take
time for the dust to settle.  For now, I have ADDED A FIXME comment to the spec
file.  But, this issue is not resolved yet.

> The compat subpackage should have "Requires: systemd-udev" to ensure
> /usr/lib/udev/rules.d is owned by an installed package. Since systemd-udev
> requires kmod, the directory /usr/lib/modprobe.d will then also be owned.

FIXED and pushed to github.

> /usr/sbin/libpsm2-compat.cmds is called from the modprobe.d rules and I don't
> think it's meant to be run by the administrator. In that case it should not
> be
> in the default $PATH. /usr/lib/libpsm2/libpsm2-compat.cmds or
> /usr/libexec/libpsm2-compat.cmds would be better.

FIXED and pushed to github.

> Should add "BuildRequires: gcc" according to
> http://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B

FIXED and pushed to github.

> = MUST items =
> 
> C/C++:
> [x]: Package does not contain kernel modules.
> [x]: Package contains no static executables.
> [x]: Header files in -devel subpackage, if present.
> [x]: ldconfig called in %post and %postun if required.
> [x]: Package does not contain any libtool archives (.la)
> [x]: Rpath absent or only used for internal libs.
> [x]: Development (unversioned) .so files in -devel subpackage, if present.
> 
> 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.
> [!]: 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]: License field in the package spec file matches the actual license.
>  Note: Checking patched sources after %prep for licenses. Licenses
>  found: "BSD (3 clause) GPL (v2)", "Unknown or generated", "BSD (4
>  clause)", "zlib/libpng", "BSD (3 clause)", "BSD (2 clause)". 3 files
>  have unknown license. Detailed output of licensecheck:
> 
>  BSD (2 clause)
>  --
>  libpsm2-10.1.7/COPYING
>  
>  BSD (3 clause)
>  --
>  libpsm2-10.1.7/include/valgrind/memcheck.h
>  libpsm2-10.1.7/include/valgrind/valgrind.h
> 
>  Why are the valgrind headers included in the source tree?
>  
>  BSD (3 clause) GPL (v2)
>  ---
>  [... 174 files ...]
>  
>  BSD (4 clause)
>  --
>  libpsm2-10.1.7/include/opa_queue.h
> 
>  The advertising (3rd) clause of the 4-clause BSD license is incompatible
>  with GPL. Fortunately, opa_queue.h is copyrighted by The Regents of the
>  University of California, who retroactively deleted the clause in 1999:
>  ftp://ftp.cs.berkeley.edu/pub/4bsd/README.Impt.License.Change
>  
>  Unknown or generated
>  
>  libpsm2-10.1.7/COMMIT
>  libpsm2-10.1.7/RELEASE
>  libpsm2-10.1.7/psm_log.h
> 
>  psm_log.h has a valid dual BSD/GPLv2 license header, which just was
>  not recognized by the automatic tool. No problem.
>  
>  zlib/libpng
>  ---
>  libpsm2-10.1.7/ptl_ips/ips_crc32.c
> 
>  The zlib license is compatible with both BSD and GPL licenses. OK.
>  
> [x]: License file installed when any subpackage combination is installed.
> 
>  The Requires are in place to fulfill this requirement as soon as the
> main
>  package includes the COPYING file (already noted above).
> 
> [x]: If the package is under multiple licenses, the licensing breakdown
>  must be documented in the spec.
> [!]: Package must own all directories that it creates.
>  Note: Directories without known owners: /usr/lib/udev,
>  /usr/lib/modprobe.d, /usr/lib/udev/rules.d
> 
>  Is the udev rules file even needed with the current hfi1 kernel driv

[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-28 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #60 from Michal Schmidt  ---
Paul,
on a Fedora system you can install it simply using:
  dnf install fedora-review

Usually it's run like this:
  fedora-review -b 
... but this does not work for this review, because you haven't been using the
recommended format with "Spec URL:" and "SRPM URL:".
So instead run it on a local SRPM:
  fedora-review --rpm-spec -n libpsm2-10.1.7-1.fc23.src.rpm

Note that it does not automate all of the review tasks. It leaves many points
for the human reviewer to check.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-28 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #59 from paul.j.re...@intel.com ---
Michal, Thank you for running fedora-review (I think that is what the tool is
called?), and adding the output and analysis/distilation to this problem
report.

Can you also include instructions for running the tool (also installing it on a
Fedora 23 host - if possible?).

I ask this as if I knew how to do that I could run it and fix problems and not
bother you with updates until it runs cleanly.

I tried to google fedora-review but did not find any obvious hits.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-27 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #58 from Michal Schmidt  ---
I see some of the exit() calls are reachable only when "HFI_BACKTRACE" is in
the environment, as a debugging feature. So those are harmless in normal
operation.
I'm not sure under what circumstances the other exits can be reached or whether
_exit() would be better in those cases.
In any case, the exit() calls are not a blocking issue for this review.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-27 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #57 from paul.j.re...@intel.com ---
Regarding the libpsm2.so calling exit(), would it be ok to change these calls
to _exit()?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-27 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #56 from Michal Schmidt  ---
(In reply to russell.w.mcguire from comment #53)
> I wanted to ask isn't the Source0 tag mostly for user interaction (what
> you're doing now) and documentation purposes? i.e. are there tools that use
> this field?

There are a few tools that can use it if it's a full URL:
 - spectool can download the source when given a spec file.
 - fedora-review can verify whether the Source URL points to a file that's
   identical to the file archived in the SRPM.
 - rebase-helper can semi-automatically update a package to a new upstream
   version.

In case Source is not a full URL, at least there must be a comment explaining
to any future packagers how to create the tarball. This is what you now have.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-27 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #55 from Michal Schmidt  ---
Reviewing libpsm2-10.1.7-1 (made from commit
faa1ef38a33b4caa64c56040a2447c1ce105b3e4).

Package Review
==

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


Issues:
===
The COPYING file must be included in the package and marked as %license.

The source tree contains header files from valgrind. Can it instead use
valgrind headers from the system? (BuildRequires: valgrind-devel)

Is 40-psm.rules even needed with the current hfi1 kernel driver?
The device nodes should already have the expected mode even without this rule
file. Please remove 40-psm.rules if that's the case.

The compat subpackage should have "Requires: systemd-udev" to ensure
/usr/lib/udev/rules.d is owned by an installed package. Since systemd-udev
requires kmod, the directory /usr/lib/modprobe.d will then also be owned.

/usr/sbin/libpsm2-compat.cmds is called from the modprobe.d rules and I don't
think it's meant to be run by the administrator. In that case it should not be
in the default $PATH. /usr/lib/libpsm2/libpsm2-compat.cmds or
/usr/libexec/libpsm2-compat.cmds would be better.

Should add "BuildRequires: gcc" according to
http://fedoraproject.org/wiki/Packaging:C_and_C%2B%2B

= MUST items =

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: ldconfig called in %post and %postun if required.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
[x]: Development (unversioned) .so files in -devel subpackage, if present.

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.
[!]: 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]: License field in the package spec file matches the actual license.
 Note: Checking patched sources after %prep for licenses. Licenses
 found: "BSD (3 clause) GPL (v2)", "Unknown or generated", "BSD (4
 clause)", "zlib/libpng", "BSD (3 clause)", "BSD (2 clause)". 3 files
 have unknown license. Detailed output of licensecheck:

 BSD (2 clause)
 --
 libpsm2-10.1.7/COPYING

 BSD (3 clause)
 --
 libpsm2-10.1.7/include/valgrind/memcheck.h
 libpsm2-10.1.7/include/valgrind/valgrind.h

 Why are the valgrind headers included in the source tree?

 BSD (3 clause) GPL (v2)
 ---
 [... 174 files ...]

 BSD (4 clause)
 --
 libpsm2-10.1.7/include/opa_queue.h

 The advertising (3rd) clause of the 4-clause BSD license is incompatible
 with GPL. Fortunately, opa_queue.h is copyrighted by The Regents of the
 University of California, who retroactively deleted the clause in 1999:
 ftp://ftp.cs.berkeley.edu/pub/4bsd/README.Impt.License.Change

 Unknown or generated
 
 libpsm2-10.1.7/COMMIT
 libpsm2-10.1.7/RELEASE
 libpsm2-10.1.7/psm_log.h

 psm_log.h has a valid dual BSD/GPLv2 license header, which just was
 not recognized by the automatic tool. No problem.

 zlib/libpng
 ---
 libpsm2-10.1.7/ptl_ips/ips_crc32.c

 The zlib license is compatible with both BSD and GPL licenses. OK.

[x]: License file installed when any subpackage combination is installed.

 The Requires are in place to fulfill this requirement as soon as the main
 package includes the COPYING file (already noted above).

[x]: If the package is under multiple licenses, the licensing breakdown
 must be documented in the spec.
[!]: Package must own all directories that it creates.
 Note: Directories without known owners: /usr/lib/udev,
 /usr/lib/modprobe.d, /usr/lib/udev/rules.d

 Is the udev rules file even needed with the current hfi1 kernel driver?
 Directory ownership of /usr/lib/modprobe.d should be ensured by requiring
 the package that owns the directory:
 Requires: kmod

[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.

 The changelog has only one entry, but the format is correct.

[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[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 /

[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-26 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #54 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #51)
> So after doing "git clean -fdx && ./makesrpm.sh" I got
> libpsm2-10.1.4-1.fc24.src.rpm
> 
> Inside it there is libpsm2-10.1.4.tar.gz (that makesrpm.sh generated using
> "make dist") and a spec file libpsm2.spec.
> The spec file's URL tag currently points to a non-existent file, because the
> latest uploaded tarball on github is for 10.1.2.
> You can fix it by one of the following ways:
>  1. Publish the 10.1.4 tarball on github to make the Source0 URL valid, or
> 
>  2. stop trying to publish the tarballs on github and instead change the spec
>file to say:
> 
># The tarball can be created by:
># git clone https://github.com/01org/opa-psm2
># cd opa-psm
># git checkout 8f9f240380ae
># make dist
>Source0: %{name}-%{version}.tar.gz
> 
> I'd choose option 1 for actual stable releases. Option 2 is good for
> packaging arbitrary git snapshots.
> 

For reasons too complicated to explain here, we decided to go with the 2nd of
the two ways you indicate above.

FIXED and pushed to github.

> I have not run fedora-review on this yet, so for now let's only point out
> minor issues inside the spec:

Can you please run fedora-review soon?  I request that we expedite this review,
and if I am given all of the problems at once, I can probably fix them all in
shorter time than waiting for new batches of problems.

> The directory %{_libdir}/psm2-compat appears unowned.

FIXED and pushed to gitbub.

> > %{__make} DESTDIR=$RPM_BUILD_ROOT install
> There is a macro for this:
> %make_install
> (Note: It's different from %makeinstall, which should be avoided.)

FIXED and pushed to github.

> > /usr/lib/modprobe.d/libpsm2-compat.conf
> This thing does not have a dedicated macro, but at least use a macro for
> /usr:
> %{_prefix}/lib/modprobe.d/libpsm2-compat.conf

FIXED and pushed to github.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-26 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #53 from russell.w.mcgu...@intel.com ---
Michal,

I wanted to ask isn't the Source0 tag mostly for user interaction (what you're
doing now) and documentation purposes? i.e. are there tools that use this
field?


Paul,
Please go this route, as it also makes the Source0 tag the same as the internal
.spec file we use. :-)  This also makes our URL impervious to changes github
may do in the future.

>   # The tarball can be created by:
>   # git clone https://github.com/01org/opa-psm2
>   # cd opa-psm2
>   # git checkout 8f9f240380ae
>   # make dist
>   Source0: %{name}-%{version}.tar.gz

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-26 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #52 from Michal Schmidt  ---
(In reply to Michal Schmidt from comment #51)
># The tarball can be created by:
># git clone https://github.com/01org/opa-psm2
># cd opa-psm

Typo: cd opa-psm2

># git checkout 8f9f240380ae
># make dist
>Source0: %{name}-%{version}.tar.gz

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-26 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #51 from Michal Schmidt  ---
So after doing "git clean -fdx && ./makesrpm.sh" I got
libpsm2-10.1.4-1.fc24.src.rpm

Inside it there is libpsm2-10.1.4.tar.gz (that makesrpm.sh generated using
"make dist") and a spec file libpsm2.spec.
The spec file's URL tag currently points to a non-existent file, because the
latest uploaded tarball on github is for 10.1.2.
You can fix it by one of the following ways:
 1. Publish the 10.1.4 tarball on github to make the Source0 URL valid, or

 2. stop trying to publish the tarballs on github and instead change the spec
   file to say:

   # The tarball can be created by:
   # git clone https://github.com/01org/opa-psm2
   # cd opa-psm
   # git checkout 8f9f240380ae
   # make dist
   Source0: %{name}-%{version}.tar.gz

I'd choose option 1 for actual stable releases. Option 2 is good for packaging
arbitrary git snapshots.


I have not run fedora-review on this yet, so for now let's only point out minor
issues inside the spec:

The directory %{_libdir}/psm2-compat appears unowned.

> %{__make} DESTDIR=$RPM_BUILD_ROOT install
There is a macro for this:
%make_install
(Note: It's different from %makeinstall, which should be avoided.)

> /usr/lib/modprobe.d/libpsm2-compat.conf
This thing does not have a dedicated macro, but at least use a macro for /usr:
%{_prefix}/lib/modprobe.d/libpsm2-compat.conf

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-26 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #50 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #49)
> (In reply to paul.j.reger from comment #48)
> > On a Fedora system:
> > Pull the 10.1 branch from github to a sandbox.
> > cd sandbox
> > ./makesrpm.sh
> > [...]
> > The source rpm is in temp.3182/SRPMS/libpsm2-10.1.3-1.fc23.src.rpm
> 
> When I do the above steps, the resulting file is called
> libpsm2-0.7.18-1.fc24.src.rpm.
> 
> This raises several questions:
> 1. Have you decided to call the tarballs and packages "libpsm2"? I see this
>change was done only on the "10.1" branch, while "master" uses the name
>"hfi1-psm". Which one is the intended name going forward?

The 'root name' of the package has been changed from 'hfi1-psm' to 'libpsm2'
for the 10.1 branch just last week.

The plan going forward is the 10.1 branch will be merged to master once:

a) we have finished development of 10.1
b) our validation group has tested it.
c) we have fixed all bugs found by our validation group.
d) our validation group cannot find any more bugs in it.
e) management approves the release of this code.

We expect a-e to take about 8 weeks.

Our current work reviewing the spec file processing is critical to developing
the 10.1 release.

> 2. Why am I getting "0.7.18" instead of "10.1.3" like you? Is the github repo
>missing some git tags?

Yes, I forgot to push one git tag named v10.1.  Sorry about that.  I fixed that
just now.  I pushed a tag called v10.1 to the github repo.

> 3. You said version 10.1 is being worked on and will take a few weeks to get
>done. If today's version is "10.1.3", how are you going to ensure version
>monotonicity?

The '3' in the version 10.1.3 corresponds to the ordinal commit since the most
recent tag.  The most recent tag is v10.1, and as of April 25, 2016 at 3:30 PM,
there have been 4 commits.  So, the current src rpm is named:

The source rpm is in temp.20704/SRPMS/libpsm2-10.1.4-1.src.rpm

Note that the version is computed in the Makefile based on a lot of factors.

>When you do the actual release of 10.1 in a few weeks, is
>its precise version going to be 10.1. where n > 3 ?

Yes.  I cannot predict the value of n as the changes have not been completely
sorted out yet.

Did you review the spec file?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-26 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #49 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #48)
> On a Fedora system:
> Pull the 10.1 branch from github to a sandbox.
> cd sandbox
> ./makesrpm.sh
> [...]
> The source rpm is in temp.3182/SRPMS/libpsm2-10.1.3-1.fc23.src.rpm

When I do the above steps, the resulting file is called
libpsm2-0.7.18-1.fc24.src.rpm.

This raises several questions:
1. Have you decided to call the tarballs and packages "libpsm2"? I see this
   change was done only on the "10.1" branch, while "master" uses the name
   "hfi1-psm". Which one is the intended name going forward?

2. Why am I getting "0.7.18" instead of "10.1.3" like you? Is the github repo
   missing some git tags?

3. You said version 10.1 is being worked on and will take a few weeks to get
   done. If today's version is "10.1.3", how are you going to ensure version
   monotonicity? When you do the actual release of 10.1 in a few weeks, is
   its precise version going to be 10.1. where n > 3 ?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-25 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #48 from paul.j.re...@intel.com ---
I have removed the spec file from the tarball and pushed the changes to the
10.1 branch of github.

NEW PROCEDURE TO TEST:

On a Fedora system:
Pull the 10.1 branch from github to a sandbox.
cd sandbox
./makesrpm.sh

The final line of the script will show a line indicating the final resting
place of the src rpm.  It is suitable for submission to koji:

The source rpm is in temp.3182/SRPMS/libpsm2-10.1.3-1.fc23.src.rpm

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-25 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #47 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #46)
> (In reply to Michal Schmidt from comment #45)
> > We don't need "rpmbuild -ta " to work.
> 
> Can you please elaborate?

Our build system needs "rpmbuild -bs " and "rpmbuild -bb " to work.

> Do you would prefer that the spec file is not present in the tar ball?

Yes, I would prefer that.

> Is it an error for the spec file to be present in the tar ball, or just a
> nuisance/inconvenience?

It's not an error. Any spec files inside the tarball will just be ignored.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-25 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #46 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #45)
> We don't need "rpmbuild -ta " to work.

Can you please elaborate?

Do you would prefer that the spec file is not present in the tar ball?

Is it an error for the spec file to be present in the tar ball, or just a
nuisance/inconvenience?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-25 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #45 from Michal Schmidt  ---
We don't need "rpmbuild -ta " to work.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-25 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #44 from paul.j.re...@intel.com ---
(In reply to Don Dutile from comment #43)
> (In reply to russell.w.mcguire from comment #42)
> > Question: Does the tarball we generate need to contain any spec file. After
> > all these comments above, and viewing a lot of the SRPMs from Fedora21-23
> > download sites, I see that most all tar.balls inside the SRPMs do not
> > contain specs.
> > 
> > It seems to simplify life if we don't need to include the Fedora specific
> > SPEC file in our tarball. Is this the correct direction.
> 
> Yes. The spec file is separate from the tarball.

It seems that if a specfile is not present in the tarfile, then, rpmbuild -ta
will issue an error and fail.  For example, I have created a tarfile WITHOUT
the spec file in it, and rpmbuild issues an error:

[pjreger@Fedora23-dev wfr-psm-new-10]$ rpmbuild -ta libpsm2-10.1.2.tar.gz
error: Failed to read spec file from libpsm2-10.1.2.tar.gz

Comments please?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-24 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #43 from Don Dutile  ---
(In reply to russell.w.mcguire from comment #42)
> Question: Does the tarball we generate need to contain any spec file. After
> all these comments above, and viewing a lot of the SRPMs from Fedora21-23
> download sites, I see that most all tar.balls inside the SRPMs do not
> contain specs.
> 
> It seems to simplify life if we don't need to include the Fedora specific
> SPEC file in our tarball. Is this the correct direction.

Yes. The spec file is separate from the tarball.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #42 from russell.w.mcgu...@intel.com ---
Question: Does the tarball we generate need to contain any spec file. After all
these comments above, and viewing a lot of the SRPMs from Fedora21-23 download
sites, I see that most all tar.balls inside the SRPMs do not contain specs.

It seems to simplify life if we don't need to include the Fedora specific SPEC
file in our tarball. Is this the correct direction.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #41 from paul.j.re...@intel.com ---
Given that this review was going so slowly, we had two meetings this morning
with:

Intel: Ira Weiny, John Fleck, Russ McGuire, and me.
Redhat: Michal Schmidt

In the meeting we clarified what we were doing, and we decided a different
approach.

Goal: obtain approval of the spec file and other packaging issues for the
unreleased 10.1 PSM code, that is a work in progress.  The 10.1 release will
probably happen in 8 weeks time frame.  This review will be done in parallel
with other activities to get 10.1 released such as validation efforts, and any
last minute changes that ripple due to the hfi1 driver being reviewed at
kernel.org.

How this work will be done:

1. I have put all of our master branch code work into the 10.1 branch at
github:

https://github.com/01org/opa-psm2/tree/10.1

This code is complete as far as I can tell.  Sorry in advance for any problems
that I left.

2. Redhat should review what I have there, and indicate comments here.  Please
pull that branch, and 'make dist', and that should create a specfile and
tarball for you to review.  The tarball is suitable for use with 'rpmbuild -ta
'.

3. Note: I have already placed a copy of the current tarball at:
https://github.com/01org/opa-psm2/releases/download/10_1/libpsm2-10.1.1.tar.gz
which should enable testing by redhat.

4. I will fix problems found by redhat, and push changes to the 10.1 branch on
github.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #40 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #35)
> (In reply to paul.j.reger from comment #32)
> > Created attachment 1149512 [details]
> > Spec file for 10.1.0.
> 
> in the spec:
> > # The source to make this rpm was created at Intel from a private git repo.
> > # The source code is available at: https://github.com/01org/opa-psm2/
> 
> Since your Source0 is now a full URL of a tarball published by the upstream
> project, you do not strictly need to comment on its creation.
> https://fedoraproject.org/wiki/Packaging:SourceURL requires comments in
> other cases, but not in this one.

I will toss the comments in my next push of changes to the spec file.

> > Source0: 
> > https://github.com/01org/opa-psm2/releases/download/10_1/%{name}-%{version}.tar.gz
> 
> Unfortunately the URL is not quite right:
> 
> $ spectool -g -S hfi1-psm.spec
> Getting
> https://github.com/01org/opa-psm2/releases/download/10_1/hfi1-psm-10.1.0.tar.
> gz to ./hfi1-psm-10.1.0.tar.gz
> curl: (22) The requested URL returned error: 404 Not Found

You are correct that it is not quite correct yet.  I did not realize that you
were going to try to build it.  I thought we were only reviewing the spec file
itself.  Next time I push changes to the spec file to this review, I will also
push a new tarball to git hub.

> > # ERRATA: need to add %{optflags} to the build.
> > %build
> > %{__make}
> 
> Would this work?:
> %build
> export CFLAGS="%{optflags}"
> make %{?_smp_mflags}

I will try that.  And if that works, I will push this change too, and I will
remove the ERRATA comment.  For what it is worth, I tried just using:

%{__make} CFLAGS="%{optflags}"

And our code did not build.  I had not fully diagnosed the problem with the
failure, but, I assume it is some bug in the Makefile.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

paul.j.re...@intel.com changed:

   What|Removed |Added

 Attachment|0   |1
#1149513 is||
   obsolete||
 Attachment|Tar ball of source for psm  |OBSOLETE: Tar ball of
   #1149513|library.|source for psm library.
description||



--- Comment #39 from paul.j.re...@intel.com ---
Comment on attachment 1149513
  --> https://bugzilla.redhat.com/attachment.cgi?id=1149513
OBSOLETE: Tar ball of source for psm library.

I have marked this as obsolete in order to complete the review of the spec file
alone.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #38 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #34)
> (In reply to paul.j.reger from comment #31)
> > As such, I have changed that version to 10.1.0. But, that version is subject
> > to change 
> 
> I'm confused. Do you mean that this is just a pre-release of 10.1.0 and a
> final 10.1.0 is yet to be released?
> Or you're not yet sure what version number the real release will have?

The 10.1 code that I pushed to github, was not released.  It represents
essentially the tip of our master branch.  The 10.1 branch has NOT been made
yet.

The purpose of putting the bits on git hub was purely to support this review to
get PSM accepted into Fedora.

The 10.1 release is still probably 8 weeks away from being released.

> > as the code is not quite ready to be released.
> 
> I think we have different ideas what it means to release something.
> It is tagged in the public git repo, a tarball was made from it and
> published for anyone to download => it is released.

I understand what you are saying.  FWIW, we have a different definition of
released.  I hope you understand?

> Do you mean this release does not have production-level quality? Consider
> using common terms like "alpha", "beta", "rc" in the version string of such
> releases, or have a well-defined numeric versioning scheme (for example:
> X.Y.Z where Y is an odd number are pre-releases).
> Consider writing release announcements to a mailing list (I see
> intel-...@lists.01.org is mentioned on
> https://github.com/01org/opa-psm2/wiki,
> but so far there were only test messages) and/or post them on a webpage.
> Whatever versioning scheme you adopt, make it understandable for your
> downstream consumers and distributors.

The quality of code is not the issue here.  We _believe_ it is production level
quality.  The issue here is that the code has not been formally tested by our
internal validation department, and as such it can not be released yet. 

> > I will push the new version of the spec file for your review.
> > 
> > Can you please re-review my spec file, and make further comments?
> 
> I will.
> 
> > Or, if you want, we can abandon this review, as Doug suggests,
> 
> I did not read that as a suggestion to abandon this review.
> We all still would like to get hfi1-psm accepted into Fedora, so the package
> needs to pass through the review.
> 
> > and just review a tar ball.
> 
> Did you create this tarball specially for the purpose of this review? That's
> not what Doug was asking for. Neither was to upload tarballs to Bugzilla.
> Just make sure the tarballs you publish on github use a sane naming and
> versioning scheme.
> 
> This tarball can be acceptable as a base for a Fedora package:
> https://github.com/01org/opa-psm2/releases/download/10_1/hfi1-psm-10.1-0.tar.
> gz
> If I were the packager, I'd infer from the file name the version of the
> software is "10.1" and "-0" is some superfluous string to ignore.

Please disregard the tarball that I posted, as it appears we can move forward
on the spec file.  I will take down the tarball to prevent confusion.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #37 from Doug Ledford  ---
(In reply to Michal Schmidt from comment #34)
> (In reply to paul.j.reger from comment #31)
> > As such, I have changed that version to 10.1.0. But, that version is subject
> > to change 
> 
> I'm confused. Do you mean that this is just a pre-release of 10.1.0 and a
> final 10.1.0 is yet to be released?
> Or you're not yet sure what version number the real release will have?
> 
> > as the code is not quite ready to be released.
> 
> I think we have different ideas what it means to release something.
> It is tagged in the public git repo, a tarball was made from it and
> published for anyone to download => it is released.
> Do you mean this release does not have production-level quality? Consider
> using common terms like "alpha", "beta", "rc" in the version string of such
> releases, or have a well-defined numeric versioning scheme (for example:
> X.Y.Z where Y is an odd number are pre-releases).
> Consider writing release announcements to a mailing list (I see
> intel-...@lists.01.org is mentioned on
> https://github.com/01org/opa-psm2/wiki,
> but so far there were only test messages) and/or post them on a webpage.
> Whatever versioning scheme you adopt, make it understandable for your
> downstream consumers and distributors.

I'd like to second this.  Once a tarball is live on a download site, that's a
release.  And releases are permanent.  If you need a structure or organization
that allows you to have production code and test code, you could do something
like openmpi.  For them, their version breaks down like so:

1.8.0
| | |
| | Version that's incremeneted on each subsequent release
| Minor point, where all odd minor point versions are non-production
| code bases for people to test with, and all even minor point releases
| are production branches
Major release version

So, they will release any number of point releases starting at, say, 1.3.0. 
They may make it all the way up to 1.3.35 if need be.  When they decide that
this development branch is ready for a production release, it basically becomes
1.4.0.  They then open 1.5.0 as the new development branch.  They can then do
parallel work on 1.4.0 and 1.5.0, where any fixes needed for production
environments go on the 1.4.0 (and 1.5.0) branches, and when they have enough
fixes to warrant it, they release 1.4.1.  Likewise, they keep working on 1.5.0,
taking in not just fixes that were needed for production but their ongoing new
development, and they make releases here as needed.  Something like that might
work for you guys.

> > Or, if you want, we can abandon this review, as Doug suggests,
> 
> I did not read that as a suggestion to abandon this review.
> We all still would like to get hfi1-psm accepted into Fedora, so the package
> needs to pass through the review.

Indeed.  What I was meaning with my statement is that you are trying to fulfill
the requirement of this review while wearing your Intel PSM developer hat and
spit out all of the things this review needs from your upstream source repo. 
My suggestion was to instead split your work into two halves.  One being the
upstream PSM developer, and that role should spit out a tarball and nothing but
a tarball.  Then, with your Fedora packager hat on, you should take that
tarball and write a Fedora specific spec file that will pass review.  Once this
review completes and we take the package into Fedora, the spec file will never
be imported from the tarball again, so there's not really much sense in trying
to get the tarball release to provide an ongoing spec file for use in Fedora. 
The Fedora spec file will be a part of the Fedora package repo, separate from
the tarball.  So, my "We only want and can only accept a tarball" is what I
expect you to do with your PSM developer hat on.  I expect you to switch to a
Fedora package hat when working on the spec file and src rpm.  The two efforts
should be separate and distinct.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #36 from paul.j.re...@intel.com ---

Michal, can we please talk on the phone so that we can discuss your needs for
versioning and I can supply you with our plans for the 10.1 release?  There
seems to be a communication breakdown on this point, and I think if we talked
on the phone we could sort it out much more quickly.

Please send me a phone number that I can call and some times that will work for
you?  We are in the Pacific Timezone (in Hillsboro, Oregon, USA).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #35 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #32)
> Created attachment 1149512 [details]
> Spec file for 10.1.0.

in the spec:
> # The source to make this rpm was created at Intel from a private git repo.
> # The source code is available at: https://github.com/01org/opa-psm2/

Since your Source0 is now a full URL of a tarball published by the upstream
project, you do not strictly need to comment on its creation.
https://fedoraproject.org/wiki/Packaging:SourceURL requires comments in other
cases, but not in this one.

> Source0: 
> https://github.com/01org/opa-psm2/releases/download/10_1/%{name}-%{version}.tar.gz

Unfortunately the URL is not quite right:

$ spectool -g -S hfi1-psm.spec
Getting
https://github.com/01org/opa-psm2/releases/download/10_1/hfi1-psm-10.1.0.tar.gz
to ./hfi1-psm-10.1.0.tar.gz
curl: (22) The requested URL returned error: 404 Not Found

> # ERRATA: need to add %{optflags} to the build.
> %build
> %{__make}

Would this work?:
%build
export CFLAGS="%{optflags}"
make %{?_smp_mflags}

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-22 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #34 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #31)
> As such, I have changed that version to 10.1.0. But, that version is subject
> to change 

I'm confused. Do you mean that this is just a pre-release of 10.1.0 and a final
10.1.0 is yet to be released?
Or you're not yet sure what version number the real release will have?

> as the code is not quite ready to be released.

I think we have different ideas what it means to release something.
It is tagged in the public git repo, a tarball was made from it and published
for anyone to download => it is released.
Do you mean this release does not have production-level quality? Consider using
common terms like "alpha", "beta", "rc" in the version string of such releases,
or have a well-defined numeric versioning scheme (for example: X.Y.Z where Y is
an odd number are pre-releases).
Consider writing release announcements to a mailing list (I see
intel-...@lists.01.org is mentioned on https://github.com/01org/opa-psm2/wiki,
but so far there were only test messages) and/or post them on a webpage.
Whatever versioning scheme you adopt, make it understandable for your
downstream consumers and distributors.

> I will push the new version of the spec file for your review.
> 
> Can you please re-review my spec file, and make further comments?

I will.

> Or, if you want, we can abandon this review, as Doug suggests,

I did not read that as a suggestion to abandon this review.
We all still would like to get hfi1-psm accepted into Fedora, so the package
needs to pass through the review.

> and just review a tar ball.

Did you create this tarball specially for the purpose of this review? That's
not what Doug was asking for. Neither was to upload tarballs to Bugzilla.
Just make sure the tarballs you publish on github use a sane naming and
versioning scheme.

This tarball can be acceptable as a base for a Fedora package:
https://github.com/01org/opa-psm2/releases/download/10_1/hfi1-psm-10.1-0.tar.gz
If I were the packager, I'd infer from the file name the version of the
software is "10.1" and "-0" is some superfluous string to ignore.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-21 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #33 from paul.j.re...@intel.com ---
Created attachment 1149513
  --> https://bugzilla.redhat.com/attachment.cgi?id=1149513&action=edit
Tar ball of source for psm library.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-21 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

paul.j.re...@intel.com changed:

   What|Removed |Added

 Attachment|0   |1
#1148717 is||
   obsolete||



--- Comment #32 from paul.j.re...@intel.com ---
Created attachment 1149512
  --> https://bugzilla.redhat.com/attachment.cgi?id=1149512&action=edit
Spec file for 10.1.0.

This is the spec file that includes %{?dist} in the Release: tag.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-21 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #31 from paul.j.re...@intel.com ---
I have put %{?dist} into the Release: tag, together with a number of other
fixes.  Further, I will reserve the use of the Release tag to distros.  I will
place the information I need entirely in the Version: tag.  As such, I have
changed that version to 10.1.0.  But, that version is subject to change as the
code is not quite ready to be released.

I will push the new version of the spec file for your review.

Can you please re-review my spec file, and make further comments?

Or, if you want, we can abandon this review, as Doug suggests, and just review
a tar ball.  I will post that too.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-20 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #30 from Doug Ledford  ---
Yes, the %{?dist} tag must be part of the release.  What this highlights is the
fact that the release tag is Fedora, RHEL, and CentOS private.  It is for the
distro to track their build of your upstream version.  It is *not* intended for
upstream to *ever* use or set.  You're entire versioning system is broken.  I
think it's best if we simply fork to a completely separate spec file for all
Red Hat products.  So here's what we need from you, and all we can accept from
you:

A tarball - It needs to have a public URL.  And it needs to be static once
released.  We perform an md5sum check on the tarball when we download it, and
then later on, when performing automated verification of sources as part of our
security protocols, we compare that md5sum against a freshly downloaded and
md5sumed tarball from your URL tag.  If these ever don't match, we know we've
caught something happening.  For that reason, once you've released a tarball
as, say, 10.1, it is fixed and permanent.  If you need to make changes, you
need a 10.2, or 10.1.1 or something else.  You should not ever expect to have
any say in what the release field is or looks like.  That is specifically and
solely up to the distro packager.  If you happen to get a Fedora account and
become a Fedora packager, then you can control the release field by editing the
spec file in the Fedora repo, not by editing the release field in the tarball
spec file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-20 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #29 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #28)
> (In reply to paul.j.reger from comment #27)
> > To be clear, 10.1, is actually, currently in a pre-release state.
> 
> I see. In that case Release value should be something like:
>  0.1.20160420git.%{?dist}
> as in the guidelines for pre-release packages:
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-
> Release_packages
> 

To further clarify, the PSM package has already been fielded in two other
versions: 10.0.0 and 10.0.1 of RHEL and SLES.

10.1 is the next incremental release and it will include Fedora too.

I am noticing that if I add %{?dist} to the Release: tag, that it propagates
throughout the entire naming conventions requiring major changes in our
packaging code in our Makefile.  For example, the Source0: tag has %{release}
in it and so that causes the .tar.gz file to include the short distro name.

Are you sure the %{?dist} is needed in the Release: tag?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-20 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #28 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #27)
> To be clear, 10.1, is actually, currently in a pre-release state.

I see. In that case Release value should be something like:
 0.1.20160420git.%{?dist}
as in the guidelines for pre-release packages:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

> Still, I can make the initial value for the Release: tag as 1.

The Release should be >= "1" only if an actually released Version or a later
post-release snapshot is being packaged.

> > From my previous comments it can be deduced that it cannot be valid for
> > "%{release}" to appear in the Source URL of a Fedora package.
> 
> Why?

Because you won't the only person increasing the Release field in the Fedora
spec. Suppose Fedora release engineering do a mass rebuild of all packages.
They bump the Release of the package and kick off a rebuild in Koji. If
"%{release}" appears in the Source tag, the URL just became invalid.

> > In your role as an upstream developer, why don't you make a properly
> > versioned tarball your primary deliverable? [...]
> I do not believe that would work for our internal needs.  We essentially
> have a continuous integration testing scheme where every change in our
> entire product creates a new product from probably millions of lines of
> code. Hundreds of these CI builds are created, and are comprised of lots of
> packages all consisting of 10.1 version,

Continuous integration is good.

> but also qualified with a unique release number.

Does the release number of the package in Fedora have to be tied to the release
number of packages you generate internally for your CI?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-20 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #27 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #26)
> > Release: 0
> 
> From what you wrote in comment #19 I can now understand where this "0" comes
> from. However, for Fedora packaging, the initial value for Release should be
> "1". A Release tag leading with "0" would indicate you're packaging a
> pre-release snapshot of 10.1.
> Also, it must contain "%{?dist}".
> See
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning

To be clear, 10.1, is actually, currently in a pre-release state.  Still, I can
make the initial value for the Release: tag as 1.  And, I can also add %{?dist}
to it too.  See my next attachment. 

> > # The source to make this rpm was created at Intel from a private git repo.
> > # The source code is mirrored at: https://github.com/01org/opa-psm2/
> 
> If it's a true mirror, then it must be possible to create the tarball from
> the public repo on github. Why mention a private repo?

The code in github is not a true mirror.  I will correct the comment in my next
push.

> > Source0: 
> > https://github.com/01org/opa-psm2/releases/10_1/%{name}-%{version}-%{release}.tar.gz
> 
> The Source0 URL expands to
> https://github.com/01org/opa-psm2/releases/10_1/hfi1-psm-10.1-0.tar.gz ,
> which does not exist (HTTP error 404).

Whoops.  Sorry.  I will fix this in my next push as well.  The URL lacked a
/download/ after releases.  The correct URL is:

https://github.com/01org/opa-psm2/releases/download/10_1/hfi1-psm-10.1-0.tar.gz

> From my previous comments it can be deduced that it cannot be valid for
> "%{release}" to appear in the Source URL of a Fedora package.

Why?

> In your role as an upstream developer, why don't you make a properly
> versioned tarball your primary deliverable? Forget about RPM packaging for
> now.
> When you tag a release in git as "10.1", you should generate a tarball from
> it
> called hfi1-psm-10.1.tar.gz (or .bz2, or .xz) and publish it.
> 
> Then a Fedora packager (it might be yourself, but in general it can be
> another person) can point to your tarball in the Source0 tag of the spec
> file.

I do not believe that would work for our internal needs.  We essentially have a
continuous integration testing scheme where every change in our entire product
creates a new product from probably millions of lines of code. Hundreds of
these CI builds are created, and are comprised of lots of packages all
consisting of 10.1 version, but also qualified with a unique release number.

> > %build
> > %{__make}
> 
> I already wrote in comment #2 this needs to respect %optflags.
> http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

Sorry, I missed your comment.  This fix will go in my next push.

> > %files devel
> > %defattr(-,root,root,-)
> 
> %defattr is redundant.

I will lose the %defattr in my next push.

> > /usr/lib64/libpsm2.so
> 
> Should use %{_libdir} here too.

Will fix in my next push.

> > # The following files were part of the devel-noship and moved to devel:
> 
> I don't think anyone would care. The comment can be removed.

Will fix in my next push.

> > %{_includedir}/hfi1diag/ptl_ips/ipserror.h
> > %{_includedir}/hfi1diag/linux-x86_64/bit_ops.h
> > %{_includedir}/hfi1diag/linux-x86_64/sysdep.h
> > %{_includedir}/hfi1diag/opa_udebug.h
> > %{_includedir}/hfi1diag/opa_debug.h
> > %{_includedir}/hfi1diag/opa_intf.h
> > %{_includedir}/hfi1diag/opa_user.h
> > %{_includedir}/hfi1diag/opa_service.h
> > %{_includedir}/hfi1diag/opa_common.h
> > %{_includedir}/hfi1diag/opa_byteorder.h
> > %{_includedir}/hfi1diag/hfi1_deprecated.h
> 
> Your package must own the directories %{_includedir}/hfi1diag,
> %{_includedir}/hfi1diag/linux-x86_64/, %{_includedir}/hfi1diag/ptl_ips/.
> You may just own the whole subtree with a single line:
>  %{_includedir}/hfi1diag

Will fix in my next push.

> > /etc/modprobe.d/hfi1-psm-compat.conf
> 
> I already commented about shipping this under /usr/lib.

Yes, we are trying to fix this in a general way.  This will undoubtedly not be
fixed in my next push.  Let us have some time to fix this one.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-20 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #26 from Michal Schmidt  ---
> Release: 0

From what you wrote in comment #19 I can now understand where this "0" comes
from. However, for Fedora packaging, the initial value for Release should be
"1". A Release tag leading with "0" would indicate you're packaging a
pre-release snapshot of 10.1.
Also, it must contain "%{?dist}".
See
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning

> # The source to make this rpm was created at Intel from a private git repo.
> # The source code is mirrored at: https://github.com/01org/opa-psm2/

If it's a true mirror, then it must be possible to create the tarball from
the public repo on github. Why mention a private repo?

> Source0: 
> https://github.com/01org/opa-psm2/releases/10_1/%{name}-%{version}-%{release}.tar.gz

The Source0 URL expands to
https://github.com/01org/opa-psm2/releases/10_1/hfi1-psm-10.1-0.tar.gz ,
which does not exist (HTTP error 404).

From my previous comments it can be deduced that it cannot be valid for
"%{release}" to appear in the Source URL of a Fedora package.

In your role as an upstream developer, why don't you make a properly
versioned tarball your primary deliverable? Forget about RPM packaging for now.
When you tag a release in git as "10.1", you should generate a tarball from it
called hfi1-psm-10.1.tar.gz (or .bz2, or .xz) and publish it.

Then a Fedora packager (it might be yourself, but in general it can be
another person) can point to your tarball in the Source0 tag of the spec file.

> %build
> %{__make}

I already wrote in comment #2 this needs to respect %optflags.
http://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

> %files devel
> %defattr(-,root,root,-)

%defattr is redundant.

> /usr/lib64/libpsm2.so

Should use %{_libdir} here too.

> # The following files were part of the devel-noship and moved to devel:

I don't think anyone would care. The comment can be removed.

> %{_includedir}/hfi1diag/ptl_ips/ipserror.h
> %{_includedir}/hfi1diag/linux-x86_64/bit_ops.h
> %{_includedir}/hfi1diag/linux-x86_64/sysdep.h
> %{_includedir}/hfi1diag/opa_udebug.h
> %{_includedir}/hfi1diag/opa_debug.h
> %{_includedir}/hfi1diag/opa_intf.h
> %{_includedir}/hfi1diag/opa_user.h
> %{_includedir}/hfi1diag/opa_service.h
> %{_includedir}/hfi1diag/opa_common.h
> %{_includedir}/hfi1diag/opa_byteorder.h
> %{_includedir}/hfi1diag/hfi1_deprecated.h

Your package must own the directories %{_includedir}/hfi1diag,
%{_includedir}/hfi1diag/linux-x86_64/, %{_includedir}/hfi1diag/ptl_ips/.
You may just own the whole subtree with a single line:
 %{_includedir}/hfi1diag

> /etc/modprobe.d/hfi1-psm-compat.conf

I already commented about shipping this under /usr/lib.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-20 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #25 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #19)
> Yes, we are trying to use one 'template' spec file to support multiple
> distributions.

It's fine if you want to use that for your (upstream) development and testing.
But I'm afraid you'll have to drop the requirement that distributions use the
exact generated spec file. Some divergence is to be expected.
You could treat the spec file in Fedora as a fork of the upstream spec file and
cherry-pick changes from one to the other as they are made in either repo.

> But, that is only one of the things that we want.  Another
> is we want our own private git repo here at Intel, and from our own private
> git repo, we want to sync a 'mirror' of the repo on github.
> A problem with this scheme (that we wish would just go away) is we are
> deriving the 'release-version' of our code using git, and literally, that
> translates to the number of commits past the version.

RPM has the separate concepts of Version and Release in order to have the
upstream in control of Version and the packager (who's not necessarily involved
in the upstream development) in control of Release.

Again, you can keep using this scheme for the packages you make for your
upstream development and testing needs.
However, for the spec file that's to be shipped in Fedora you have to let go of
absolute control of the Release field.

(In reply to Ira Weiny from comment #20)
> How are changes by other packagers tracked?

Every Fedora package has a git repository where the spec file and possible
Fedora-specific patches are kept:
http://pkgs.fedoraproject.org/cgit/rpms/

(In reply to paul.j.reger from comment #24)
> I have since re-worked this change so that the .tar.gz file is referenced
> from an URL on git hub.

Thank you. I will do another review iteration.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-19 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #24 from paul.j.re...@intel.com ---
(In reply to paul.j.reger from comment #21)
> I think I have addressed the issues surrounding the Source0 tag.  I will
> attach the new spec file template in just a little bit.
> 
> My solution was just to use the following comments and leave the Source0 tag
> un-touched:
> 
> # The source to make this rpm was created at Intel from a private git repo.
> # The source code is mirrored at: https://github.com/01org/opa-psm2/
> Source0: %{name}-%{version}-%{release}.tar.gz
> 
> Does this address your concerns?

I have since re-worked this change so that the .tar.gz file is referenced from
an URL on git hub.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-19 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

paul.j.re...@intel.com changed:

   What|Removed |Added

 Attachment|0   |1
#1148280 is||
   obsolete||



--- Comment #23 from paul.j.re...@intel.com ---
Created attachment 1148717
  --> https://bugzilla.redhat.com/attachment.cgi?id=1148717&action=edit
Spec file for v10.1 of PSM.

This is an example of an instance of the spec file produced from logic in the
Makefile.  It may have been too much to review the template for the spec file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-18 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

paul.j.re...@intel.com changed:

   What|Removed |Added

 Attachment|0   |1
#1147688 is||
   obsolete||



--- Comment #22 from paul.j.re...@intel.com ---
Created attachment 1148280
  --> https://bugzilla.redhat.com/attachment.cgi?id=1148280&action=edit
Template for spec file - 18-04-2016

This is the template file used to generate spec files for creating rpm's.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-18 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #21 from paul.j.re...@intel.com ---
I think I have addressed the issues surrounding the Source0 tag.  I will attach
the new spec file template in just a little bit.

My solution was just to use the following comments and leave the Source0 tag
un-touched:

# The source to make this rpm was created at Intel from a private git repo.
# The source code is mirrored at: https://github.com/01org/opa-psm2/
Source0: %{name}-%{version}-%{release}.tar.gz

Does this address your concerns?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-18 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #20 from Ira Weiny  ---
(In reply to Michal Schmidt from comment #18)
> (In reply to paul.j.reger from comment #15)
> > This is the template for the spec file that we use to generate the
> > hfi1-psm.spec file.
> 
> So the resistance to changing the spec file stems from the fact that you are
> trying to maintain a common spec file for more than one distribution. That's
> not how packaging for distributions is usually done (though it's not
> unprecedented either).
> 
> Realize that you are not going to be the only person making changes to the
> spec file in Fedora. Any Fedora 'provenpackager' is allowed to make changes
> (they should have a good reason to). Release engineers will occasionally
> bump the release number of all packages when doing mass rebuilds after
> significant toolchain changes. Preventing any divergence is going to be
> almost impossible.

How are changes by other packagers tracked?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-18 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #19 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #18)
> (In reply to paul.j.reger from comment #15)
> > This is the template for the spec file that we use to generate the
> > hfi1-psm.spec file.
> 
> So the resistance to changing the spec file stems from the fact that you are
> trying to maintain a common spec file for more than one distribution. That's
> not how packaging for distributions is usually done (though it's not
> unprecedented either).
> 
> Realize that you are not going to be the only person making changes to the
> spec file in Fedora. Any Fedora 'provenpackager' is allowed to make changes
> (they should have a good reason to). Release engineers will occasionally
> bump the release number of all packages when doing mass rebuilds after
> significant toolchain changes. Preventing any divergence is going to be
> almost impossible.

Yes, we are trying to use one 'template' spec file to support multiple
distributions.  But, that is only one of the things that we want.  Another is
we want our own private git repo here at Intel, and from our own private git
repo, we want to sync a 'mirror' of the repo on github.  A problem with this
scheme (that we wish would just go away) is we are deriving the
'release-version' of our code using git, and literally, that translates to the
number of commits past the version.  And since there are two git repositories,
the same collection of source on the two git repos may have different release
versions.

I guess we need to solve these problems now.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-18 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #18 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #15)
> This is the template for the spec file that we use to generate the
> hfi1-psm.spec file.

So the resistance to changing the spec file stems from the fact that you are
trying to maintain a common spec file for more than one distribution. That's
not how packaging for distributions is usually done (though it's not
unprecedented either).

Realize that you are not going to be the only person making changes to the spec
file in Fedora. Any Fedora 'provenpackager' is allowed to make changes (they
should have a good reason to). Release engineers will occasionally bump the
release number of all packages when doing mass rebuilds after significant
toolchain changes. Preventing any divergence is going to be almost impossible.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-18 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #17 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #13)
> > Could you please use a full URL in the Source tag?
> > http://fedoraproject.org/wiki/Packaging:SourceURL
> 
> I need to know if this _needs_ to be changed?

It's preferred to use a full URL to the source archive. If for some reason no
such URL exists, at least there must be a comment in the spec file explaining
how to obtain that exact source archive.
Did you read http://fedoraproject.org/wiki/Packaging:SourceURL ? It even has
examples for github hosted projects.
I don't understand why this is controversial. What disadvantages do you see?

(In reply to paul.j.reger from comment #14)
> Do you know if SUSE will automatically get the dependency on libuuid?

I'm pretty sure it will.
At least on a current openSUSE Tumbleweed it works as expected. This is what
happens if I try to remove libuuid1:
# rpm -e libuuid1
error: Failed dependencies:
 libuuid.so.1()(64bit) is needed by (installed) libSM6-1.2.2-5.4.x86_64
... and many more errors like this, while there are none about "libuuid1".
This demonstrates that openSUSE too uses dependencies on library sonames and
not on library package names.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-15 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #16 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #12)
> Sure, go ahead and post the spec here.

I have posted the template for the spec file to this problem report.

Note that this template for the spec file is in two active reviews.  One
review, is internal here at Intel.  The other is in this problem report.

I will try to propagate comments back and forth to keep all parties informed of
all comments.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-15 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #15 from paul.j.re...@intel.com ---
Created attachment 1147688
  --> https://bugzilla.redhat.com/attachment.cgi?id=1147688&action=edit
Template for specfile

This is the template for the spec file that we use to generate the
hfi1-psm.spec file.  We use sed to set various entities in the template that
changes such as @VERSION@ and @RELEASE@.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-15 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #14 from paul.j.re...@intel.com ---
Another comment, on comment 2:

>> %if 0%{?rhel}%{?rhl}%{?fedora}
>> Requires: libuuid
>> %else
>> Requires: libuuid1
>> %endif
>
> The binary package will automatically get a dependency on
> "libuuid.so.1()(64bit)", so there is no need for an explicit Requires.
> http://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

Do you know if SUSE will automatically get the dependency on libuuid?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-15 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #13 from paul.j.re...@intel.com ---
Regarding comment 2, I received some serious pushback from a member of my team
here at Intel regarding this comment:

> Source0: %{name}-%{version}-%{release}.tar.gz
>
> Could you please use a full URL in the Source tag?
> http://fedoraproject.org/wiki/Packaging:SourceURL

I need to know if this _needs_ to be changed?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-15 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #12 from Michal Schmidt  ---
Sure, go ahead and post the spec here.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-14 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #11 from paul.j.re...@intel.com ---
I have addressed most of the issues in the spec file cited in comment 2.  The
changed spec file is in a Gerrit review here at Intel.  Would it help if I
posted the changed spec file here so that the review could continue?

I ask this only in the interest of expediting this review.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-14 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #10 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #9)
> (In reply to Michal Schmidt from comment #2)
> > To facilitate the review process (notably the 'fedora-review' helper tool),
> > the two URLs should point directly to the spec file and SRPM, not to a
> > download page.
> 
> Do you need this done?  It appears that you got the spec file from the below.

I got the spec file because I can think, but I would like an automaton
("fedora-review -b 1324590") to be able to find it. This works only if the URLs
point to the actual files.

> > Are you already a member of the 'packager' group in Fedora?
> 
> I don't know.  How do I find that out?

If this is your first Fedora package, you are not a packager yet.
You can see the groups you are a member of by logging in to
https://admin.fedoraproject.org/accounts/ and looking at your account
information.
Anyway, I checked the list of packagers and you are not in it. That's OK, I am
a "sponsor", so I can add you to 'packagers' eventually.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-13 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #9 from paul.j.re...@intel.com ---
(In reply to Michal Schmidt from comment #2)
> (In reply to paul.j.reger from comment #0)
> > Spec URL: https://github.com/01org/opa-psm2/releases/tag/10_1
> > SRPM URL: https://github.com/01org/opa-psm2/releases/tag/10_1
> 
> To facilitate the review process (notably the 'fedora-review' helper tool),
> the two URLs should point directly to the spec file and SRPM, not to a
> download page.

Do you need this done?  It appears that you got the spec file from the below.

> > Description: The PSM Messaging API, or PSM API, is the low-level
> > user-level communications interface for the Intel(R) OPA
> > family of products. PSM users are enabled with mechanisms
> > necessary to implement higher level communications
> > interfaces in parallel environments.
> 
> Just curious: Is "PSM" an acronym? "Parallel ...something.. Messaging"?

PSM == Performance Scaled Messaging Library 

> > Fedora Account System Username: pjreger
> 
> Are you already a member of the 'packager' group in Fedora?

I don't know.  How do I find that out?

More later.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-13 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #8 from Don Dutile  ---
(In reply to Ira Weiny from comment #7)
> (In reply to Honggang LI from comment #3)
> > Paul,
> >  Please replace the package name hfi-psm1 with libpsm2, as we had imported
> > it into RHEL-7.2 with name 'libpsm2'. 
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1173296
> > 
> 
> This seems like a RHEL specific issue.  AFAIK no patch was ever submitted
> upstream to make this change.  Blindly changing this now will affect other
> users which have chosen to use the package as it is.
> 
> If you have a patch which will correctly obsolete the existing packages we
> would be happy to review it.

Have to agree w/Ira -- if upstream didn't change the name, and users are
familiar with the package name as-is, we should change RHEL.
OPA support in RHEL is 'tech-preview', so we can change anything we want, until
it goes to 'fully supported'.   
The name change was probably involved in the hastily put together workaround
with the psm1-psm2 name conflicts when both packages were installed on a system
(that had opa & qib on it, for example).  The multi-fabric, not-all-OPA config
must be supported, so as long as the upstream <*>psm<*> libraries have been
cleaned up to make this work (without different compile options to compat-libs,
breaking old psm apps), then we should change RHEL-7 to reflect upstream.
According to a rather long email thread, the library
incompatibility/name-collision issue has been solved, so RHEL should be able to
mimic rest of upstream wrt naming.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-13 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

Ira Weiny  changed:

   What|Removed |Added

 CC||ira.we...@intel.com



--- Comment #7 from Ira Weiny  ---
(In reply to Honggang LI from comment #3)
> Paul,
>  Please replace the package name hfi-psm1 with libpsm2, as we had imported
> it into RHEL-7.2 with name 'libpsm2'. 
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1173296
> 

This seems like a RHEL specific issue.  AFAIK no patch was ever submitted
upstream to make this change.  Blindly changing this now will affect other
users which have chosen to use the package as it is.

If you have a patch which will correctly obsolete the existing packages we
would be happy to review it.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-11 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #6 from paul.j.re...@intel.com ---
PSM is an acronym for "Performance Scaled Messaging"

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-11 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #5 from Michal Schmidt  ---
Created attachment 1145900
  --> https://bugzilla.redhat.com/attachment.cgi?id=1145900&action=edit
SRPM from RHEL 7.2

For reference, here's the libpsm2 SRPM from RHEL 7.2.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-11 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #4 from Michal Schmidt  ---
(In reply to Honggang LI from comment #3)
>  Please replace the package name hfi-psm1 with libpsm2, as we had imported
> it into RHEL-7.2 with name 'libpsm2'. 

True, this would make our life easier. Though we should be able to deal with it
either way.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-11 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #3 from Honggang LI  ---
Paul,
 Please replace the package name hfi-psm1 with libpsm2, as we had imported it
into RHEL-7.2 with name 'libpsm2'. 

https://bugzilla.redhat.com/show_bug.cgi?id=1173296

It is will be an issue if fedora and rhel has a identical package with
different names. And add 'BuildRequires: systemd' into the spec file, otherwise
it will be failed to build in koji (mock) system.

thanks

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-11 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

Michal Schmidt  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|nob...@fedoraproject.org|mschm...@redhat.com
  Flags||fedora-review?



--- Comment #2 from Michal Schmidt  ---
(In reply to paul.j.reger from comment #0)
> Spec URL: https://github.com/01org/opa-psm2/releases/tag/10_1
> SRPM URL: https://github.com/01org/opa-psm2/releases/tag/10_1

To facilitate the review process (notably the 'fedora-review' helper tool), the
two URLs should point directly to the spec file and SRPM, not to a download
page.

> Description: The PSM Messaging API, or PSM API, is the low-level
> user-level communications interface for the Intel(R) OPA
> family of products. PSM users are enabled with mechanisms
> necessary to implement higher level communications
> interfaces in parallel environments.

Just curious: Is "PSM" an acronym? "Parallel ...something.. Messaging"?

> Fedora Account System Username: pjreger

Are you already a member of the 'packager' group in Fedora?

Looking at the spec file:

> #
> #  This file is provided under a dual BSD/GPLv2 license.  When using or
> #  redistributing this file, you may do so under either license.
> #
> #  GPL LICENSE SUMMARY
> [...]
> #  BSD LICENSE
> [...]
> # Copyright (c) 2014-2015 Intel Corporation. All rights reserved.
> #

It's OK to have a copyright and license header in a spec file in Fedora,
though it is uncommon. Most packagers rely on the default (MIT) license
specified by the Fedora Project Contributor Agreement (FPCA):
https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#License_of_Fedora_SPEC_Files

> Summary: Intel PSM Libraries
> Name: hfi1-psm
> Version: 0.7
> Release: 13
> License: GPLv2

Seems it should say more precisely: BSD or GPLv2
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Dual_Licensing_Scenarios

> Group: System Environment/Libraries

The Group tag is unnecessary.
http://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

> URL: http://www.intel.com/

Please use a URL that's more specific to hfi1-psm. Maybe
URL: https://github.com/01org/opa-psm2/

> Source0: %{name}-%{version}-%{release}.tar.gz

Could you please use a full URL in the Source tag?
http://fedoraproject.org/wiki/Packaging:SourceURL

> Prefix: /usr

Please drop this tag.
http://fedoraproject.org/wiki/Packaging:Guidelines#Relocatable_packages

> BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root

Don't specify BuildRoot.
http://fedoraproject.org/wiki/Packaging:Guidelines#Tags_and_Sections

> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig

It should not be necessary to specify those since you are using the
"%post -p /sbin/ldconfig" form to call ldconfig. In this case rpmbuild is
supposed to detect the dependency automatically.
http://fedoraproject.org/wiki/Packaging:Scriptlets#Shared_libraries

> ExclusiveArch: x86_64

A short comment explaining the reason for ExclusiveArch would be nice.

> %if 0%{?rhel}%{?rhl}%{?fedora}
> Requires: libuuid
> %else
> Requires: libuuid1
> %endif

The binary package will automatically get a dependency on
"libuuid.so.1()(64bit)", so there is no need for an explicit Requires.
http://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

> BuildRequires: libuuid-devel
> Conflicts: opa-libs

What is the purpose of this Conflicts tag? What is "opa-libs"?
It's not a package in Fedora.
http://fedoraproject.org/wiki/Packaging:Guidelines#Conflicts

> Obsoletes: hfi-psm
> Obsoletes: hfi-psm-debuginfo

Unversioned Obsoletes are dangerous. What is the purpose of these Obsoletes?
There never was a "hfi-psm" package in Fedora, was it?

> %package devel
> Summary: Development files for Intel PSM
> Group: System Environment/Development

See a previous comment for Group tag.

> Requires: %{name} = %{version}-%{release}

Better add %{?_isa} too, even though the package is exclusive to one arch.
http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig

I think only the main package needs to run ldconfig.

> Requires: libuuid-devel

> Conflicts: opa-devel
> Obsoletes: hfi-psm-devel

See previous comments about Conflicts and Obsoletes.

> %package compat
> Summary: Development files for Intel PSM

Better adjust the Summary for -compat to make it different from -devel.

> Group: System Environment/Development
> Requires: %{name} = %{version}-%{release}
> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig
> Obsoletes: hfi-psm-compat

See previous comments.

> %description
> The PSM Messaging API, or PSM API, is the low-level
> user-level communications interface for the Intel(R) OPA
> family of products. PSM users are enabled with mechanisms
> necessary to

[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-08 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590

Honggang LI  changed:

   What|Removed |Added

 CC||ddut...@redhat.com,
   ||dledf...@redhat.com,
   ||ho...@redhat.com,
   ||jsho...@redhat.com,
   ||mschm...@redhat.com
 Blocks||1315609



-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org


[Bug 1324590] Review Request: hfi1-psm - Intel PSM Libraries

2016-04-06 Thread bugzilla
https://bugzilla.redhat.com/show_bug.cgi?id=1324590



--- Comment #1 from paul.j.re...@intel.com ---
My Koji build is:

https://koji.fedoraproject.org/koji/taskinfo?taskID=13576497

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
http://lists.fedoraproject.org/admin/lists/package-review@lists.fedoraproject.org