Bug 819480

Created attachment 582629 [details]
This is limesurvey spec file

Created attachment 582663 [details]
This is limesurvey source rpm

http://fedoraproject.org/wiki/Join_the_package_collection_maintainers

Have you already read the above? Please create a review ticket as described in http://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Create_Your_Review_Request and don't forget FE-NEEDSPONSOR.

Adding needsponsor and cleaning things up a bit. 

I'm going to work with srineth on this.

ok, a few general comments here. ;) 

First, now that we are in review, please remember to increase the release and add a changelog entry everytime you fix issues. This helps make sure that I am looking at the same version you are and avoids confusion. 

Attaching spec and srpms to the bug can be anoying for folks who wish to just look at the spec easily. See me about getting some space on fedorapeople.org to place your spec and src.rpm files on. 

Some misc small things we should fix: 

1) Please fix the Summary. "The limesurvey RPM" doesn't tell people what this is. Think of it as if you were someone who had never heard of limesurvey. Describe it in a short sentence. 

2) You will need to work on the files section some more: 

In %files, when you specify a directory, it takes that and everything under it. 
When you specify %dir it only includes that specific directory. 
You shouldn't include files/directories twice. This results in a bunch of: warning: File listed twice errors from rpmbuild. So, you are going to have to drop the: %{_datadir}/limesurvey/*
and list in turn _every_ directory in there, and also "%dir %{_datadir}/limesurvey/" to get the top directory. 

3) Once files is fixed up, we need to finish an install and see if it completes. I've not looked in detail at the install, but if it simply makes a config.php file or the like, we might consider shipping a placeholder one of those to make it easy for fedora users to simply edit the config file without needing to run the installer. 

4) Once thats determined, we need to make sure the application actually works. ;)

I see some other bundled applications/libs here:
- jquery (version 1.4.2 in devel-trunk, recent is something like 1.7.x
- jQuery UI - v1.8.19 - 2012-04-16 
- adodb (already packaged as php-adodb
- ckeditor and kcfinder
- ajaxupload.js 

to be continued, I'm pretty sure, I didn't found everything.

Note that on the javascript ones: 
http://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_libraries

"At this time JavaScript intended to be served to a web browser on another computer is specifically exempted from this but this will likely change in the future"

so, I think we are ok on jquery and ajaxupload at least.

Yes, I wouldn't try to un-bundle jquery! In doubt, this would result in patching every javascript. I guess, we would get an exception for bundling jquery, if really necessary.

Created attachment 583451 [details]
limesurvey updated spec file

With the help of the Kevin and Smoogen, limesurvey spec file could be updated.

ok, some misc comments on the version in comment #9: 

1) You did increase the release to 0.3%{?dist}, but you don't have any changelog entries. Please add changelog entries for each new release at the end explaining what you changed. 

2) Your links for the upload and templates still don't match up. You have: 

...

cp -pr upload ${RPM_BUILD_ROOT}%{_var}/www/limesurvey/upload
cp -pr templates ${RPM_BUILD_ROOT}%{_var}/www/limesurvey/templates
...

/bin/ln -sf ../../../var/limesurvey/upload ${RPM_BUILD_ROOT}%{_datadir}/limesurvey/upload
/bin/ln -sf ../../../var/limesurvey/templates ${RPM_BUILD_ROOT}%{_datadir}/limesurvey/templates
...

%attr(0775,apache,apache) %{_var}/www/limesurvey/templates
%attr(0775,apache,apache) %{_var}/www/limesurvey/upload

two of those use /var/www/ and one uses /var/limesurvery.

Created attachment 585593 [details]
updated spec files and rpm packagings

You should post the full urls to spec and srpm
(you simply put a part of a url in that attachment)

Corrected: 
SPEC: http://smooge.fedorapeople.org/Downloads/limesurvey/limesurvey.spec
SRPM: http://smooge.fedorapeople.org/Downloads/limesurvey/limesurvey-2.00-0.4.fc17.src.rpm

(RPM doesn't matter)

Making some progress for sure here. ;) 

A few issues still however: 

1) The limesurvey.conf for apache needs adjusting for the new /var/www/limesurvey area. It's still pointing to /usr/share/ and not working. For that matter, perhaps we could put all but the 3 directories that need to be writable back in /usr/share? or does that not work?

2) The link under /var/www/limesurvey/application/third_party/ for the php-gettext isn't working (because of the move to /var/www/limesurvey/)
Now needs to be: 'ln -s ../../../../../usr/share/php/gettext/ ./php-gettext'

3) After fixing the above 2 items, the installer still seems to complain that 3 directories are "Found & Unwritable": 
/application/config
/upload
/templates

So, perhaps it's looking for those in the wrong place? We need to get past that error to continue in the install. ;(

Just passing by I had a look at the last spec file (hosted by smooge)

Few remarks:
- You are using %define while you should not:
http://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

- /var/www is not allowed for web-app, see:
http://fedoraproject.org/wiki/Packaging/Guidelines#Web_Applications

- What is the different between %{name} and %{realname}?

- the source0 is not a URL, since upstream's website is down I cannot check, is this a checkout of the sources or just a release candidate which has no direct URL?

- The changelog is not correctly formatted (I would expect rpmlint to complain on this)

- If the intended target is Fedora and EL6, there are a couple of elements which can go away in the spec (the %clean section, the %defattr, the group)

After talking with srineth on IRC here are my comments:

1) source code is RC1, RC2 is out.
2) package will fail to install without httpd. I highly recommend making this package independent of apache for people who use webservers such as nginx.
3) I was able to build the package after changing/commenting a few things in the spec file.

links to files:

http://vicodan.fedorapeople.org/limesurvey-2.00-0.5.fc17.noarch.rpm http://vicodan.fedorapeople.org/limesurvey-2.00-0.5.fc17.noarch.srpm http://vicodan.fedorapeople.org/limesurvey.spec

Also rpmlint is your friend.

This is new Spec file and src.rpm file
SPEC : http://srineth.fedorapeople.org/limesurvey.spec
src.rpm : http://srineth.fedorapeople.org/

Per our conversation on IRC after running rpmlint you got the following error:

limesurvey.spec: W: invalid-url Source0: http://download.limesurvey.org/Unstable_releases/limesurvey200RC2-build120528.tar.bz2 HTTP Error 404: Not Found
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

Please change "Source0" line to the following URL  below and run rpmlint again.

Source0:        http://download.limesurvey.org/Unstable_releases/limesurvey200RC2-build120528.tar.gz

After further investigation RC4 has been released.

This brings me to my first point:

1) If you are going to become the package maintainer for something you MUST KEEP UP WITH UPSTREAM! If you can't then don't package a release candidate or package the latest stable version!
2) Your spec file was a complete mess. I had to clean it up and redo it by hand. Please review the spec file that I have done and try to install the rpm I have built by hand.
3) Again, I do not like the idea of making this an apache specific package. While I understand apache is the official/default webserver of Fedora not everyone runs "httpd". Please keep this in mind and plan for it in your package.
4) I would even go so far as to remove the httpd in the requires field or create a seperate package for nginx and/or lighthttpd or script it in a way so that the configure script checks for the webserver that is installed on the end user's machine.


I have built RC4 successfully after revising the spec file and using the latest source, and changing the URL to my fedorapeople URL.

links:

http://vicodan.fedorapeople.org/limesurvey.spec
http://vicodan.fedorapeople.org/limesurvey200RC4-build120622.tar.gz
http://vicodan.fedorapeople.org/limesurvey-2.0-1.fc17.src.rpm
http://vicodan.fedorapeople.org/limesurvey-2.0-1.fc17.noarch.rpm

[dan@Fedora17 SPECS]$ rpmlint limesurvey.spec 
limesurvey.spec:38: W: macro-in-comment %{SOURCE1}
limesurvey.spec:38: W: macro-in-comment %{_sysconfdir}
limesurvey.spec:57: W: macro-in-comment %config
limesurvey.spec:57: W: macro-in-comment %{_sysconfdir}
limesurvey.spec:58: W: macro-in-comment %config
limesurvey.spec:58: W: macro-in-comment %{_sysconfdir}
limesurvey.spec:59: W: macro-in-comment %attr
limesurvey.spec:59: W: macro-in-comment %{limedir}
limesurvey.spec:60: W: macro-in-comment %attr
limesurvey.spec:60: W: macro-in-comment %{limedir}
limesurvey.spec:61: W: macro-in-comment %attr
limesurvey.spec:61: W: macro-in-comment %{limedir}
limesurvey.spec:62: W: macro-in-comment %attr
limesurvey.spec:62: W: macro-in-comment %{limedir}
0 packages and 1 specfiles checked; 0 errors, 14 warnings.

(lines commented out for obvious reasons)

@Kevin,

Any elder wisdom you could provide would help here.

I just got back today, but I'm going to try in the next few days to take a closer look at where we are here... 

Dan: I'm working with srineth here. They are new to rpm packaging, so there's definitely a learning curve. Things need to be balanced between teaching and learning here. I don't want to write the entire spec here, as I would like srineth to learn how to. As to the httpd issues, I suggest you take them up on the packaging list and come up with a proposal for the Fedora Packaging Committee.  

Anyhow, I intend to look at things and update this bug in the next few days with status... thanks for all the comments folks.

Ok. I have taken the various spec files and merged in all the changes.

Here's the notes on the changes I made:

- Updated to RC4.
- Cleaned up requres. mysql shouldn't be required. httpd was in twice.
- Adjusted httpd conf: fixed paths, added FollowSymLinks.
- Fixed link to application/config (was missing another ../)

SPEC: http://www.scrye.com/~kevin/fedora/limesurvey/limesurvey.spec
SRPM: http://www.scrye.com/~kevin/fedora/limesurvey/limesurvey-2.00-0.7.fc18.src.rpm

After doing that I built things again and installed in a test vm.

The installer doesn't want to find things because its looking at
the permissions of 'application/config' expecting it to be a writable
directory. We have it as a link to a writable directory.

If I manually:
rm application/config
cp -a /var/lib/limesurvey/application/config /usr/share/limesurvey/application/config

The installer works and completes for me.

So, I think we need to report this as a bug to the limesurvey folks
and see if they can get it fixed before 2.00 is released.

If they cannot or don't want to, we will have to look at making
all of it install in /var/lib/limesurvey but I would prefer to avoid that.

srineth: Can you file a bug with limesurvey on this? I can assist you in
forming it if you like. http://bugs.limesurvey.org/ should be the place.

If any other folks would care to look at this spec (aside from the above issue) and provide more feedback that would be great too.

Any progress here?

Not much. I filed the upstream bug a while back: 

http://bugs.limesurvey.org/view.php?id=6362

no reply upstream that I see, but it's possible that it's fixed in a newer version. 

I've not had time to package up the newest upstream, but that would be the next thing to do, and see if they have this fixed upstream already or if we can work around the issue in some other way.

How can I help move this review forward ?

2.05 will be out soon.

Any ideas from the submitter? We'd like to hear some words from you, too.

(In reply to Matthias Runge from comment #6)
> I see some other bundled applications/libs here:
> - jquery (version 1.4.2 in devel-trunk, recent is something like 1.7.x
> - jQuery UI - v1.8.19 - 2012-04-16 
> - adodb (already packaged as php-adodb
> - ckeditor and kcfinder
> - ajaxupload.js 

FYI, I'll first work on a kcfinder package.

(In reply to Dan Mashal from comment #16)
> 2) package will fail to install without httpd. I highly recommend making
> this package independent of apache for people who use webservers such as
> nginx.

It is currently not possible to be server agnostic, because Fedora's ckeditor package requires httpd.

There are two third_party directories in the current release:
https://github.com/LimeSurvey/LimeSurvey/tree/2.00_plus_131107/third_party
https://github.com/LimeSurvey/LimeSurvey/tree/2.00_plus_131107/application/third_party

The 2.05 betas contain even more bundled libraries. *sigh*

About kcfinder, I've started a quick-n-dirty spec, but I'm having trouble figuring how to handle multi-tenancy. With the current spec, two packages requiring kcfinder will use the same configuration and upload directory.

https://bitbucket.org/dridi/fedora_packages/src/16de96c/kcfinder.spec

Working with upstream may be hard, the latest kcfinder release is from 2011, and the last commit is from 2012... At least limesurvey uses the latest version.

Any ideas ?

I can't see any updates from the submitter since 2012/06, so closing it should be the proper way.

If you want to submit it, please reopen it.

Removing FE-NEEDSPONSOR from the closed review tickets.