Bug 213594 - Review Request: eclipse-phpeclipse
Review Request: eclipse-phpeclipse
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Anthony Green
Fedora Package Reviews List
:
: 203205 (view as bug list)
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-11-02 01:16 EST by Brandon Holbrook
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-20 20:23:04 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Build log from mock (17.08 KB, text/plain)
2006-11-09 19:00 EST, Anthony Green
no flags Details
Latest mock build results on fc6 (17.25 KB, text/plain)
2006-12-16 11:01 EST, Anthony Green
no flags Details

  None (edit)
Description Brandon Holbrook 2006-11-02 01:16:37 EST
Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-8.src.rpm

Description: This is a RESUBMISSION of eclipse-phpeclipse after the last review (Bug 203025) Stalled.
Comment 1 Brandon Holbrook 2006-11-02 01:18:28 EST
*** Bug 203205 has been marked as a duplicate of this bug. ***
Comment 2 Brandon Holbrook 2006-11-02 01:20:24 EST
I mistyped the original ticket number in comment 0: the original really was Bug
203205
Comment 3 Anthony Green 2006-11-02 06:30:19 EST
Brandon: I don't think it's OK to add your own license file to %doc, even if it
is the appropriate one.  

I believe this rule is....
MUST: 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 must be included in %doc.

I tried this once with vkeybd and see where it got me...
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=189889
Comment 4 Brandon Holbrook 2006-11-02 10:06:43 EST
Wow that's funny, because all of mine and others' (that I know of) PEAR/PECL
packages manually include a copy of the PHP license.

The tarball DOES contain an .html version of the license, but I thought an ASCII
version would be more portable so I manually included it.  If you like, I can
just stick the .html in %doc and remove the .txt file.
Comment 5 Anthony Green 2006-11-02 10:15:31 EST
(In reply to comment #4) 
> The tarball DOES contain an .html version of the license, but I thought an ASCII
> version would be more portable so I manually included it.  If you like, I can
> just stick the .html in %doc and remove the .txt file.

I think that would be best.  I'll do a review of your next SRPM.

AG
Comment 7 Anthony Green 2006-11-09 18:59:03 EST
(In reply to comment #6)
> Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
> SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-9.src.rpm

This fails to build in mock for me.  I don't know why.  I'll attach the build log.
Comment 8 Anthony Green 2006-11-09 19:00:30 EST
Created attachment 140853 [details]
Build log from mock
Comment 9 Ben Konrath 2006-11-09 19:54:14 EST
That build error is happening because the eclipse SDK packages have changed a
little since the review was submitted. You need to build with either: 

eclipse -noSplash  \
        -application org.eclipse.ant.core.antRunner \
        -DjavacFailOnError=true \
        ...

or:

java -cp $SDK/startup.jar \
     -Dosgi.sharedConfiguration.area=%{_libdir}/eclipse/configuration          
                \
     -Duser.home=$homedir                        \
      org.eclipse.core.launcher.Main             \
     -DjavacFailOnError=true \
      ... 

I like the first build method better because it will automatically pick up
anything we add to the binary launcher to help discover things like new
eclipseextension locations, config locations etc. But I've been having some
problems with GCJ using that build method - GCJ seems to use up all the memory
on my system and then build gets an OutOfMemory exception and die. So if the
first build method doesn't work, then try the second. I hope this helps and
don't be afraid to ask if you have any questions.
Comment 10 Brandon Holbrook 2006-11-28 01:25:50 EST
Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-10.src.rpm

Okay I spent a couple days tweaking the java command and now have something that
builds again.

Ben, I tried using `eclipse` to do my build, but every time it died with a
'Could not open Display' error.  With or without '-noSplash', I couldn't get it
to work.  Is there another trick to get eclipse to run command-line-only?
Comment 11 Brandon Holbrook 2006-11-28 01:47:38 EST
Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-11.src.rpm

I bumped the eclipse-platform requirement to 3.2 to reflect the current eclipse
version in fc6/devel
Comment 12 Ben Konrath 2006-11-28 16:43:19 EST
(In reply to comment #10)
> Ben, I tried using `eclipse` to do my build, but every time it died with a
> 'Could not open Display' error.  With or without '-noSplash', I couldn't get it
> to work.  Is there another trick to get eclipse to run command-line-only?

No, I can't think of anything off hand. It should be ok to use what you have in
-11 but just set the osgi.sharedConfiguration.area property:

java -cp $SDK/startup.jar \
     -Dosgi.sharedConfiguration.area=%{_libdir}/eclipse/configuration \
     ....

Let me know if you have any other questions. Ben
Comment 13 Brandon Holbrook 2006-11-28 23:39:44 EST
Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-12.src.rpm

Okay I've added that configuration line, but have a question or two.  The line
as you provided it didn't work.  /usr/lib64/eclipse/configuration doesn't exist
on my system (or in /usr/lib/ on my i386 system), nor has yum ever heard of a
package providing said directory.  There IS, however, a
/usr/share/eclipse/configuration, which looks like it contains the proper
contents.  SO, I changed the line you gave to =%{_datadir}/eclipse/configuration
and everything started working beautifully.  Is there some other configuration
info that is supposed to exist in libdir that I don't have, or was your line
just a mistype?
Comment 14 Ben Konrath 2006-11-29 14:47:11 EST
You need 
-Dosgi.sharedConfiguration.area=%{_libdir}/eclipse/configuration for rawhide
because we changed the packages to be multilib compatible. We are putting out an
update to fc6 with this change as well. I'll report back when the updated
packages have been pushed. Sorry for not mentioning this in the last post. 
Comment 15 Anthony Green 2006-12-16 11:01:16 EST
Created attachment 143855 [details]
Latest mock build results on fc6

This is the latest mock build result for this package.	Is it supposed to build
for FC6?
Comment 16 Andrew Overholt 2006-12-18 11:15:25 EST
The osgi.sharedConfiguration.area is incorrect.  It needs to be as Ben mentioned
in comment #14.
Comment 17 Brandon Holbrook 2006-12-18 21:55:21 EST
Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-13.src.rpm

Yeah sorry, when the new eclipse RPMs came out I built new packages for myself
but forgot to add them to this review.  Enjoy.
Comment 18 Anthony Green 2006-12-19 16:42:27 EST
(In reply to comment #17)
> Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
> SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-13.src.rpm

Thanks.  

This is almost ready.  See lines starting with 'X' below.

* package meets and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present. 
* build root is correct.  
* license field matches the actual license.
* license is open source-compatible.
* License text included in package.
* source files match upstream (extracted from upstream cvs so no md5sum available.)
* latest version is being packaged.
* BuildRequires are proper.
* package builds in mock.

X rpmlint on eclipse-phpeclipse-1.1.8-13.fc6.i386.rpm says:
W: eclipse-phpeclipse wrong-file-end-of-line-encoding
/usr/share/eclipse/features/net.sourceforge.phpeclipse_1.1.8/cpl-v10.html

Fix it with sed in the %prep section: 
%{__sed} -i 's/\r//' src/path/to/cpl-v10.html

rpmlint on eclipse-phpeclipse-1.1.8-13.fc6.src.rpm says:
W: eclipse-phpeclipse strange-permission make-phpeclipse-source-archive.sh 0755

I think we can ignore this.

* final provides and requires are sane:
eclipse-phpeclipse-1.1.8-13.fc6.i386.rpm
  core.jar.so  
  debug.jar.so  
  externaltools.jar.so  
  launch.jar.so  
  phpeclipse.jar.so  
  phphelp.jar.so  
  smartyui.jar.so  
  ui.jar.so  
  webbrowser.jar.so  
  webcore.jar.so  
  xmlcore.jar.so  
  xmlui.jar.so  
  eclipse-phpeclipse = 1.1.8-13.fc6
==
  /usr/bin/rebuild-gcj-db   
  eclipse-platform >= 1:3.2.1
  httpd  
  java-gcj-compat >= 1.0.33
  php  

* shared libraries are present, but no ldconfig required.
* package is not relocatable.
X does not own the directories it creates.
  %files contains...
%{_libdir}/gcj/%{name}/*
  but I think that should be...
%{_libdir}/gcj/%{name}
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* %clean is present.
* %check is not present
* scriptlets OK
* code, not content.
* documentation is integrated, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
* not a GUI app (no .desktop file required).
* not a web app.


Comment 19 Brandon Holbrook 2006-12-19 23:31:02 EST
Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-14.src.rpm

Both taken care of
Comment 20 Anthony Green 2006-12-20 07:50:30 EST
(In reply to comment #19)
> Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
> SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-14.src.rpm
> 
> Both taken care of

Thanks, but why did you add a call to "bash" after the end-of-line conversion?
Comment 21 Brandon Holbrook 2006-12-20 17:20:29 EST
Spec URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse.spec
SRPM URL: http://theholbrooks.org/RPMS/eclipse-phpeclipse-1.1.8-15.src.rpm

That's a feature, not a bug :)  Actually it was leftover debugging that I forgot
to take out.
Comment 22 Anthony Green 2006-12-20 17:26:17 EST
This package is APPROVED.  Thanks!
Comment 23 Brandon Holbrook 2006-12-20 20:23:04 EST
Imported, FC6 branch requested, and build requested (job 24299)

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