Bug 507958 - Review Request: eclipse-rse - Eclipse Remote System Explorer framework
Review Request: eclipse-rse - Eclipse Remote System Explorer framework
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Andrew Overholt
Fedora Extras Quality Assurance
:
: 252223 (view as bug list)
Depends On: 507693 507710
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-24 16:35 EDT by Jeff Johnston
Modified: 2009-07-28 09:57 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-07-23 17:50:55 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
overholt: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jeff Johnston 2009-06-24 16:35:50 EDT
Spec URL: ftp://sources.redhat.com/pub/newlib/eclipse-rse.spec
SRPM URL: ftp://sources.redhat.com/pub/newlib/eclipse-rse-3.0.3-1.fc12.src.rpm
Description: Remote System Explorer (RSE) is a framework and toolkit in Eclipse
 Workbench that allows you to connect and work with a variety of remote systems.  It is required by the latest CDT release 6.0.
Comment 1 Andrew Overholt 2009-06-29 13:54:47 EDT
Thanks for the submission, Jeff.  A few comments:

- I think you're missing Requires on eclipse-cdt and eclipse-emf
- will we move to 3.1 once we get a Galileo CDT build?
- please use either %{buildroot} or $RPM_BUILD_ROOT but not both
- we should perhaps have Eclipse in the Summary field
- I think "framework" can be dropped from the Summary field
- please change the permissions on the fetch script to avoid this rpmlint warning:

eclipse-rse.src: W: strange-permission fetch-rse.sh 0775

- you should probably mark a top-level feature's about.html as %doc
- you should look for @build@ and replace it with the same qualifier that upstream uses.  In fact, you should use forceContextQualifier (see eclipse-mylyn.spec for an example) to ensure our versions are the same as upstream's for 3.0.3

The package builds for me, follows the packaging guidelines, and functions in Eclipse.  Thanks for the high quality submission.  Once the minor issues above are cleaned up, I will approve this.
Comment 2 Jeff Johnston 2009-07-20 17:16:46 EDT
(In reply to comment #1)
> Thanks for the submission, Jeff.  A few comments:
> 
> - I think you're missing Requires on eclipse-cdt and eclipse-emf

done

> - will we move to 3.1 once we get a Galileo CDT build?

Yes.  CDT 6.0 only requires RSE >= 3.0.

> - please use either %{buildroot} or $RPM_BUILD_ROOT but not both

done

> - we should perhaps have Eclipse in the Summary field

done

> - I think "framework" can be dropped from the Summary field

ok, done

> - please change the permissions on the fetch script to avoid this rpmlint
> warning:
> 
> eclipse-rse.src: W: strange-permission fetch-rse.sh 0775
> 

done

> - you should probably mark a top-level feature's about.html as %doc

done.  I get a warning about file being specified twice, but I don't see any other examples in any of the other eclipse projects.

> - you should look for @build@ and replace it with the same qualifier that
> upstream uses.  In fact, you should use forceContextQualifier (see
> eclipse-mylyn.spec for an example) to ensure our versions are the same as
> upstream's for 3.0.3
> 

I can't use forceContextQualifier because the versions are being set by map files.  The RSE project has many version qualifiers for the various plugins and features.  It does not create a single build id for a release and none of the qualifiers are filled in for the various tags.  I have fixed this by using the map files to a) fetch the plugins/features for the tarball b) create a featureVersions.properties and pluginVersions.properties file from the same map files.  Both a and b are performed in the fetch-rse.sh script.  The build now matches the upstream version numbers, including the suffixes which are used for features.  I have opened an RFE for pdebuild to allow map files to be specified which should make this easier for future rpm packaging.

> The package builds for me, follows the packaging guidelines, and functions in
> Eclipse.  Thanks for the high quality submission.  Once the minor issues above
> are cleaned up, I will approve this.  

Thanks.
Comment 3 Jeff Johnston 2009-07-20 17:17:36 EDT
I forgot to mention that I have overwritten the spec file and SRPM:

Spec URL: ftp://sources.redhat.com/pub/newlib/eclipse-rse.spec
SRPM URL: ftp://sources.redhat.com/pub/newlib/eclipse-rse-3.0.3-1.fc12.src.rpm
Comment 4 Alexander Kurtakov 2009-07-21 09:42:25 EDT
(In reply to comment #2)
> > - you should probably mark a top-level feature's about.html as %doc
> 
> done.  I get a warning about file being specified twice, but I don't see any
> other examples in any of the other eclipse projects.
> 
You can directly use paths as they are inside sources and this will prevent this warnings from showing, e.g.:
%doc org.eclipse.rse.sdk-feature/epl-v10.html
%doc org.eclipse.rse.sdk-feature/license.html
Comment 5 Andrew Overholt 2009-07-21 10:50:28 EDT
A few more minor things:

- you have ${_jvmdir} and %{_jvmdir}
- this is weird:  $RPM_BUILD_ROOT%{install_loc} ... please use %{buildroot}%{install_loc}.  In fact, please s/$RPM_BUILD_ROOT/%{buildroot}/g in the whole file
- do what Alex suggests for the doc files

Thanks for dealing with the qualifier situation.
Comment 6 Jeff Johnston 2009-07-21 13:13:06 EDT
(In reply to comment #5)
> A few more minor things:
> 
> - you have ${_jvmdir} and %{_jvmdir}

fixed to use %{_jvmdir}

> - this is weird:  $RPM_BUILD_ROOT%{install_loc} ... please use
> %{buildroot}%{install_loc}.  In fact, please s/$RPM_BUILD_ROOT/%{buildroot}/g
> in the whole file

done

> - do what Alex suggests for the doc files
> 

done

> Thanks for dealing with the qualifier situation.  

Replaced files at:

Spec URL: ftp://sources.redhat.com/pub/newlib/eclipse-rse.spec
SRPM URL: ftp://sources.redhat.com/pub/newlib/eclipse-rse-3.0.3-1.fc12.src.rpm
Comment 7 Andrew Overholt 2009-07-21 13:41:31 EDT
Thanks.  This submission is approved.
Comment 8 Jeff Johnston 2009-07-21 14:43:16 EDT
New Package CVS Request
=======================
Package Name: eclipse-rse
Short Description: Eclipse Remote System Explorer
Owners: jjohnstn
Branches: 
InitialCC: jjohnstn
Comment 9 Jason Tibbitts 2009-07-23 12:35:46 EDT
CVS done.
Comment 10 Jeff Johnston 2009-07-23 17:50:55 EDT
eclipse-rse-3.0.3-1.fc12 successfully built
Comment 11 Alexander Kurtakov 2009-07-28 09:57:29 EDT
*** Bug 252223 has been marked as a duplicate of this bug. ***

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