Bug 790628

I have no packages, and thus need a sponsor.

Hi Alec. If the license for this package is indeed MIT, then this package can go into Fedora. Please open a review in Fedora's bug tracker, post the link here, and close this ticket as CLOSED INVALID.

Woah, whoops! This is RH's BZ, never mind :) Carry on :)

Just a note about the "instruction set override" in the jam file. Such hardcoding needs to be removed completely because Fedora builds and runs also on ppc/sparc/s390 (all have 32-bit and 64-bit variants) and maybe even other arches. The defaults set by %{optflags} or gcc itself should be used.

I have actually tried... What those lines says is basically "If generating 486 on a x86_64 host, please do. Otherwise, use the gcc "native" option", the latter kind of default. One thing I missed is that there actually *is* a %ifnarch macro, which should be used here. Would that be sufficient?

If the "native" option gets translated to -march=native, then it's wrong. It will generate code for the builder architecture, but Fedora usually supports lower levels, eg. building on armv7 builder it will generate running on armv7 (or better), but Fedora package must run also on armv5. The lowest supported level is set for Fedora packages via the %{optflags} macro.

Thanks for input!

New attempt: use march from %optflags, using 'native' as default if it's not defined (as is the case on my x86_64 box). Although anything is possible, totally removing this from the build setup seems cumbersome, at least. Could this be ok?

Actual, relevant flags on my x86_64: "g++"  -ftemplate-depth-128 -O0 -finline-functions -Wno-inline -Wall -march=native -pthread -fPIC -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic

fPIC is added by me, to support relinking to dynamic lib.

Also fixed linkage problem revealed by rpmlint on installed package.

Spec: http://dl.dropbox.com/u/17870887/adobe-2/adobe-source-libraries.spec
Srpm:http://dl.dropbox.com/u/17870887/adobe-2/adobe-source-libraries-1.0.43-2.fc15.src.rpm

As a comparison, on an Atom box: "g++"  -ftemplate-depth-128 -O0 -finline-functions -Wno-inline -Wall -march=i686 -pthread -fPIC -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom

i. e., march is picked up from %otpflags

I was wrong, again. It builds OK without defining any architecture at all. 'instruction-set' and 'architecture' removed from jamroot.jam. New URLs:

http://dl.dropbox.com/u/17870887/adobe-3/adobe-source-libraries.spec
http://dl.dropbox.com/u/17870887/adobe-3/adobe-source-libraries-1.0.43-3.fc15.src.rpm

Not only bombono-DVD seems to need this. Got this message:
On 02/16/2012 05:36 PM, Karel Volný wrote:
> Hi,
>
> just FYI, ASL is needed also for libgigi. Which is needed for
> Freeorion.
[cut]

Looking into last comment, it seems that ASL is bundled in libgigi in Russian Fedora.

(In reply to comment #9)
> http://dl.dropbox.com/u/17870887/adobe-3/adobe-source-libraries.spec
Just some basic questions/comments:

1. The source download pointed to by the web page is a sf download. You would usually use a full URL for the Source0. Fedora uses a specific fixed URL to access sf sources, see:
https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net

Unless you are packaging direct from version control, and if so, should state why, and the Release: would need work to fit with one of Fedora's pre/post release naming schemes, see:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Post-Release_packages

2. Where possible, provide comment info on Source1,2. eg: exact URL retrieved from (in that case, why not use the url directly ?), or where it came from ?

3. In your changelog, please use a blank line to separate each dated entry.

4. Don't use large amount of white space between changelog date, email, and version parts.

5. Changelog version: don't include {?dist} ie: .fc15, see:
https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs "must item"

6. Don't have you editor write config items to the spec file. (expandtab etc).

7. Static libs: see:
https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries

This looked promising regarding bjam and shared libraries:
http://stackoverflow.com/questions/1768943/building-boost-on-linux-library-names

Might be worth looking to see if another rpm based distro solved this already, eg Mandriva, Suse etc., or hints from debian based distro.

8. tools/bjam: is this tool already in fedora ? If so, then BuildRequire it instead.

These items are more technical; I haven't tried to build or analyse the prep / build commands. ps. I'm not a sponsor.

(In reply to comment #12)
> (In reply to comment #9)
> > http://dl.dropbox.com/u/17870887/adobe-3/adobe-source-libraries.spec
> Just some basic questions/comments:
Thanks!
 
> 1. The source download pointed to by the web page is a sf download. You would
> usually use a full URL for the Source0. Fedora uses a specific fixed URL to
> access sf sources, see:
> https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net
The source is actually  a combo of ASL and boost + a link, packed together in the way ASL needs to be built. I've put a short-form of this info in the comment. While this could be done it the %prep stage, I have concluded that it becomes just a to messy (yes, I have tried :) )

> Unless you are packaging direct from version control, and if so, should state
> why, and the Release: would need work to fit with one of Fedora's pre/post
> release naming schemes, see:
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Post-Release_packages
What's wrong with "Release:  3%{?dist}" ?

> 2. Where possible, provide comment info on Source1,2. eg: exact URL retrieved
> from (in that case, why not use the url directly ?), or where it came from ?
Not much to say, I wrote it :)
 
> 3. In your changelog, please use a blank line to separate each dated entry.
Fixed
 
> 4. Don't use large amount of white space between changelog date, email, and
> version parts.
Fixed
 
> 5. Changelog version: don't include {?dist} ie: .fc15, see:
> https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs "must item"
Fixed

> 6. Don't have you editor write config items to the spec file. (expandtab etc).
Fixed
 
> 7. Static libs: see:
> https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
Is this applicable?! I'm not packaging a static library, I'm building a static library which is converted to a dynamic in the following steps. Or am I missing something?

> This looked promising regarding bjam and shared libraries:
> http://stackoverflow.com/questions/1768943/building-boost-on-linux-library-names
Which of the issues in this link do you think are not solved in this packaging. I don't really follow you...

> Might be worth looking to see if another rpm based distro solved this already,
> eg Mandriva, Suse etc., or hints from debian based distro.
Once again, solved what?!
 
> 8. tools/bjam: is this tool already in fedora ? If so, then BuildRequire it
> instead.
Yes, it's there, but not compatible, I have actually tried. Since this is not installed, just used in the build process I don't see the problem here either.

[cut]

(In reply to comment #13)
> (In reply to comment #12)
> > 1. The source download pointed to by the web page is a sf download. You would
> > usually use a full URL for the Source0. Fedora uses a specific fixed URL to
> > access sf sources, see:
> > https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net
> The source is actually  a combo of ASL and boost + a link, packed together in
> the way ASL needs to be built. I've put a short-form of this info in the
> comment. While this could be done it the %prep stage, I have concluded that it
> becomes just a to messy (yes, I have tried :) )

Usually we package either a major version, or some post or pre release version from some sort of code repository. I can't tell which this is.

> > Unless you are packaging direct from version control, and if so, should state
> > why, and the Release: would need work to fit with one of Fedora's pre/post
> > release naming schemes, see:
> > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Post-Release_packages
> What's wrong with "Release:  3%{?dist}" ?
If you are using the upstream zip/archive then it's fine, and just include the sf.net download using the method in the guidelines.

If this is a source checkout from some code repository, the guidelines show how to specify that type of checkout (eg date, cvs/svn/git version/checksum). This comes from rpm being a build from source system, where anyone should be able build the identical binary based on the information in the spec. Hence we would need to know the upstream checkout version to start with... There are possibly some exceptions to this.

> > 7. Static libs: see:
> > https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
> Is this applicable?! I'm not packaging a static library, I'm building a static
> library which is converted to a dynamic in the following steps. Or am I
> missing something?

You might have to query someone ith more knowledge in this area, but it seems that you are bundling a different version of boost, and then linking to that, rather than to the distribution's boost version ?

> Once again, solved what?!
Fedora packages avoid bundling libraries that are already in the distro, and so you would need to get specific permission/allowance to bundle it. I might be totally off base - perhaps another opinion would be good.

> > 8. tools/bjam: is this tool already in fedora ? If so, then BuildRequire it
> > instead.
> Yes, it's there, but not compatible, I have actually tried. Since this is not
> installed, just used in the build process I don't see the problem here either.

The build process would also normally be required to be built either from packages already in Fedora, or from source, although a few packages have explicit exceptions (eg java from memory).

Overall, I probably can't provide much more guidance, someone with more experience in these areas would be helpful.

(In reply to comment #14)
> (In reply to comment #13)
> > (In reply to comment #12)
[cut]
> Usually we package either a major version, or some post or pre release version
> from some sort of code repository. I can't tell which this is.
Now, from a review standpoint, what's the problem? Besides that this is unusual? The build procedure has certain requirements, and I have solved it this way. If this is wrong, please explain why.

> > > Unless you are packaging direct from version control, and if so, should state
> > > why, and the Release: would need work to fit with one of Fedora's pre/post
> > > release naming schemes, see:
> > > https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Post-Release_packages
> > What's wrong with "Release:  3%{?dist}" ?
> If you are using the upstream zip/archive then it's fine, and just include the
> sf.net download using the method in the guidelines.
> 
> If this is a source checkout from some code repository, the guidelines show how
> to specify that type of checkout (eg date, cvs/svn/git version/checksum). This
> comes from rpm being a build from source system, where anyone should be able
> build the identical binary based on the information in the spec. Hence we would
> need to know the upstream checkout version to start with... There are possibly
> some exceptions to this.
Have you actually packed up and looked into the source? It might help. Anyway, maybe we should wait until some of the grown-ups could advice on the possible problems here.

> > > 7. Static libs: see:
> > > https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
> > Is this applicable?! I'm not packaging a static library, I'm building a static
> > library which is converted to a dynamic in the following steps. Or am I
> > missing something?
> 
> You might have to query someone ith more knowledge in this area, but it seems
> that you are bundling a different version of boost, and then linking to that,
> rather than to the distribution's boost version ?
I'm  not bundling boost. What I do is to build a static library against a private boost copy in build time. As a last step I relink this against system boost libraries. The private copy of boost is not installed. So, I don't really don't know what to ask about.

That I'm not bundling is easily confirmed looking at %files.

> > Once again, solved what?!
> Fedora packages avoid bundling libraries that are already in the distro, and so
> you would need to get specific permission/allowance to bundle it. I might be
> totally off base - perhaps another opinion would be good.
Basically, I think you are if you think I'm bundling boost. I'm not.

> The build process would also normally be required to be built either from
> packages already in Fedora, or from source, although a few packages have
> explicit exceptions (eg java from memory).
tools/bjam is built from sources in the first bootstrap step of the build. It's not part of the source.

> Overall, I probably can't provide much more guidance, someone with more
> experience in these areas would be helpful.
Thanks for your help!

Addendum, the source discussion: what the guidelines basically boils down to is that if that if the source url can't be used to obtain the source (as is the case here) the comment should describe how to get it. And that's what current comment does.

First of all: I apologize. I am definitely bundling, and it was plain wrong to say something else, especially since I was so sure.   

However, after some time I realized that David is right about bundling. After some discussion in the devel list (basically, I got some advice), I have updated. Expanding the changelog:
- get-source.sh now removes everything besides the tools subdirectory from the bundled boost library.
- jamroot is patched to not include other references to boost source.
- spec file is modified from the the fact that bundling takes place, covered by the general "Conditionalized functionality" exception.
- I'm still relinking, partly to make sure there are no references to the bundled boost lib.

A nice side-effect is that that the srpm has shrinked to ~9MB from ~63MB.

New links:
spec: http://dl.dropbox.com/u/17870887/adobe-4/adobe-source-libraries.spec
srpm: http://dl.dropbox.com/u/17870887/adobe-4/adobe-source-libraries-1.0.43-4.fc15.src.rpm

See also: http://lists.fedoraproject.org/pipermail/devel/2012-February/162947.html

Updated after test with fedora-review tool:
fedora-review -b 790628 --mock-config fedora-16-i386

Comments:

[!]: MUST If (and only if) the source package includes the text of the license(s)...
Seems like a tool error, LICENSE_1_0_0.txt is in docs as expected.

[!]: SHOULD SourceX / PatchY prefixed with %{name}
I have used the 'asl' prefix instead, to avoid insanely long names.

 E: zero-length /usr/share/doc/adobe-source-libraries-doc-1.0.43/documentation/performance/index.html
Don't think this should cause any harm, whereas removing it possibly might screw up some links. Ergo leaving it as-is.

W: strange-permission get-source.sh 0755L
This is as intended, get-source is a script used to build %Source0

W: invalid-url Source0: adobe-source-libraries-1.0.43-1.47.0.tar.gz
Properly commented in lines above (http://fedoraproject.org/wiki/Packaging:SourceURL#Troublesome_URLs).

 W: no-documentation in devel package OK since it requires base package which has the docs.

New Links:
http://dl.dropbox.com/u/17870887/asl-5/adobe-source-libraries.spec
http://dl.dropbox.com/u/17870887/asl-5/adobe-source-libraries-1.0.43-5.fc15.src.rpm

I would suggest looking into the zero-length file to see what the purpose is for this file; though if you're unable to find out upstream or else wise, it might be best to ignore it.

(In reply to comment #17)
> - get-source.sh now removes everything besides the tools subdirectory from the
> bundled boost library.

Note that this is usually removed in %prep unless there is something legally troubling with it. Although it isn't explicitly required and since it dramatically reduces the size of the source, I would see it as acceptable.

> - jamroot is patched to not include other references to boost source.

Is this the asl-fedora-flags.patch or is it one of the sed/other lines? Not to nit pick, but it doesn't seem very clear in the spec file, in the name of the patch, or the comments.


Also, what is source2? It doesn't seem to be mentioned in the commenting

(In reply to comment #19)
[cut]

Upstream is basically silent. There are a large number of patches available, but nothing seem to be merged or otherwise commented. Given the nature of the file, I have decided to leave it. After all, this is documentation, not configuration or code.

> Note that this is usually removed in %prep unless there is something legally
> troubling with it. Although it isn't explicitly required and since it
> dramatically reduces the size of the source, I would see it as acceptable.
Agreed. There are other reasons as well: spec simplicity, a reasonable way to handle those perforce repositories.

> 
> > - jamroot is patched to not include other references to boost source.
> 
> Is this the asl-fedora-flags.patch or is it one of the sed/other lines? Not to
> nit pick, but it doesn't seem very clear in the spec file, in the name of the
> patch, or the comments.
It's in the asl-fedora-flags.patch. You are right, this need to be renamed. BTW, this is a place for nit-picking, no need to apologize ;)

> Also, what is source2? It doesn't seem to be mentioned in the commenting
Well, the .pc extension basically says it all: it's a pkg-config configuration file, used by other packages to figure out flags required to build using this lib. This is a (the?) standard mechanism for C libraries. Should I comment it?

Although the patch needs to be renamed, I'm not publishing new links at this point.

(In reply to comment #20)
> Well, the .pc extension basically says it all: it's a pkg-config configuration
> file, used by other packages to figure out flags required to build using this
> lib. This is a (the?) standard mechanism for C libraries. Should I comment it?

A small comment either stating what it is or where to download it (if you did not create it) wouldn't hurt.

For example:
>>#Source2 is a required pkgconfig that isn't included in the build/source

Or whatever you would think would make it clear, as I'm not familiar and haven't looked into how this package source builds.

Removing FE-NEEDSPONSOR since I'm  nowadays is sponsored.

The get-source.sh from SRPM from comment 18 still packs the full boost, so I used the stripped adobe-source-libraries-1.0.43-1.47.0.tar.gz from SRPM.

I tried to purge the content of adobe_root/boost_1_47_0/tools directory and it still builds fine. I did rpmdiff of RPMs created before and after purge of this directory and it seems they only differ in timestamps, so are they really needed?

(In reply to comment #23)
> The get-source.sh from SRPM from comment 18 still packs the full boost, so I
> used the stripped adobe-source-libraries-1.0.43-1.47.0.tar.gz from SRPM.

Time to publish new links (right version of get-source.sh, comments update)
http://dl.dropbox.com/u/17870887/adobe-5/adobe-source-libraries-1.0.43-5.fc16.src.rpm
http://dl.dropbox.com/u/17870887/adobe-5/adobe-source-libraries.spec

> I tried to purge the content of adobe_root/boost_1_47_0/tools directory and it
> still builds fine. I did rpmdiff of RPMs created before and after purge of this
> directory and it seems they only differ in timestamps, so are they really
> needed?

For me, they seem to be. How did you test?  This fails for me:
$ rpmbuild -ba ~/rpmbuild/SPECS/adobe-source-libraries.spec
[OK] 
$ rpm -rf ~/rpmbuild/BUILD/adobe_root/boost_1_46_0/
$ rpmbuild -bc --short-circuit  ~/rpmbuild/SPECS/adobe-source-libraries.spec 

Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.iEGOhY
+ umask 022
+ cd /home/mk/rpmbuild/BUILD
+ cd adobe_root
+ cd adobe_source_libraries
+ bjam link=static toolset=gcc variant=release asl_dev
Unable to load Boost.Build: could not find build system.
---------------------------------------------------------
/home/mk/rpmbuild/BUILD/adobe_root/adobe_source_libraries/boost-build.jam attempted to load the build system by invoking

Updating get-source.sh to exclude adobe_platform_libraries which not are used. Update done in-place, same links.

(In reply to comment #24)
> For me, they seem to be. How did you test?

The SPEC needs:
BuildRequires:  boost-build

then the mock build works for me.

Now, that was a a review remark worth waiting for! Everything becomes just so much simpler. Blushing... New links:
http://dl.dropbox.com/u/17870887/adobe-6/adobe-source-libraries.spec
http://dl.dropbox.com/u/17870887/adobe-6/adobe-source-libraries-1.0.43-6.fc16.src.rpm

It is much better now.


Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[?]: MUST Changelog in prescribed format.
Comment bellow.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[-]: MUST Macros in Summary, %description expandable at SRPM build time.
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Fully versioned dependency in subpackages, if present.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: 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 is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
[x]: MUST License file installed when any subpackage combination is installed.
[!]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
Comment bellow.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Requires correct, justified where necessary.
[!]: MUST Rpmlint output is silent.
Comment bellow.
[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/tmp/adobe/asl_1.0.43.tgz :
  MD5SUM this package     : eac9f3eec40ed1f41d1e4671289b5e8b
  MD5SUM upstream package : 04b18d3b682008146f76fc0dcc19c4c9

[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[x]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.
[-]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[x]: SHOULD Package functions as described.

Very simple functional test passed.

[x]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: SHOULD Scriptlets must be sane, if used.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source0: http://sourceforge.net/projects/adobe-source/files/adobe-
     source/%{version}/asl_%{version}.tgz (asl_%{version}.tgz) Source1: adobe-
     source-libraries.pc (adobe-source-libraries.pc) Patch0: asl-gcc.patch
     (asl-gcc.patch) Patch1: asl-fedora-build-options.patch (asl-fedora-build-
     options.patch)
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.

Only x86_64 tested.

[-]: SHOULD %check is present and all tests pass.
[-]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

Issues:
[!]: MUST Rpmlint output is silent.
  adobe-source-libraries-devel.x86_64: W: no-documentation
    > Probably OK
  adobe-source-libraries-doc.x86_64: E: zero-length /usr/share/doc/adobe-source-libraries-doc-1.0.43/documentation/performance/index.html
    > Please query upstream about this file, also nice to have: [ -s documentation/performance/index.html ] || rm -f documentation/performance/index.html
  6 packages and 1 specfiles checked; 1 errors, 1 warnings.

[?]: MUST Changelog in prescribed format.
  It seems not to be against the guidelines, but the changelog from example differs:
  http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
  So I would prefer not using the space indentation for version-release suffix and also not inserting VIM metadata in the end of SPEC file (# vim: tabstop=4:expandtab).

[!]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
  $RPM_BUILD_ROOT is used, according to other vars/macros usage %{buildroot} should be used instead.

[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/tmp/adobe/asl_1.0.43.tgz :
  MD5SUM this package     : eac9f3eec40ed1f41d1e4671289b5e8b
  MD5SUM upstream package : 04b18d3b682008146f76fc0dcc19c4c9

  Both tarballs are OK, but wrong URL is used in sources - it leads to HTML file, not to the archive -> md5sum mismatch
  Please use sourceforge URL as noted in: http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net

[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source0: http://sourceforge.net/projects/adobe-source/files/adobe-
     source/%{version}/asl_%{version}.tgz (asl_%{version}.tgz) Source1: adobe-
     source-libraries.pc (adobe-source-libraries.pc) Patch0: asl-gcc.patch
     (asl-gcc.patch) Patch1: asl-fedora-build-options.patch (asl-fedora-build-
     options.patch)
  
Not blocker, nice to have.

Other:
For adobe-source-libraries-doc package, I would rather see the doc in /usr/share/doc/adobe-source-libraries-1.0.43 directory instead of
/usr/share/doc/adobe-source-libraries-doc-1.0.43

Generated by fedora-review 0.1.3
External plugins:

(In reply to comment #28)
> It is much better now.

Yup. Thanks to you 

[cut]

>   adobe-source-libraries-doc.x86_64: E: zero-length
> /usr/share/doc/adobe-source-libraries-doc-1.0.43/documentation/performance/index.html
>     > Please query upstream about this file, also nice to have: [ -s
> documentation/performance/index.html ] || rm -f
> documentation/performance/index.html

Fixed, link in spec.

> [?]: MUST Changelog in prescribed format.
>   It seems not to be against the guidelines, but the changelog from example
> differs:
>   http://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
>   So I would prefer not using the space indentation for version-release suffix
> and also not inserting VIM metadata in the end of SPEC file (# vim:
> tabstop=4:expandtab).

It's just kind of a habit, and the two other packages I own have this layout. They where reviewed by my sponsor, and he accepted as soon he understood it was on purpose. But if you insist, I will of course remove the whitespace. The vim metadata is removed
 
> [!]: MUST Package consistently uses macros (instead of hard-coded directory
>      names).
>   $RPM_BUILD_ROOT is used, according to other vars/macros usage %{buildroot}
> should be used instead.

Fixed

> [!]: MUST Sources used to build the package match the upstream source, as
[cut]
>   Please use sourceforge URL as noted in:
> http://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net

Fixed. The reference URL doesn't work, I tried to stay as close as possible.

> [!]: SHOULD SourceX / PatchY prefixed with %{name}.
[cut]
> Not blocker, nice to have.

and prefixing w adobe-source-libraries instead of asl would give insanely long names...

> 
> Other:
> For adobe-source-libraries-doc package, I would rather see the doc in
> /usr/share/doc/adobe-source-libraries-1.0.43 directory instead of
> /usr/share/doc/adobe-source-libraries-doc-1.0.43

Yet another good spot. Fixed.

New links:
http://dl.dropbox.com/u/17870887/adobe-7/adobe-source-libraries.spec
http://dl.dropbox.com/u/17870887/adobe-7/adobe-source-libraries-1.0.43-7.fc16.src.rpm

(In reply to comment #29)
> But if you insist, I will of course remove the whitespace
No problem with the current state, feel comfortable with you package.

Other issues:
The documentation is now packaged twice (in the base package and in the doc subpackage). It is sideeffect of rpmbuild, it can be avoided by e.g. %exclude.

I would probably remove the doxygen files from the docs and let there only the html/images.

(
> The documentation is now packaged twice 

Arrrgh... Fixed

> I would probably remove the doxygen files from the docs and let there only the
> html/images.

Once again, you're right. Fixed

Modifications in place, same links, still release 7, documented in changelog.

Some minor problems:

rpmlint ./adobe-source-libraries.spec 
./adobe-source-libraries.spec:123: W: macro-in-%changelog %exclude)
0 packages and 1 specfiles checked; 0 errors, 1 warnings.

Use '%%exclude' or 'exclude macro' in changelog.

I would probably remove the 'doxyfile' from docs.
I think it make only sense to pack doc/examples, doc/html, doc/templates, the other dirs are empty (contained the *.dox before) or are helpers for build process.

(In reply to comment #32)

> ./adobe-source-libraries.spec:123: W: macro-in-%changelog %exclude)

Fixed. And this time, I run rpmlint myself before submitting.

> I would probably remove the 'doxyfile' from docs.

Yes, of course. Fixed

> I think it make only sense to pack doc/examples, doc/html, doc/templates, the
> other dirs are empty (contained the *.dox before) or are helpers for build
> process.

Indeed, the html docs seems to work anyway(!)  Fixed

Giving fedora review +

New Package SCM Request
=======================
Package Name: adobe-source-libraries
Short Description: General Purpose Addon C++ libs for Boost and STL
Owners: leamas
Branches: f15 f16 f17 rawhide
InitialCC:

Invalid branch requested, rawhide==devel and is automatically provided. 
Summary and SCM request package name differ, please fix.

Sorry, this is not completely clear to me...

New Package SCM Request
=======================
Package Name: adobe-source-libraries
Short Description: General purpose C++ libraries
Owners: leamas
Branches: f15 f16 f17
InitialCC:

Should set fedora-cvs flag as well

Summary must match the package name.

Git done (by process-git-requests).

f15,f16,f17 built OK

Hi Alex, congrats on getting asl into Fedora.

Just a note regarding the changelog: there shouldn't be duplicate
version-release entries listed. In Fedora, you must always increment the
release when committing / building a package. 

If you are working on updates privately (while developing the changes), you
could simply add the extra change comments to the one changelog entry.

adobe-source-libraries-1.0.43-7.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/adobe-source-libraries-1.0.43-7.fc15

adobe-source-libraries-1.0.43-7.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/adobe-source-libraries-1.0.43-7.fc16

adobe-source-libraries-1.0.43-7.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/adobe-source-libraries-1.0.43-7.fc17

(In reply to comment #42)
> Hi Alex, congrats on getting asl into Fedora.
> 
> Just a note regarding the changelog: there shouldn't be duplicate
> version-release entries listed. In Fedora, you must always increment the
> release when committing / building a package. 
> 
I also dislike it, but IMHO it should be OK if there was not no build:
http://fedoraproject.org/wiki/Packaging:Guidelines#Repeat_the_old_version_release_with_a_new_entry

adobe-source-libraries-1.0.43-8.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/adobe-source-libraries-1.0.43-8.fc15

adobe-source-libraries-1.0.43-7.fc16 has been pushed to the Fedora 16 stable repository.