Bug 225622 - Merge Review: boost
Summary: Merge Review: boost
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matěj Cepl
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 17:46 UTC by Nobody's working on this, feel free to take it
Modified: 2018-04-11 12:02 UTC (History)
14 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-05-02 06:43:45 UTC
Type: ---
Embargoed:
mcepl: fedora-review+


Attachments (Terms of Use)
spec file diff implementing merge review proposals (6.55 KB, patch)
2007-04-08 21:00 UTC, Patrice Dumas
no flags Details | Diff
remove compile flags, some optionally, and soname (2.24 KB, patch)
2007-04-08 21:04 UTC, Patrice Dumas
no flags Details | Diff
implement review remarks (6.73 KB, patch)
2007-04-20 15:30 UTC, Patrice Dumas
no flags Details | Diff
rpmlint log about boost packages (100.99 KB, text/plain)
2007-11-12 12:49 UTC, Laurent Rineau
no flags Details
cleanups explained above and excerpted from previous patch rebased (2.04 KB, patch)
2007-11-12 21:10 UTC, Patrice Dumas
no flags Details | Diff

Description Nobody's working on this, feel free to take it 2007-01-31 17:46:27 UTC
Fedora Merge Review: boost

http://cvs.fedora.redhat.com/viewcvs/devel/boost/
Initial Owner: bkoz

Comment 1 Benjamin Kosnik 2007-01-31 18:36:05 UTC
What is to be done?

Comment 2 Patrice Dumas 2007-02-17 22:53:06 UTC
The package should be adjusted to adhere to the fedora packaging 
guidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
for the merge of fedora Core and fedora Extras.

Issues:

* the Source should lead to a real url

* BuildRoot is not the preferred one

* PreReq should be replaced by the appropriate Requires(post)....
In the case of boost, an even better solution would be to use

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

* BuildRequires of libs shouldn't be necessary, they are brought in by 
  the -devel, so the following should be removed:
BuildRequires: python
BuildRequires: bzip2-libs
BuildRequires: zlib
BuildRequires: libicu

* there shouldn't be a mail sent for the test results in the default 
  case. If you really want it, I think you should consider using a
  conditional.

* Boost Software License seems to me to be very similar with the MIT 
  license. Maybe MIT-like could be used?

* in the %doc of the main package there should certainly be LICENSE_1_0.txt
  README, and many html files from the source directory, for example the 
  faq, but also many others.

* why don't you use bjam for installing?

* in the doc subpackage the directory should be tagged with %doc.

* what you do with soname is dubious. Why don't you use the upstream
  numbering?

* the %optflags are not used during the build.

* It is not very clear to me whether the devel package requires zlib-devel,
  bzip2-devel, and so on, or not.

* there is a very strange
Obsoletes: boost-doc <= 1.30.2

* the main package should certainly 
Provides: boost-python = %{version}-%{release}

* rpmlint shows that
 - there are bad perms for static libs, they should be 0644
 - some source files have bad perms, they shouldn't be executables
 - there are some scripts mixed with the headers, that were certainly
   used during build, they should be removed.
and
W: boost macro-in-%changelog check
W: boost rpm-buildroot-usage %prep rm -rf $RPM_BUILD_ROOT
E: boost no-cleaning-of-buildroot %install

There are also many undefined-non-weak-symbol
W: boost undefined-non-weak-symbol /usr/lib/libboost_python.so.1.33.1
PyExc_ImportError
for libboost_python, the python library should certainly be used during
the link of that library.

* the static libraries should certainly be moved to another
  subpackage like boost-static or boost-devel-static or something
  similar

* mkdir -p $RPM_BUILD_ROOT%{_docdir}
is unuseful

* add dots in %description

* -doc should be in Group Documentation (although Group doesn't matter much)
  and -devel in Development/Libraries

* it seems to me that -doc shouldn't require the main package.

* you should keep the timestamps for doc and headers by using -p

Suggestions:

* add / in %files to directory, to show visually that these are directories
  and not files

* use %defattr(-, root, root, -) instead of %defattr(-, root, root)

* put the html doc in the -doc subbpackage docdir and not in the main 
  package docdir, using %doc


Comment 3 Hans de Goede 2007-03-22 21:43:02 UTC
Repeating from above:

There are also many undefined-non-weak-symbol
W: boost undefined-non-weak-symbol /usr/lib/libboost_python.so.1.33.1
PyExc_ImportError
for libboost_python, the python library should certainly be used during
the link of that library.

Yes, that would also have caught bug 233523 way earlier through rpm deps breaking.

Also as I'm recompiling boost now because of bug 233523, you need to make sure
RPM_OPT_FLAGS get used during compile, currently that isn't happening.



Comment 4 Benjamin Kosnik 2007-03-27 17:39:14 UTC
I took a first crack at this with boost-1.33.1-12. It doesn't get to all the
points on the list but many of them. The rest are pending.

-benjamin

Comment 5 Benjamin Kosnik 2007-03-28 10:05:19 UTC
Hans suggests adding:

 -lpython should be added to the libboost_python LDFLAGS/LIBS?

from 

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=233523

Comment 6 Benjamin Kosnik 2007-03-28 10:07:57 UTC
In general, patches and more elaborate descriptions are welcome for these
remaining items.

Ie:

(good): lpython should be added to the libboost_python LDFLAGS/LIBS

is better than

(?) add dots in %description

-benjamin

Comment 7 Patrice Dumas 2007-03-28 10:17:01 UTC
Please repost the list of item you want that I elaborate on, I don't
know exactly which ones were fixed. My points may be terse, but I 
am ready to respond to any question and elaborate on any point if 
you think it isn't clear. I can even send patches if you give 
me directions on what you want to fix yourself and what you'd like to
be helped with.

About the description, there were final dots missing in %description
but they seem to be all fixed. 

Comment 8 Benjamin Kosnik 2007-03-28 10:27:08 UTC
Fixed in -12:
* BuildRoot is not the preferred one

* PreReq should be replaced by the appropriate Requires(post)....
In the case of boost, an even better solution would be to use

* Boost Software License seems to me to be very similar with the MIT 
  license. Maybe MIT-like could be used?

* BuildRequires of libs shouldn't be necessary, they are brought in by 
  the -devel, so the following should be removed:

* It is not very clear to me whether the devel package requires zlib-devel,
  bzip2-devel, and so on, or not.

* there is a very strange
Obsoletes: boost-doc <= 1.30.2

* the main package should certainly 
Provides: boost-python = %{version}-%{release}

- there are bad perms for static libs, they should be 0644

* the static libraries should certainly be moved to another
  subpackage like boost-static or boost-devel-static or something
  similar 
 
Fixed, but I think probably boost-devel-static would be better. Thoughts? I
think this name is ok according to the guidelines, but what are other packages
doing WRT this issue?

* mkdir -p $RPM_BUILD_ROOT%{_docdir}
is unuseful

* add dots in %description

* -doc should be in Group Documentation (although Group doesn't matter much)
  and -devel in Development/Libraries

* it seems to me that -doc shouldn't require the main package.

* you should keep the timestamps for doc and headers by using -p

* use %defattr(-, root, root, -) instead of %defattr(-, root, root)

* put the html doc in the -doc subbpackage docdir and not in the main 
  package docdir, using %doc

Remaining MUST:
* rpmlint shows that

* libboost_python issues

* install permissions issues

* the %optflags are not used during the build.

Remaining SHOULD:

 there shouldn't be a mail sent for the test results

Comment 9 Benjamin Kosnik 2007-03-28 10:32:08 UTC
1) why not use boost-jam for install?

It provides no advantage when we are doing staged builds, and also doesn't work
with prefix. In addition, it doesn't get the permissions correct. I'm not quite
sure why the permissions are incorrect in rpmlint considering they are
explicitly set by install to be the correct values. Any hacking by others in
this area would be appreciated.

2) soname

What upstream boost does with soname is dubious IMHO. In particular, boost libs
should not change SONAMES based on gcc versions if gcc versions are compat. Ie,
gcc-3.4, gcc-4.0, gcc-4.1 are compat. If using upstream boost versioning, they
are not. 

In general, there is no ABI checking in upstream boost. Fedora does not have
this luxury.

Mostly, they leave this as a decision for vendors, one of whom is Fedora. The
plan WRT Fedora is to provide some guidance for people using older boosts that
are not ABI-compat with current boost. Thus the soname bump. 

Comment 10 Ralf Corsepius 2007-03-28 10:35:08 UTC
(In reply to comment #8)
> * the static libraries should certainly be moved to another
>   subpackage like boost-static or boost-devel-static or something
>   similar 
>  
> Fixed, but I think probably boost-devel-static would be better. Thoughts?
Feel strongly encouraged to drop them, c.f.
http://fedoraproject.org/wiki/Packaging/Guidelines#head-82d97fc4a3421310f4e2971180e4165965b65662

Comment 11 Patrice Dumas 2007-04-08 21:00:57 UTC
Created attachment 151940 [details]
spec file diff implementing merge review proposals

Comment 12 Patrice Dumas 2007-04-08 21:04:02 UTC
Created attachment 151941 [details]
remove compile flags, some optionally, and soname

Comment 13 Patrice Dumas 2007-04-08 21:08:25 UTC
My patch replaces boost-gcc-tools.patch and boost-cxxflags-debug.patch.
I think that it could be submitted upstream.

Otherwise among the changes in the spec, I renamed
devel-static to static-devel to shut up rpmlint.
I readded Obsoletes, they are right.

Comment 14 Benjamin Kosnik 2007-04-10 09:13:24 UTC
Thanks Patrice.

#11 looks good for me. 

As far as devel-static vs. static-devel vs. static, I don't see any other
packages using static-devel. Do you? If not, why not? Is this something that
should be asked on fedora-devel?

#12 detail why you're doing the inlining and optimization changes. In addition,
as long as you're doing this, you might as well do %optflags changes too.

best,
benjamin


Comment 15 Patrice Dumas 2007-04-10 11:30:55 UTC
(In reply to comment #14)
> Thanks Patrice.
> 
> #11 looks good for me. 
> 
> As far as devel-static vs. static-devel vs. static, I don't see any other
> packages using static-devel. Do you? If not, why not? Is this something that
> should be asked on fedora-devel?

The guideline regarding static libs going in a separate package is
relatively new. I agree it is a good idea to have a separate package 
for the static libraries. I personally don't care whether it is named
-static, -static-devel, -devel-static and the guidelines leave this detail 
to the packager. I choosed -static-devel to silent rpmlint.

> #12 detail why you're doing the inlining and optimization changes. In addition,
> as long as you're doing this, you might as well do %optflags changes too.

I am not doing an inlining and optimization change, I am allowing the
user building boost to remove completely the <optimization> and 
<inlining> flags. Is there an other way?

Regarding the optflags, it would be bad to hardcode them in a patch, they
change depending on the platform, release, and so on and so forth. I
pass them through 
export GXX="%__cxx $RPM_OPT_FLAGS"
I couldn't find a way to pass them with <cxxflags> since, unless I'm wrong,
<cxxflags> only allows to pass them one by one.

And I also set  <optimization>no <inlining>no to avoid any <optimization>
and <inlining> flags to be set, such that they don't overwrite what is 
set in the optflags (and this is allowed by the patch in comment #12).


As a side note I had quite a hard time with bjam. It lacks a bit of
documentation and examples. I couldn't understand how to pass the python
linking flags that would allow to solve the underlinking issue, 
as explained in comment #5.

Comment 16 Patrice Dumas 2007-04-10 12:47:47 UTC
(In reply to comment #9)
> 1) why not use boost-jam for install?
> 
> It provides no advantage when we are doing staged builds, and also doesn't work
> with prefix. In addition, it doesn't get the permissions correct. I'm not quite
> sure why the permissions are incorrect in rpmlint considering they are
> explicitly set by install to be the correct values. Any hacking by others in
> this area would be appreciated.

I added a comment in my spec patch summarizing your point.

> 2) soname
> 
> What upstream boost does with soname is dubious IMHO. In particular, boost libs
> should not change SONAMES based on gcc versions if gcc versions are compat. Ie,
> gcc-3.4, gcc-4.0, gcc-4.1 are compat. If using upstream boost versioning, they
> are not. 
> 
> In general, there is no ABI checking in upstream boost. Fedora does not have
> this luxury.
>
> Mostly, they leave this as a decision for vendors, one of whom is Fedora. The
> plan WRT Fedora is to provide some guidance for people using older boosts that
> are not ABI-compat with current boost. Thus the soname bump. 

I don't understand exactly what you are meaning. With the current 
patcheset, and without changing soname, the soname version used
is the boost version. This seems to be right, if you are saying that
"Fedora does not have the luxury to check the boost ABI change",
since it means that the soname has to be changed for every boost 
release.

In that case the library name could be like
libboost_python.so.1.33.1
the soname would be 
libboost_python.so.1.33.1
and there would be a so link in devel
libboost_python.so
pointing to libboost_python.so.1.33.1

You may also be saying the reverse, namely that you check the ABI
compatibility and you don't break ABI for each release, that's why
you need a soname version that don't use the boost version, but 
instead an integer you bump only when there has been an ABI 
change. Is is the case?



(As a side note, even without boost-base.patch applied the gcc 
version isn't hardcoded in the soname. The soname is like:
libboost_python-gcc-1_33_1.so.1.33.1
(or libboost_python-gcc-1_33_1.so.2 with <sonameversion>2 and
my patch or, I guess, the previous patch).)

Comment 17 Patrice Dumas 2007-04-20 15:28:32 UTC
In fact the guidelines prefer -static, I'll update my patch.

Comment 18 Patrice Dumas 2007-04-20 15:30:40 UTC
Created attachment 153190 [details]
implement review remarks

replace static-devel with static

Comment 19 Benjamin Kosnik 2007-05-17 15:49:25 UTC
Hey! This is just a notice that with the new boost release, and after fedora 7
gets out the door, I intend to start hacking on this stuff again, with the goal
of merging in as much of Patrice's work, then moving to the boost-1.34 codebase
and that work from january on the srpms, and then rebasing f7 for this new
version of boost.

Thanks for your patience.

Comment 20 Kevin Fenzi 2007-06-09 04:36:00 UTC
Hey Patrice: Are you reviewing this package? Or just providing pre-review comments?

If you are reviewing it, can you set fedora-review to ? and assign it to yourself? 

If not, this should be assigned to 'nobody' until a reviewer comes along. 

Comment 21 Patrice Dumas 2007-06-10 19:56:49 UTC
Yeah, I can consider that I am reviewing it. But I am very
open to somebody else doing the formal review.

Comment 22 Patrice Dumas 2007-06-26 09:35:00 UTC
There is a new release, 1.34.0

Comment 23 Benjamin Kosnik 2007-06-29 13:03:55 UTC
I'm waiting for boost-1.34.1, actually. 

I put up tentative rpms for this new boost code base here:

http://people.redhat.com/bkoz/boost-1.34.1/

There are some issues with the boost-devel package, in that the .so symlinks
have relative path names (that are incorrect). Also, I'll need to run this
through the build process to determine the SONAME, which will probably be bumped
to .3 depending on ABI issues.

I also disabled testing for release. The srpm from boost-1.34.0.* posted here:
http://lists.boost.org/Archives/boost/2007/01/115877.php

Should have these patches. (Which will have to be revisted.)

Please base any future patches off this code base.

best,
benjamin

Comment 24 Ed Hill 2007-09-23 22:47:12 UTC
Hi Benjamin,

The people.redhat.com URL in #23 returns a 404 for me.  So I grabbed the 
latest boost SRPM from devel and I'd like to (please!) request that you 
replace the line:

  (cd tools/jam/src && ./build.sh)

with

  (cd tools/jam/src && CFLAGS=$RPM_OPT_FLAGS ./build.sh cc )

and then please consider shipping the bjam executable either as a part of 
boost-devel or perhaps as a boost-bjam sub-package.  I'll gladly submit a
patch if that helps.

thanks!
Ed



Comment 25 Laurent Rineau 2007-11-12 12:49:48 UTC
Created attachment 255181 [details]
rpmlint log about boost packages

Can somebody update the status of this bug?

Who is submitting the review request? Benjamin seems to be the maintainer of
the package.

Who is reviewing it? Patrice, are you still on it?

rpmlint is very versatile about boost's packages. I attach its log.

The license issue is a blocker, as far as I understand the guidelines.

Comment 26 Patrice Dumas 2007-11-12 21:10:55 UTC
Created attachment 255781 [details]
cleanups explained above and excerpted from previous patch rebased

Comment 27 Patrice Dumas 2007-11-12 21:15:48 UTC
As I said in Comment 21:
Yeah, I can consider that I am reviewing it. But I am very
open to somebody else doing the formal review.

Anybody can reassign to himself with my blessing. I never
care about being assigned anyway, I care about having
my comments acted upon, though ;-).

Also I still have to look at the build, it seems that
some things that I did in the previous patch may be still needed.
And Ed also made a comment.

Comment 28 Patrice Dumas 2008-01-28 20:28:51 UTC
The patch is still to be considered. I can rebase it, but
I believe it will still apply.

Comment 29 Petr Machata 2008-03-27 13:29:59 UTC
While fixing #437032, I addressed following concerns:

* in the %doc of the main package there should certainly be LICENSE_1_0.txt
  README, and many html files from the source directory, for example the 
  faq, but also many others.
  [We currently distribute "lib", "doc" and "more" directories.]

From Patrice's patch:
* BuildRequires/Requires/Provides cleanups
* %prep and %check cleanups
* rename to boost-static, which requires that boost-static obsoletes and 
  provides old boost-devel-static in addition to mere rename.

Moreover, I revamped %install section so that it doesn't launch one "install"
per file, but chains installs together via xargs where possible.  This simple
change actually improves overall build time considerably.

Some of these changes are in not-yet-built -14 release of the package, some of
them are already in -13 which was built yesterday.

Comment 30 Patrice Dumas 2008-04-05 00:16:29 UTC
Unless I missed something, the LICENSE_1_0.txt and README are not in the main
package, would be better if they were there.

You should drop
BuildRequires: bzip2-libs

The license tag should simply be: Boost

Maybe the flag could be
--with-python-root=%{_prefix}
instead of
--with-python-root=/usr

Comment 31 Patrice Dumas 2008-04-05 19:31:43 UTC
1.35.0 is out, and there are certainly fixes to allow to have cxxflags be put
after default flags.

Comment 32 Susi Lehtola 2009-05-03 07:15:45 UTC
Ping?

It seems the package still does not use %{optflags}

Comment 33 Denis Arnaud 2009-05-04 01:27:44 UTC
Besides, for Boost 1.37 (Fedora 11 / Rawhide), there seems to be an issue with the dependency on libicu, as in the build.log of RMOL (http://koji.fedoraproject.org/koji/getfile?taskID=1334520&name=root.log):

DEBUG backend.py:554:  /usr/bin/yum --installroot /var/lib/mock/dist-f12-build-444978-72210/root/  resolvedep  'libicu' 'gsl-devel >= 1.8' 'ghostscript' 'cppunit-devel >= 1.10' 'texlive-latex' 'boost-devel >= 1.34' 'doxygen' 'texlive-dvips'
DEBUG util.py:280:  Executing command: /usr/bin/yum --installroot /var/lib/mock/dist-f12-build-444978-72210/root/  resolvedep  'libicu' 'gsl-devel >= 1.8' 'ghostscript' 'cppunit-devel >= 1.10' 'texlive-latex' 'boost-devel >= 1.34' 'doxygen' 'texlive-dvips'
DEBUG util.py:256:  0:libicu-4.2-0.1.d03.fc12.x86_64
DEBUG util.py:256:  0:gsl-devel-1.12-3.fc11.x86_64
DEBUG util.py:256:  0:ghostscript-8.64-6.fc12.x86_64
DEBUG util.py:256:  0:cppunit-devel-1.12.1-2.fc11.x86_64
DEBUG util.py:256:  0:texlive-latex-2007-42.fc11.x86_64
DEBUG util.py:256:  0:boost-devel-1.37.0-6.fc11.x86_64
DEBUG util.py:256:  1:doxygen-1.5.8-2.fc11.x86_64
DEBUG util.py:256:  0:texlive-dvips-2007-42.fc11.x86_64
DEBUG util.py:319:  Child returncode was: 0
DEBUG backend.py:554:  /usr/bin/yum --installroot /var/lib/mock/dist-f12-build-444978-72210/root/  install  'libicu' 'gsl-devel >= 1.8' 'ghostscript' 'cppunit-devel >= 1.10' 'texlive-latex' 'boost-devel >= 1.34' 'doxygen' 'texlive-dvips'
DEBUG util.py:280:  Executing command: /usr/bin/yum --installroot /var/lib/mock/dist-f12-build-444978-72210/root/  install  'libicu' 'gsl-devel >= 1.8' 'ghostscript' 'cppunit-devel >= 1.10' 'texlive-latex' 'boost-devel >= 1.34' 'doxygen' 'texlive-dvips'
DEBUG util.py:256:  boost-1.37.0-6.fc11.x86_64 from build has depsolving problems
DEBUG util.py:256:    --> Missing Dependency: libicuuc.so.40()(64bit) is needed by package boost-1.37.0-6.fc11.x86_64 (build)
DEBUG util.py:256:  boost-1.37.0-6.fc11.x86_64 from build has depsolving problems
DEBUG util.py:256:    --> Missing Dependency: libicui18n.so.40()(64bit) is needed by package boost-1.37.0-6.fc11.x86_64 (build)
DEBUG util.py:256:  Error: Missing Dependency: libicuuc.so.40()(64bit) is needed by package boost-1.37.0-6.fc11.x86_64 (build)
DEBUG util.py:256:   You could try using --skip-broken to work around the problem
DEBUG util.py:256:  Error: Missing Dependency: libicui18n.so.40()(64bit) is needed by package boost-1.37.0-6.fc11.x86_64 (build)
DEBUG util.py:256:   You could try running: package-cleanup --problems
DEBUG util.py:256:                          package-cleanup --dupes
DEBUG util.py:256:                          rpm -Va --nofiles --nodigest
DEBUG util.py:256:  The program package-cleanup is found in the yum-utils package.
DEBUG util.py:319:  Child returncode was: 1

Comment 34 Michael Schwendt 2009-05-04 07:19:27 UTC
Denis, check your build target. It's dist-f12 not dist-f11. In dist-f12 (Fedora 12 development) there's a fresh ABI-incompatible icu.

Comment 35 Denis Arnaud 2009-05-04 18:15:47 UTC
(In reply to comment #34)
> Denis, check your build target. It's dist-f12 not dist-f11. In dist-f12 (Fedora
> 12 development) there's a fresh ABI-incompatible icu.  

You are right, of course. However, the (Fedora CVS) devel branch now corresponds to F12 (and no longer to F11). And, as far as I understand (from https://fedoraproject.org/wiki/PackageMaintainers/Join#Request_Builds), in order to submit an update of RMOL (bug 489233), I first need to have RMOL build successfully on the devel branch, that is, for F12.

I have tried to add explicit dependency on libicu (and libicu-devel) in the RPM specification file, but the build did not succeed either. The issue seems to come from a non-explicit dependency of Boost-1.37. That's why I posted my comment on this bug report (but maybe bug 496188 is more appropriate?).

I have no Rawhide distribution, so I cannot test it directly on my machine. Any feedback will be welcome on how I should proceed (to be able to build the new version of RMOL).

Comment 36 Michael Schwendt 2009-05-04 18:39:46 UTC
You can submit builds for each branch in cvs separately.

devel : currently won't build, as somebody would first need to do a rebuild of boost (for the new libicu) -- I could submit a rebuild attempt in koji but the provenpackagers guidelines are not clear on that unfortunately -- packages that build successfully in devel will be published in Rawhide automatically (on the next day) but only as soon as the F-11 freeze is over

F-11 F-10 F-9 : builds done for these targets need to be released as updates via the Fedora Updates System (bodhi)

If you have other questions, let's do it in private mail as not to flood this Merge Review ticket. ;)

Comment 37 Till Maas 2009-09-16 23:42:28 UTC
Please address all other issues mentioned here.

Also I noted the following:
- Source is not a valid URL
- Source should iirc be Source0 instead
- The patches do not have any upstream status comments:
https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

Please remove the NotReady from the Whiteboard, once all known issues are addressed and action from a reviewer is needed again.

Btw. there is also a new release available, maybe this allows to remove some patches. It is also already possible to branch for F12, so that the spec in devel can be fixed for F13, in case this is somehow necessary.

Comment 38 Jan Kratochvil 2010-03-09 20:17:01 UTC
obsoleted:
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

Missing %{?_isa} tags for multilib installations:
{,Build}Requires: *-devel -> {,Build}Requires: *-devel%{?_isa}
(sure gettext flex bison etc. dependencies should be without %{?_isa})
http://fedoraproject.org/wiki/PackagingDrafts/ArchSpecificRequires

Comment 39 Matěj Cepl 2011-02-12 22:26:44 UTC
I guess results of the package review at bug 673839 should be relevant here as well.

Comment 40 Denis Arnaud 2011-08-17 12:41:00 UTC
The "mainstream" Boost package is now tracked by #711845. So, this bug could be closed.

Comment 41 Matěj Cepl 2011-08-18 10:22:40 UTC
(In reply to comment #40)
> The "mainstream" Boost package is now tracked by #711845. So, this bug could be
> closed.

No, this is not right ... this bug is a merge review (a special kind of package review), not bothering about need to upgrade.

Comment 42 Petr Machata 2011-08-19 14:33:14 UTC
Hm, this bug fell off my radar.  I made some amendments some time ago, but forgot to put myself on the CC list.  Thanks for putting me back in loop.  Matej is of course right, the points raised by others need to be addressed.

Comment 43 Petr Machata 2011-08-30 17:37:36 UTC
I addressed the comments that I found here or in bug 673839.  The updated package is in the repository.  Partial scratch build is available here:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=3311740

This failed on x86_64 for some reason—make is not found mid-installation.  I don't know what this could have been caused by, but it's not the first time that I see this.  It usually goes away on rebuild, which I didn't attempt at this time.

Comment 44 Gwyn Ciesla 2012-04-05 13:25:29 UTC
Ping?

Comment 45 Petr Machata 2012-04-05 15:28:50 UTC
I uploaded the package for review here:
  http://people.redhat.com/~pmachata/boost-review/

note that that's simply today's master.  Looking into what's in GIT will give you up to date results.

Comment 46 Matěj Cepl 2012-04-05 16:19:41 UTC
Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ MUST: rpmlint must be run on every package. The output should be posted in
the review

mitmanek:boost $ rpmlint -i boost-1.48.0-10.fc18.src.rpm 
boost.src: W: patch-not-applied Patch1: boost-cmake-soname.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

boost.src: W: patch-not-applied Patch12: boost-1.48.0-polygon.patch
A patch is included in your package but was not applied. Refer to the patches
documentation to see what's wrong.

boost.src: W: no-rhel-conditional boost.spec
There is a check for fedora version but no check for version of RHEL.

1 packages and 0 specfiles checked; 0 errors, 3 warnings.
mitmanek:boost $ 

Packager, could you please comment on these, specifically I am a little bit concerned about the last one (I don't care that much whether you keep some patch in the package without applying it). Of course, if you have different specs in different branches and you don't merge them it is OK, otherwise it may get problematic in some future version of RHEL.

+ MUST: package named according to the Package Naming Guidelines
+ MUST: The spec file name must match the base package %{name}
+ MUST: The package must meet the Packaging Guidelines .
+ MUST: The package licensed with a Fedora approved license and meets the
Licensing Guidelines
+ MUST: The License field in the package spec file matches the actual
license
Boost and MIT and Python
+ 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.
License is included.
+ MUST: The spec file must be written in American English.
+ MUST: The spec file for the package MUST be legible.
+ MUST: The sources used to build the package must match the upstream
source, as provided in the spec URL. Reviewers should use md5sum for this task
MD5: d1e9a7a7f532bb031a3c175d86688d95
+ MUST: The package successfully compiles and builds into binary rpms on at
least one primary architecture - built many times in koji and brew
0 MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch
+ MUST: All build dependencies must be listed in BuildRequires, except for any
that are listed in the exceptions section of the Packaging Guidelines
0 MUST: The spec file handles locales properly. This is done by using the
%find_lang macro
No locales are present.
+ MUST: Every binary RPM package (or subpackage) which stores shared library
files (not just symlinks) in any of the dynamic linker's default paths, must
call ldconfig in %post and %postun.
+ MUST: Packages must NOT bundle copies of system libraries
0 MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review, along with the rationalization for
relocation of that specific package. Without this, use of Prefix: /usr is
considered a blocker
+ MUST: Package must own all directories that it creates. If it does not create
a directory that it uses, then it should require a package which does create
that directory
+ MUST: Package must not list a file more than once in the spec file's %files
listings
+ MUST: Permissions on files must be set properly. Every %files section must
include a %defattr(...) line.
+ MUST: Each package must have a %clean section, which contains rm -rf
%{buildroot} (or $RPM_BUILD_ROOT).
+ MUST: Each package must consistently use macros
+ MUST: The package must contain code, or permissible content
+ MUST: Large documentation files must go in a -doc subpackage
+ MUST: If a package includes something as %doc, it must not affect the runtime
of the application
+ MUST: Header files must be in a -devel package
+ MUST: Static libraries must be in a -static package
0 MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
+ MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel package
+ MUST: devel packages must require the base package using a fully versioned
dependency: Requires: %{name} = %{version}-%{release}
+ MUST: Packages must NOT contain any .la libtool archives, these must be
removed in the spec if they are built
0 MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section
+ MUST: Packages must not own files or directories already owned by other
packages
+ MUST: At the beginning of %install, each package MUST run rm -rf %{buildroot}
(or $RPM_BUILD_ROOT)
+ MUST: All filenames in rpm packages must be valid UTF-8

Please just comment on the result of rpmlint and I think we can close this.

Comment 47 Petr Machata 2012-04-30 11:04:43 UTC
Thanks for the review.

(In reply to comment #46)
> mitmanek:boost $ rpmlint -i boost-1.48.0-10.fc18.src.rpm 
> boost.src: W: patch-not-applied Patch1: boost-cmake-soname.patch

It is applied, just in a roundabout way:
sed 's/_FEDORA_SONAME/%{sonamever}/' %{PATCH1} | %{__patch} -p0 --fuzz=0

> boost.src: W: patch-not-applied Patch12: boost-1.48.0-polygon.patch

That was a bug.

> boost.src: W: no-rhel-conditional boost.spec

Fixed, that check was indeed missing.

Comment 48 Matěj Cepl 2012-04-30 18:32:57 UTC
APPROVED. Please build a new release in Rawhide.

Comment 49 Benjamin Kosnik 2012-04-30 19:46:11 UTC
Nice job on this Petr.

Comment 50 Petr Machata 2012-05-01 19:04:40 UTC
(In reply to comment #48)
> APPROVED. Please build a new release in Rawhide.

Thanks.  Robert Scheck has built a new package recently.  This happens to contain all the relevant fixes.

(In reply to comment #49)
> Nice job on this Petr.

... and others ;)


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