Bug 530090

Good:
+ Package nameing fullfill naming guidelines
+ Package is written in English
+ Consistently usage of rpmmacros
+ URL shows on proper project homepage
+ Could download upstream package via spectool -g
+ Package fullfill emacs packaging guidelines
+ Package tarball matches with upstream
(md5sum: 940f1b6a08a346af8c0174ce4623b5cf)
+ License tag states GPLv3 as a OSS license
+ Local build works fine
+ Rpmlint is quite on source rpmmacros
+ Rpmlint is ok for binary rpms
+ Koji build works fine.
+ Local install and uninstall works fine
+ files has proper file permission
+ All files are owned by the package
+ Documentation is small, so we need no extra subpackage
+ Package has proper Changelog

Bad:
- Package doesn't contains most recent release of the application.
  The most recent release is 30.8.
- License tag should be GPLv2+ which is stated in the header of the source code files
- Why you use '-n %{name}-el'. We should prefer 'el' for the subpackage name.
- Package doesn't contains a verbatin copy of the license. please contact upstream
  for getting one in the upstream package.

1.Package updated to most recent release of the application;

2. License tag changed to GPLv2+

3. changed subpackage name from '-n %{name}-el' to 'el'

SPEC file : http://sagarun.fedorapeople.org/SPECS/emacs-goodies.spec
SRPM : http://sagarun.fedorapeople.org/SRPMS/emacs-goodies-30.8-1.fc11.src.rpm 


Upstream maintainer was contacted regarding inclusion of verbatim copy of the license.

License information of individual files are available at: http://packages.debian.org/changelogs/pool/main/e/emacs-goodies-el/emacs-goodies-el_30.8-1/emacs-goodies-el.copyright

Created attachment 365743 [details]
build log

Good:
+ Could downloading upstream tar ball via spectool -g
+ Packaged tar ball matches with upstream
(md5sum: 5dee468c7c599abd3b03bfe3144ccc0f)
+ Specified license on the licens tag matches with copyright
  notes on the source files.

Bad:
- Local build fails. Please see on the attached build log
- Package doesn't contains verbatin copy of the license text

I apologize, it was my mistake, just verified the md5sum of my local source srpm and the remote one; There was a mismatch; Re uploaded the source rpm; Please build using it; Thanks;

Sorry, I have retry it without any success.

Does that means, still local build fails? I have tested it  ( using 'rpmbuild --rebuild src.rpm') and its working for me;

Yes, I'm talking about the local build.

Created attachment 365765 [details]
Local Build log

I have attached my build log, i don't know why local builds fails ;

Yes, build fails :) .

Seems like a grave bug from upstream; He actually *seems* to be fixed it in  previous release (30.7) http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=550225

But bug reappears(?) in 30.8; Will file a bug report there ASAP;

Here is the link on how i  reproduced the bug:

http://sagarun.fedorapeople.org/misc/debianbug.txt  ( error is in line 568)

May be i should package the latest stable release? ( 29.3-2 )

The defunx macro of session.el has caused me to stop shipping a pre-built version of the autoload file in the Debian package, because different flavours of Emacs would create different yet incompatible versions of the autoload file, in particular where that macro was concerned.

Your compile.sh script runs when building the package.  It builds the autoload file and byte-compiles the files using a single flavour of emacs.  Do you ship only one flavour of Emacs? (e.g Emacs23)

Your easiest solution would be to skip byte-compilation for session.el.

Just to clarify...  This bug no longer occurs in Debian.  There is something specific to your build method, which appears to be a pruned-down version of mine on Debian, causing it.

Helping you won't be trivial as I don't know the fedora build and installation system supporting emacs packages.

As the the `Package doesn't contains verbatin copy of the license text', I am no sure exactly what you want since the package is a collection of third-party files.  Would adding the GPL-V2 and GPL-V3 files under the debian/ directory be enough?  I wouldn't do anything with them in Debian, and of course lintian might complain.

Also... you have not applied the patches under debian/patches, which must be done prior to building the autoload file (which itself would be better to do for each Emacs flavour upon installation rather than at build time).

I have examined the patches, for some of them ( http://pastebin.ca/1641495 ) i need dpatch-run; Which seems to be unavailable in fedora;  I am searching for a way to fix this; The good part is, applying 51_session_autoload patch alone fixes the compilation error;

I request the sponsor to comment on peter Galbraith's query on license issue;

It's great that the session.el issue is resolved.  I thought the "dpatches" should all be compatible with the patch command, although I haven't tried.
dpatch is _mostly_ a wrapper around patch anyway.

Patches are applied now:

SPEC file: http://sagarun.fedorapeople.org/SPECS/emacs-goodies.spec

SRPM : http://sagarun.fedorapeople.org/SRPMS/emacs-goodies-30.8-2.fc11.src.rpm

http://koji.fedoraproject.org/koji/taskinfo?taskID=1778230
http://koji.fedoraproject.org/koji/taskinfo?taskID=1778232

Good:
+ Local build works fine
+ Rpmlint quite for source package
+ Rpmlint quite for binary packages
+ Local install/uninstall works fine

Bad:
- Mock seems to hang after the following messages in the build.log:
gnus-eyecandy.el:159:24:Warning: `mapcar' called for effect; use `mapc' or
    `dolist' instead
In end of data:
gnus-eyecandy.el:193:1:Warning: the following functions are not known to be
    defined: gnus-group-group-name, gnus-group-entry, gnus-active,
    gnus-server-get-method, gnus-info-method, gnus-info-marks,
    gnus-info-level, gnus-info-score, gnus-range-length,
    gnus-group-timestamp-delta, annotations-at, make-extent, make-annotation,
    set-extent-property, make-glyph, set-glyph-face
Wrote /builddir/build/BUILD/emacs-goodies-el-30.8/elisp/gnus-bonus-el/gnus-eyecandy.elc

I haven't tried building packages in my local machine with mock, will try it soon; But koji uses mock ( version 0.9.14 ) too and the build was successful  http://koji.fedoraproject.org/koji/getfile?taskID=1778231&name=build.log

I have re-uploaded the source rpm; Found md5 mismatch; Please recheck mock build; Thanks

Are you guys content having the debian/ directory (with the patches that you need) in the separate .diff.gz file?  Or would you rather it all be bundled together in the orig.tar.gz file?

Just wondering...

The patches are kept in separate .diff file;

My patches are.  So you are happy with this?

(In reply to comment #22)
> My patches are.  So you are happy with this?  

Yeah!

Ok, great.

Let me know about what you'd like concerning the "verbatim copy of the license" at some point.  I suppose I could add copies of the various GPL flavors used in the top directory of the tar ball.

(In reply to comment #19)

> I have re-uploaded the source rpm; Found md5 mismatch; Please recheck mock
> build; Thanks  

I have re-downloaded the package and retry to mack a mock build without any success.

Best Regards:

Jochen Schmitt

Yes, tried to build the package with mock 0.9.17 ; Mock hangs; It seems to be executing '/usr/bin/idn --quiet --idna-to-ascii --usestd3asciirules' with gnus-filterhist.el ; Killed idn, then everything went well; Don't know the reason yet for this issue; @PSG you have anything to say on this?

(In reply to comment #24)
> Ok, great.
> 
> Let me know about what you'd like concerning the "verbatim copy of the license"
> at some point.  I suppose I could add copies of the various GPL flavors used in
> the top directory of the tar ball.  

yes, please add a COPYING file on top level directory of emacs-goodies package;

I don't know why the build is failing.

Concerning the COPYING file, some are GPL-2-or-later and some are GPL-3-or-later.  Should I use a COPYING file that includes both?

Either Add both licenses in COPYING file or use two files COPYING-GPLV2 COPYING-GPLV3; Its your choice ;

Sweet, thanks.

Yesterday i built version 30.11 , Same ,local mock build hangs running  /usr/bin/idn. 


Thanks Peter for adding the license file :)

> Thanks Peter for adding the license file :)  

No problem!  Just uploaded 31.0-1 today.  It adds vm-bogofilter.el in vm-bonus-el.  Up to you whether you want to add it...  There's a patch for it under debian/patches

Peter

@Peter

Where is the upload?

(In reply to comment #33)
> @Peter
> 
> Where is the upload?  


Source tar ball: http://ftp.de.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_31.1.orig.tar.gz

patch: http://ftp.de.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_31.1-1.diff.gz

url: http://packages.debian.org/unstable/editors/emacs-goodies-el

Sorry, I'm expect a SPEC file and a source package. It's your task to create a working package based on the files with you have cited on comment #34

> @Peter
> 
> Where is the upload?  

Since the question was directly at me, I presume that Arun assumed that you were talking about the _upstream_ sources that I provide, not the RedHat packaging that he provides.  That appears to be at :

http://sagarun.fedorapeople.org/SRPMS/
http://sagarun.fedorapeople.org/SPECS/emacs-goodies.spec

but the very latest isn't packaged.  It can sometimes be a fast-moving target followed by months of inactivity.

Peter

Bad:
- I couldn't download tar ball from upstream vis spectool -g

At an additional comment to #36, if you have a new package release, please post the URL to the SPEC file and the source package in the same form as at the beginning of the review request.

Best Regards:

Jochen Schmitt

Please don't expect to find prior than current versions of the "upstream" source tar file when it is no longer current.  Debian doesn't keep them.  If you seek that kind of stability, it would be wise to package the stable release which should be still for around a year or so:

 http://ftp.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_29.3.orig.tar.gz
 http://ftp.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_29.3-2.diff.gz

Sorry:

s/should be still for around a year/should be still available for around a year/

Updated to version 31.1-1

SRPMS: http://sagarun.fedorapeople.org/SRPMS/emacs-goodies-31.1-1.fc11.src.rpm

SPECS: http://sagarun.fedorapeople.org/SPECS/emacs-goodies.spec

koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1832998

I don't know whether there is a place for this in the SPEC file, but another URL for emacs-goodies-el sources is:

 https://alioth.debian.org/projects/pkg-goodies-el/

The CVS repo is hosted there.

If there no ftp or http source for the upstream sources which you are used in your package, you can specified a plain file name.

In this case you should add a comment in your package in which you should explain exactly, how you have created the source file based on your CVS repository.

Best Regards:

Jochen Schmitt

Jochen,

No disrecpect intended, but I get the feeling that you don't have a full handle on this package.  There is no _upstream_ ftp source, as in sourceforge or something, because it is a collection of other smaller works from various places, assembled together for Debian.  Now fedora can benefit from my integration work (or simple aggregation) and use the Debian source tar ball as the "source".  Then, forget about my CVS repo and use the Debian source tar ball, but be aware than it is a moving target.  ;-)

Otherwise, the Debian tar ball is created from the make-orig.sh script in CVS, but the Debian diff file is created by the Debian package-building scripts.

Hope this helps. Of course Arun SAG understood all this.

Peter

New version:

http://ftp.de.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_31.2.orig.tar.gz 
http://ftp.de.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_31.2-1.diff.gz

@Arun,

Please tag the files from Peter and create a proper SPEC file and source rpm package, so we may continue this package review.

Best Regards:

Jochen Schmitt

Spec URL:http://sagarun.fedorapeople.org/SPECS/emacs-goodies.spec
SRPM URL:http://sagarun.fedorapeople.org/SRPMS/emacs-goodies-31.2-1.fc12.src.rpm


Koji builds F12,F11 :

http://koji.fedoraproject.org/koji/taskinfo?taskID=1850843
http://koji.fedoraproject.org/koji/taskinfo?taskID=1850846

Good:
+ Could download source tarball via spectool -g
+ Package tar ball matches with upstream
(md5sum: ec409b4628c2c9a4f9789cb7fb570271)
+ Local build works fine
+ Koji scratch build works fine

Bad:
- The provided patch is very huge, so I have the following question:
  * Can we splitt the patch into several smaller patches
  * Do you have submitted the patches to upstream?
    Please add a comment about this on top of the
    Patch tag.

(In reply to comment #47)

> Bad:
> - The provided patch is very huge, so I have the following question:
>   * Can we splitt the patch into several smaller patches

We can, but i just removed the information specific to debian in the texi(info) file and made it as a patch.

>   * Do you have submitted the patches to upstream?

There is no need to do so.

>     Please add a comment about this on top of the
>     Patch tag.  

Sure. I will add a comment

(In reply to comment #48)
> >   * Do you have submitted the patches to upstream?
> 
> There is no need to do so.

You should _always_ submit patches upstream! So, people who use upstream packages benefit from our work. Please do so.

(In reply to comment #49)
> 
> You should _always_ submit patches upstream! So, people who use upstream
> packages benefit from our work. Please do so.  

Yes, we should always submit patches to upstream :-) , but in this case the patch i have created for emacs-goodies is specific to fedora, and the debian upstream may not need it. Any way i am sending this patch to PSG, who is also following this conversation.

Sorry, we have two patches in the package:

emacs-goodies-el.texi.patch which ist a small patch. But the only thing what I can see in the patch is the try to replace the string 'emacs-goodies' whith 'emacs-goodies-el'. This is not good, because the el subpackage should only contains the emacs sources of this emacs releated package. this mean, in common a user will only install the emacs-goodies package and the el subpackage may be only installed, if the user want to examinate or to modified the emacs sources of the macros.

emacs-goodies-el_31.2-1.diff which has a size of approximately 477 Kilobytes and lok like the way how applications are pached in Debina. I can't believe, that this patch conatins only Fedora specific modifications. So please talk to the upstream maintainer how this patch may integrated in the main source tar ball.

(In reply to comment #51)
> Sorry, we have two patches in the package:
> 
> emacs-goodies-el.texi.patch which ist a small patch. But the only thing what I
> can see in the patch is the try to replace the string 'emacs-goodies' whith
> 'emacs-goodies-el'. This is not good, because the el subpackage should only
> contains the emacs sources of this emacs releated package. this mean, in common
> a user will only install the emacs-goodies package and the el subpackage may be
> only installed, if the user want to examinate or to modified the emacs sources
> of the macros.

Hi, it does the reverse of what you have said :-) . It replaces 'emacs-goodies-el' string with 'emacs-goodies'

> emacs-goodies-el_31.2-1.diff which has a size of approximately 477 Kilobytes
> and lok like the way how applications are pached in Debina. I can't believe,
> that this patch conatins only Fedora specific modifications. So please talk to
> the upstream maintainer how this patch may integrated in the main source tar
> ball.  

It is a upstream provided patch. i got the patch from _debian upstream itself_ http://ftp.de.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_31.2-1.diff.gz ( i will add a comment about it in SPEC file). when applying this patch it gives birth to 48 more patches :-)

@PSG could you please integrate those patches inside source tar ball if possible?

Comments added in spec file before Patches:

Spec URL: http://sagarun.fedorapeople.org/SPECS/emacs-goodies.spec
SRPM URL: http://sagarun.fedorapeople.org/SRPMS/emacs-goodies-31.2-2.fc12.src.rpm

Good:
+ SPEC basename matches with package name
+ Package contains recent release of the application
+ Consistantly usage of rpm macros
+ URL tag shows on proper project homepage
+ Could download upstream tar ball via spectool -g
+ Package tarball matches with upstream
(m5dsum: ec409b4628c2c9a4f9789cb7fb570271)
+ License tag states GPLv2+ as a valid OSS license
+ Package contains verbatin copy of the license text
+ Package contains el subpackage for emacs lisp sources
+ Subpackage contains proper Required tag agains main package
+ Local build works fine
+ Package has proper Buildroot definition
+ Buildroot will be clean at start of %clean and %install
+ Rpmlint is silent on source package
+ Rpmlint is silent on binary package
+ Mock build works fine
+ Local install/uninstall works fine
+ Application seems to work properly
+ Files has proper permission
+ All files are owned by the package
+ There are new files or dirs belong to other packages
+ %doc stanza is small, so we need no separate doc subpackage
+ Package has proper Changelog

Bad:
- I could fount GPLv3 licensed source files in the package.
  Pliease change license tag to 'GPLv2+ and GPLv3'

> @PSG could you please integrate those patches inside source tar ball if
> possible?  

See comments #20 to #23 above.  So you are changing your mind?

(In reply to comment #55)
> 
> See comments #20 to #23 above.  So you are changing your mind?  

Well, I have no problems with having the patches in separate diff file, but the reviewer/sponsor feels that the patch is big and needs to be integrated with the orig.tar.gz :-) .

#fixed License

SPEC URL:http://sagarun.fedorapeople.org/SPECS/emacs-goodies.spec

SRPM URL:http://sagarun.fedorapeople.org/SRPMS/emacs-goodies-31.2-3.fc12.src.rpm

(In reply to comment #56)

> > See comments #20 to #23 above.  So you are changing your mind?  
> Well, I have no problems with having the patches in separate diff file, but the
> reviewer/sponsor feels that the patch is big and needs to be integrated with
> the orig.tar.gz :-) .  

The Fedora policies says, that patches which are not Fedora specific should be forwarded to upstream for integration into the main sources.

Because the big diff files seams to be a patch which is not Fedora specific, it may be nice if we can got integrate it into the main sources.

The advance of integrate the patch into the main sources are:

1.) Better maintainmence for the packages

2.) Clarification that this is part of the common upstream distribution of the software

Of courses it's not a blocker to have such big patch in the package as long as you have tried to sent this patch to upstream and have documented this in a comment in the SPEC file.

Best Regards:

Jochen Schmitt

Good:
+ License tag was changed to 'GPLv2+ and GPLv3'

*** APPROVED ***

Please create at FAS account and sign the CLA. After you have done this, please create a membership request to the packager group and let me know your FAS Userid.

Best Regards:

Jochen Schmitt

(In reply to comment #59)
> Good:
> + License tag was changed to 'GPLv2+ and GPLv3'
> 
> *** APPROVED ***
> 

Thanks for approving the package :-)

> Please create at FAS account and sign the CLA. After you have done this, please
> create a membership request to the packager group and let me know your FAS
> Userid.
> 

My FAS id is 'sagarun' , i submitted more than one  packages in bugzilla with this package, i already got sponsored from one of them.

https://admin.fedoraproject.org/pkgdb/users/packages/sagarun

New Package CVS Request
=======================
Package Name: emacs-goodies
Short Description: Miscellaneous add-ons for Emacs
Owners: sagarun
Branches: F-11 F-12

(In reply to comment #58)

> The Fedora policies says, that patches which are not Fedora specific should be
> forwarded to upstream for integration into the main sources.
> 
> Because the big diff files seams to be a patch which is not Fedora specific, it
> may be nice if we can got integrate it into the main sources.
> 
> The advance of integrate the patch into the main sources are:
> 
> 1.) Better maintainmence for the packages
> 
> 2.) Clarification that this is part of the common upstream distribution of the
> software

Tricky.  The thing is that I am not the real upstream author for the majority of these files.  I have bundled them and made their setup and integration easier for Debian.  So in that regard my changes rightly appear in a separate diff file (the same as if you made the changes).  But for you, I am "upstream" for this repackaging.  I _could_ include the patches in the tar file and still comply with Debian's own package policies (although some in Debian might not agree) by still keeping the patches under the debian/ directory, in debian/patches/.

I may do this in the next release.

Peter

(In reply to comment #62)

> Tricky.  The thing is that I am not the real upstream author for the majority
> of these files.  I have bundled them and made their setup and integration
> easier for Debian.  So in that regard my changes rightly appear in a separate
> diff file (the same as if you made the changes).  But for you, I am "upstream"
> for this repackaging.  I _could_ include the patches in the tar file and still
> comply with Debian's own package policies (although some in Debian might not
> agree) by still keeping the patches under the debian/ directory, in
> debian/patches/.
> I may do this in the next release.

@Peter

you are not the real upstream author?

Who is the real project owner? Is there another project homepage then this one from Debian? From my point of view, we should avoid the creation of differents "upstreams" tar balls. so it may be nice, if you can work together with the "real" upstream to get a common tar ball in with the huge diff file is integrated.

Best Regards:

Jochen Schmitt

(In reply to comment #63)
 
> @Peter
> 
> you are not the real upstream author?

See https://bugzilla.redhat.com/show_bug.cgi?id=530090#c43
We are going in circles...

emacs-goodies-el is a Debian package consisting of a _collection_ of miscellaneous Emacs add-ons.  I am the Debian maintainer of that package.  I am only "upstream" for a few of the included files.  The other authors are all listed in the debian/copyright file.

> Who is the real project owner? Is there another project homepage then this one
> from Debian?

There is no homepage for it because it is a collection of files from various authors that were suggested by Debian users, and packaged for Debian under a single package.
 
>             From my point of view, we should avoid the creation of differents
> "upstreams" tar balls. so it may be nice, if you can work together with the
> "real" upstream to get a common tar ball in with the huge diff file is
> integrated.

There is no single "real" upstream.  The huge diff is mine, but the tar ball is as well, so it would be easy for me to integrate the diff into the tar ball.

> Best Regards:
> 
> Jochen Schmitt

There is no more diff file.  The debian directory, along with all patches, are integrated in the tar ball in this new version:

http://ftp.debian.org/debian/pool/main/e/emacs-goodies-el/emacs-goodies-el_31.3.tar.gz

-- 
Peter S. Galbraith, Debian Developer          <psg>
                                 http://people.debian.org/~psg

cvs done.