Bug 390571 - Review Request: eclipse-setools - SETools Eclipse Plugin
Summary: Review Request: eclipse-setools - SETools Eclipse Plugin
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 299171
TreeView+ depends on / blocked
 
Reported: 2007-11-19 15:50 UTC by Dave Sugar
Modified: 2007-12-05 14:04 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-12-05 14:04:19 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Dave Sugar 2007-11-19 15:50:39 UTC
Spec URL: http://oss.tresys.com/projects/slide/browser/trunk/build/SPEC/eclipse-setools.spec
SRPM URL: http://oss.tresys.com/projects/slide/chrome/site/srpm/eclipse-setools-3.3.0-2.src.rpm

Description: 
Eclipse SETools is an eclipse plugin exposing the java interfaces for SELinux policy analysis tools for use in eclipse plugins.
The package inclues java runtime libraries for the following:
 	
 libapol                policy analysis library
 libpoldiff             semantic policy difference library
 libqpol                library tha abstracts policy internals
 libseaudit             parse and filer SELinux audit messages in log files

Comment 1 Mamoru TASAKA 2007-11-20 07:15:07 UTC
It seems this does not build on x86_64.
http://koji.fedoraproject.org/koji/taskinfo?taskID=249678

By the way, while I don't know well about this package, are there
any reason you disable ppc/ppc64 build?

Comment 2 Dave Sugar 2007-11-20 13:16:07 UTC
I don't have a ppc/ppc64 machines to attempt to build the sources.  I don't have
enough information on how to setup symlinks on those hosts.   But, there is no
reason it shouldn't work as long as all the dependencies are available.

Comment 3 Dave Sugar 2007-11-20 13:26:37 UTC
I'm also confused about the build.  It says 'failed' for x86_64, but it looks
like the build host was ppc4.fedora.phx.redhat.com which seems to only build ppc
and ppc64.

Maybe I'm just confused.

Comment 4 Mamoru TASAKA 2007-11-20 14:38:28 UTC
The host which actually tried to rebuild differ.
Please see the link
http://koji.fedoraproject.org/koji/taskinfo?taskID=249679

Comment 5 Dave Sugar 2007-11-20 15:47:25 UTC
I see, and I think I have fixed it.  For some reason the build succeeded on my
machine.  But the path to find the jar files was not correct in the ant script.
 New RPM at 
http://oss.tresys.com/projects/slide/chrome/site/srpm/eclipse-setools-3.3.0-3.src.rpm
same SPEC location 
http://oss.tresys.com/projects/slide/browser/trunk/build/SPEC/eclipse-setools.spec

Comment 6 Mamoru TASAKA 2007-11-21 10:21:36 UTC
CC-ing Andrew Overholt.

Andrew, I would appreciate it if you would review this bug as
it seems that this review request requires some knowledge for
eclipse.

Comment 7 Andrew Overholt 2007-11-21 17:47:01 UTC
Hi,

There really isn't much to know about packaging Eclipse plugins :).

For building, either they build with PDE Build or they use some custom build
procedure.  This plugin seems to fall into the latter category.  I don't know if
the BuildArch is necessary.

I can't comment on specific build issues since I haven't tried building, but I
am curious about the fragments.  What platform-specific bits are there?  Some
sort of JNI bits?  Fragments should go in arch-specific dirs like
%{_libdir}/eclipse/plugins to meet LSB compliance.  Other than that small issue,
it looks to me like things are good with the package.  The Fedora review process
may of course find other issues, but from a high-level standpoint, it looks fine
to me :)

HTH,

Andrew "man, that looks ramble-y" Overholt

Comment 8 Dave Sugar 2007-11-23 18:54:15 UTC
Ok, I have moved the arch-specific stuff into %{_libdir}/eclipse/plugins.  It is
a bunch of SWIG wrapped stuff. 

The BuildArch is there because it is not setup to build on PPC.  Mostly because
I don't have access to a PPC machine to get it setup. Though I think it should
be ok, as long as the dependencies are available.  

SRPM: 
http://oss.tresys.com/projects/slide/chrome/site/srpm/eclipse-setools-3.3.0-4.src.rpm
SPEC:
http://oss.tresys.com/projects/slide/browser/trunk/build/SPEC/eclipse-setools.spec




Comment 9 Mamoru TASAKA 2007-11-28 14:19:55 UTC
Sorry for delay.

Well, while actually I don't know about eclipse,

For 3.3.0-4:
! First of all, the formal release 3.3.2 seems to be there.

* %setup
  - %setup is not quiet. Please use "%setup -q".

* License
  - Now the license tag "GPL" is not valid for Fedora.
    http://fedoraproject.org/wiki/Packaging/LicensingGuidelines
    http://fedoraproject.org/wiki/Licensing

    By the way, how we can know that this is licensed under
    GPL?

* Perl modules dependency
  - For perl modules dependency, please write not the rpm name directly
    but the module name the rpm provides, for example:
---------------------------------------------------
BuildRequires: perl(XML::XPath)
---------------------------------------------------
    ref:    
    http://fedoraproject.org/wiki/Packaging/Perl

* Timestamps
  - When using "cp" or "install" to install files, please add
    "-p" option to keep timestamps on installed files.

* Files entry
  - Now we recommend %defattr(-,root,root,-)
  - By the way, the %files entry
----------------------------------------------------
%files
%dir foo/
foo/*
----------------------------------------------------
    can be simplified by
----------------------------------------------------
%files
foo/
----------------------------------------------------


Comment 10 Mamoru TASAKA 2007-11-28 14:25:43 UTC
Well, one more issues

* Absolute symlinks
  - rpmlint eclipse-setools*rpm shows (rpmlint is in rpmlint rpm)
---------------------------------------------------------------
eclipse-setools.i386: W: symlink-should-be-relative
/usr/lib/eclipse/plugins/com.tresys.setools.linux.x86_3.3.2/lib/libjseaudit.so
/usr/lib/libjseaudit.so.4
---------------------------------------------------------------
    We now requests that all symlinks should be not absolute but
    relative.

Comment 11 Dave Sugar 2007-11-29 13:25:16 UTC
Ok, I think I have gotten these things corrected.
SETools 3.3.2 has been released into rawhide so I have updated the dependency
here.  
Fixed the setup, cp and files entry
Removed perl module dependency - wasn't needed any longer
License is stated at public website
http://oss.tresys.com/projects/setools/wiki/license
I fixed the symlinks, but there is still some complaint about the .so files.
I was also messing with ppc support.  While I can't test it, it might now build.

Hopefully this is it. 

Updated Links:

SRPM: 
http://oss.tresys.com/projects/slide/chrome/site/srpm/eclipse-setools-3.3.2-1.src.rpm
SPEC:
http://oss.tresys.com/projects/slide/browser/trunk/build/SPEC/eclipse-setools.spec

Comment 12 Mamoru TASAKA 2007-11-30 13:11:22 UTC
Well, for 3.3.2-1:

* License
> License is stated at public website
> http://oss.tresys.com/projects/setools/wiki/license
  - In this case, please use LGPLv2+.
    (However, would you ask upstream developer to include license
     text in the tarball from next version?)

* SourceURL
  - Now as it seems you are using formally released tarball,
    please write a full URL for Source0.

* rpmlint: E: empty-debuginfo-package
  - For this package, the files under arch-specific directories
    (i.e. %_libdir) are either text files or symlinks, so
    debuginfo is useless for this package.

    So please refer to the section "Useless or incomplete debuginfo 
    packages due to other reasons" of
    http://fedoraproject.org/wiki/Packaging/Debuginfo
    and write in the spec file like:

-------------------------------------------------------
Requires: setools-libs-java >=
%{require_setools_major_ver}%{require_setools_fix_ver}
Requires: eclipse-platform
Requires: java-gcj-compat >= 1.0.33

BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
# Files under %%_libdir are either text files or symlinks to the libraries
# in setools-libs-java, so debuginfo rpm is useless.
%define  debug_package %{nil}

%description
--------------------------------------------------------
    Note: in the comment or %changelog, please use %% so that macros
          won't be expanded.

(In reply to comment #11)
> I fixed the symlinks, but there is still some complaint about the .so files.
  - Can be ignored for this package.

> I was also messing with ppc support.  While I can't test it, it might now build.
  - Actually this builds on all archs.
    http://koji.fedoraproject.org/koji/taskinfo?taskID=267040


Comment 13 Dave Sugar 2007-11-30 13:44:44 UTC
Ok, I fixed the debuginfo-package and license.
Good to know the ppc compiles ok.

As for the SourceURL, the source is pulled from the svn repo and not the source
tar file http://oss.tresys.com/projects/setools/wiki/download.  This RPM is just
depeding on the RPM built from that source (setools-java-libs.rpm and
setools-libs.rpm).  It is adding functionality on top of those rpms, which
provide the java bindings into eclipse for slide and other eclipse plugins.

Updated Links:

SRPM: 
http://oss.tresys.com/projects/slide/chrome/site/srpm/eclipse-setools-3.3.2-2.src.rpm
SPEC:
http://oss.tresys.com/projects/slide/browser/trunk/build/SPEC/eclipse-setools.spec

Comment 14 Mamoru TASAKA 2007-11-30 14:06:17 UTC
Well, I have not checked 3.3.2-2, however
* For source tarball
  - In such case, I recommend to use 
    "3.3.2-X.1997svn%{?dist}" for EVR (if this is after the "formal"
    3.3.2 release of eclipse-setools, otherwise I recommend
    3.3.2-0.X.1997svn%{?dist}) where X is to be incremented as 1, 2,...
    You can refer to
    http://fedoraproject.org/wiki/Packaging/NamingGuidelines

Comment 15 Dave Sugar 2007-11-30 14:35:42 UTC
You are suggesting the 1997svn because the source is being pulled from an SVN
tree directly?  Would it be better to generate and post a source tarball from
the SVN tree and use that in Source0?

And 3.3.2-X vs 3.3.2-0.X because of the versioning of the dependency
setools-java-libs?  The 3.3.2 release is 'formal'.  If there are code changes in
the future the version will go to 3.3.3 (or higher).  I don't think this rpm
should care about the -X of the setools-libs-java rpm.

Personally I would like to leave the version as 3.3.2-X and post a source tarball.

What is your opinion?

Comment 16 Mamoru TASAKA 2007-11-30 14:55:37 UTC
(In reply to comment #15)
> You are suggesting the 1997svn because the source is being pulled from an SVN
> tree directly?  Would it be better to generate and post a source tarball from
> the SVN tree and use that in Source0?

Umm? What do you mean by the second sentence? Is the tarball
currently used as Source0 is not from SVN tree?

I am suggesting 1997svn because your spec file reads that
you created Source0 from svn repository and when I tried the
current revision seems 1997.

> And 3.3.2-X vs 3.3.2-0.X because of the versioning of the dependency
> setools-java-libs?  
No. As written by http://fedoraproject.org/wiki/Packaging/NamingGuidelines ,
if the tarball used as Source0 is the "pre-release" of 3.3.2
you should use "3.3.2-0.X" for EVR of Fedora rpm. If not
(i.e. if the tarball used as Source0 is after the release of 3.3.2),
you should use "3.3.2-X"

> The 3.3.2 release is 'formal'.  If there are code changes in
> the future the version will go to 3.3.3 (or higher).  I don't think this rpm
> should care about the -X of the setools-libs-java rpm.

X (this is a integer) is not related to setools. X means that
if you change the spec file of eclipse-setools, you should change
the EVR as 3.3.2-1, 3.3.2-2, ..... as before.

> Personally I would like to leave the version as 3.3.2-X and post a source
> tarball.
Well, would you write again how you created the Source0?
- If there is a URL from which we can download the tarball used
  as Source0, use the URL as Source0 and use 3.3.2-{1,2,3,...} as
  EVR.
- If you used svn to create souce tarball, you should use
  3.3.2-2.svn1997 -> 3.3.2-3.svn1997 -> 3.3.2-4.svn1998 ->
  3.3.3-1.svn2000 (for example).



Comment 17 Dave Sugar 2007-11-30 16:00:49 UTC
I have been creating Source0 by an export from svn and creating a tar from that
- as noted in the SPEC file  (lines 28-34).  I'm wondering if it would be better
to actually build and post the source tarball and use that as Source0 to avoid
the svn1997 stuff in the RPM name.

I think I understand what you are saying.  
You are saying that because the source is pulled from svn use the svn1997 naming
for the RPM.
But, once the 3.3.2 version is released (and tarball posted) then change the
Source0 to reference to tarball and RPM name to 3.3.2-X.
Is that correct?



Comment 18 Mamoru TASAKA 2007-11-30 16:25:56 UTC
(In reply to comment #17)
> I have been creating Source0 by an export from svn and creating a tar from that
> - as noted in the SPEC file  (lines 28-34).  I'm wondering if it would be better
> to actually build and post the source tarball and use that as Source0 to avoid
> the svn1997 stuff in the RPM name.

You are saying that you are the upstream and you can put the 3.3.2 tarball
somewhere so that we can download it directly? It is very preferable.

> 
> I think I understand what you are saying.  
> You are saying that because the source is pulled from svn use the svn1997 naming
> for the RPM.
Yes.

> But, once the 3.3.2 version is released (and tarball posted) then change the
> Source0 to reference to tarball and RPM name to 3.3.2-X.
Yes, but this means that the formal 3.3.2 is *not yet released*?
If my understanding is correct, for now use must use 3.3.2-0.X.svnYYYY.

We expects:
3.3.2-0.1.svn1998 -> 3.3.2-0.2.svn1998 -> 3.3.2-0.3.svn2000 ->
3.3.2-0.4.svn2002 -> (3.3.2 tarball is released) -> 3.3.2-1 ->
3.3.2-2 -> .. -> 3.3.3-0.1.svn2100 -> ....
 



Comment 19 Dave Sugar 2007-11-30 17:05:03 UTC
Exactly, we are uptream for the eclispe-setools rpm and this is a pre-release.  
I have updated the version to eclipse-setools-3.3.2-0.2.svn1998 as requeested.

SRPM: 
http://oss.tresys.com/projects/slide/chrome/site/srpm/eclipse-setools-3.3.2-0.2.svn1998.src.rpm
SPEC:
http://oss.tresys.com/projects/slide/browser/trunk/build/SPEC/eclipse-setools.spec



Comment 20 Mamoru TASAKA 2007-11-30 17:26:10 UTC
Very trivial issue, but please fix the below before you commit
into CVS (but please read below first).

* Fri Nov 30 2007 Dave Sugar <dsugar> - 3.3.2-0.2svn1998
                                                            ^
For sponsor needed ticket, we requests the submitter to
pre-review other person's review requests or submit another review
request per
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored
However you have another review request already, which will hopely
be accepted too with another few correction.

-------------------------------------------------------------------
    This package (eclipse-setools) is APPROVED by me
-------------------------------------------------------------------

Please follow the procedure according to:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor (at the stage, please also write on
this bug for confirmation that you requested for sponsorship)
Then I will sponsor you.

If you want to import this package into Fedora 7/8, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on Fedora rebuilding system).

If you have questions, please ask me.

Comment 21 Dave Sugar 2007-11-30 19:52:18 UTC
Ok, I have gone through the 'Get a Fedora Account' experience.
I am requesting sponsorship.
I have corrected the changelog.

Thanks for all your help and patience!

I have also updated the eclipse-slide.spec while going through this one so it
shouldn't be to far off. 

Comment 22 Mamoru TASAKA 2007-12-01 07:00:40 UTC
Now I should be sponsoring you. Please follow
"Join" wiki again.

Comment 23 Dave Sugar 2007-12-04 13:58:15 UTC
New Package CVS Request
=======================
Package Name: eclipse-setools
Short Description: An Eclipse interface to SETools libraries
Owners: dsugar
Branches: F-7 F-8 EL-5
InitialCC: 
Cvsextras Commits: yes

Comment 24 Mamoru TASAKA 2007-12-04 17:55:10 UTC
Just note:
- Currently F-7, F-8 has setools-3.2-3.fc7, setools-3.3.1-7.fc8,
  respectively.
- And for now rawhide rebuild of srpm in comment 19 fails due to
  openssl soname bump.
  https://www.redhat.com/archives/fedora-devel-list/2007-December/msg00239.html
  http://koji.fedoraproject.org/koji/taskinfo?taskID=273860
  On rawhide you have to wait until gnome-vfs2 is rebuilt 
  against newest openssl.

Comment 25 Kevin Fenzi 2007-12-04 19:56:30 UTC
cvs done.

Comment 26 Mamoru TASAKA 2007-12-05 01:38:03 UTC
Now gnome-vfs2 is rebuilt against new openssl and
eclipse-setools can be rebuilt on rawhide.

Comment 27 Dave Sugar 2007-12-05 14:04:19 UTC
All added into cvs and built successfully.
Note that we did a public release so I bumped the version to 3.3.2-1 and updated
the Source URL to reference the posted tar.gz file.


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