Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
Summary: Review Request: ustr - String library, very low memory overhead, simple to import Product: Fedora Version: devel Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: james.antill@redhat.com QAContact: extras-qa@fedoraproject.org CC: fedora-package-review@redhat.com,notting@redhat.com
Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.0-2.fc7.src.rpm Description: Hi, I already have a couple of packages, so hopefully this shouldn't be too bad :). rpmlint gives a couple of warnings, but they should be ignored AIUI.
Micro string library, very low overhead from plain strdup() (Ave. 44% for 0-20B strings). Very easy to use in existing C code. At it's simplest you can just include a single header file into your .c and start using it. This package also distributes pre-built shared libraries.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-07-25 14:35 EST ------- Created an attachment (id=159970) --> (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=159970&action=vie...) mock build log of ustr 1.0.0-2
Well, actually a lot of rpmlint complaints...
----------------------------------------------------- W: ustr incoherent-version-in-changelog 1.0.0-1 1.0.0-2.fc8 W: ustr invalid-license MIT, LGPL, BSD W: ustr unstripped-binary-or-object /usr/lib/libustr-1.0.so.1.0.0 W: ustr invalid-license MIT, LGPL, BSD W: ustr strange-permission ustr.spec 0600 W: ustr-debug no-documentation E: ustr-debug library-without-ldconfig-postin /usr/lib/libustr-debug-1.0.so.1.0.0 E: ustr-debug library-without-ldconfig-postun /usr/lib/libustr-debug-1.0.so.1.0.0 W: ustr-debug no-dependency-on ustr W: ustr-debug summary-ended-with-dot String library, very very low memory overhead, simple to import. W: ustr-debug invalid-license MIT, LGPL, BSD W: ustr-debug unstripped-binary-or-object /usr/lib/libustr-debug-1.0.so.1.0.0 W: ustr-debug-static no-documentation W: ustr-debug-static summary-ended-with-dot String library, very very low memory overhead, simple to import. W: ustr-debug-static invalid-license MIT, LGPL, BSD E: ustr-debuginfo empty-debuginfo-package W: ustr-debuginfo invalid-license MIT, LGPL, BSD W: ustr-devel hidden-file-or-dir /usr/share/ustr-1.0.0/.gdbinit W: ustr-devel summary-ended-with-dot String library, very very low memory overhead, simple to import. W: ustr-devel invalid-license MIT, LGPL, BSD W: ustr-static no-documentation W: ustr-static summary-ended-with-dot String library, very very low memory overhead, simple to import. W: ustr-static invalid-license MIT, LGPL, BSD ------------------------------------------------------ Summary * version mismatch between the last entry of %changelog and rpm EVR. * Perhaps MIT/LGPL/BSD will remove rpmlint complaint. * Please change the permission of spec/tarball to 0644 * Calling ldconfig is needed for library * "unstripped-binary-or-object" is usually due to wrong permission (i.e. this binary should have executable permission, usually 0755) * Please remove dot from the end of summary * debuginfo rpm is empty. This means that debug option "-g" flag is not used on compile. I checked the build log (attached) but no useful information is gained from the build log. - Please make the build log more verbose - and Fedora specific compilation flags are perhaps not honored. * What is the file /usr/share/ustr-1.0.0/.gdbinit for?
Other quick comment: * Source must be written with full URL. * Please check directories' ownership. * Usually the dependency for main or subpackage should be version-release number specific * summary seems all the same for all packages?? * Please use %_includedir * Files under %_datadir/doc or %_mandir are automatically tagged as %doc
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-25 14:56 EST ------- * version mismatch between the last entry of %changelog and rpm EVR.
Fair enough.
* Perhaps MIT/LGPL/BSD will remove rpmlint complaint.
I'll try that.
* Please change the permission of spec/tarball to 0644
This will happen automatically when it goes into Fedora CVS, no? Doing this outside of Fedora CVS means playing with my umask settings.
* Calling ldconfig is needed for library
AIUI this isn't required. You either need to call ldconfig, or explicitly generate the symlinks that ldconfig would do ... but not both. And the former is preferred. If you could point to something official that says otherwise, I'll gladly change it.
* "unstripped-binary-or-object" is usually due to wrong permission (i.e. this binary should have executable permission, usually 0755)
Fixed.
* Please remove dot from the end of summary
Yeh, I thought I'd done that but I only did the main package.
* debuginfo rpm is empty. This means that debug option "-g" flag is not used on compile. I checked the build log (attached) but no useful information is gained from the build log. - Please make the build log more verbose - and Fedora specific compilation flags are perhaps not honored.
I'll look into this, it doesn't use autoconf ... so I might well need to pass something for CFLAGS.
* What is the file /usr/share/ustr-1.0.0/.gdbinit for?
It can be copied to a users (developer using the library) home dir, and is copied via. the ustr-import to the working directory. I can change the library to call it gdbinit.txt, if I really need to ... it just seems like a false warning (and as with all lints there's no good way to turn it off).
Other quick comment: * Source must be written with full URL.
Fixed.
* Please check directories' ownership.
Which directories?
* Usually the dependency for main or subpackage should be version-release number specific
As far as I can see they all do. Which one isn't?
* summary seems all the same for all packages??
This is common for libraries, no? For instance glib2 and glib2-devel have the same summary but different descriptions ... what else should I be doing?
* Please use %_includedir
Fixed.
* Files under %_datadir/doc or %_mandir are automatically tagged as %doc
Ok, removed the doc tag.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-25 15:05 EST ------- * Calling ldconfig is needed for library
Nevermind, I've fixed this. I was thinking of something else.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-26 01:56 EST ------- New versions of both the srpm and spec file at:
Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.1-0.fc7.src.rpm
...rpmlint on all the generated binaries gives:
W: ustr strange-permission ustr.spec 0600 W: ustr-debug no-documentation W: ustr-debug no-dependency-on ustr W: ustr-debug-static no-documentation W: ustr-devel hidden-file-or-dir /usr/share/ustr-1.0.1/.gdbinit W: ustr-static no-documentation
...the "no docs" are false positives, as is the .gdbinit file. The no-dep-on ustr is just wrong, and the ustr.spec snafu will be fixed by getting it in CVS.
Anything else?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-07-26 11:18 EST ------- Created an attachment (id=160029) --> (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=160029&action=vie...) mock build log of ustr 1.0.1-0 on rawhide i386
* I just tried to rebuild, but it failed (please check the bug attached) * And please make the build log more verbose. The log like ------------------------------------------------- Compiling for A DBG lib: ustr-b-dbg-code.c ------------------------------------------------- is not useful. We cannot check if compilation flags are correct from this log.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-26 21:42 EST ------- * I just tried to rebuild, but it failed (please check the bug attached)
Ahh, a bug! That'll teach me to not check i386 too.
* And please make the build log more verbose.
This is a really bizarre request, but OK. I've put a mildly horrible hack in so it'll spam all the details to the build.log. New version at the same URLs as last time:
Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.1-0.fc7.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-07-26 23:32 EST -------
From next time please bump release number every time you modify
your spec/srpm.
http://fedoraproject.org/wiki/Packaging/FrequentlyMadeMistakes -------------------------------------------------------------- ncrease the "Release" tag every time you upload a new package to avoid confusion. The reviewer and other interested parties probably still have older versions of your SRPM lying around to check what has changed between the old and new packages; those get confused when the revision didn't change. --------------------------------------------------------------
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-07-27 12:41 EST ------- Created an attachment (id=160126) --> (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=160126&action=vie...) mock build log of ustr 1.0.1-0 on rawhide i386 (2nd time)
(Assuming that you are the upstream of this package and you have not yet released 1.0.1 formally)
* For /sbin/ldconfig, usually we don't write Requires(post) and so on.
* rpm (sub)packages which contains pkgconfig .pc files should have "Requires: pkgconfig"
* mock build log says that fedora specific compilation flags are not honored.
* The following directories are not owned by any packages. ------------------------------------------------------ %{_datadir}/ustr-%{version}/ %{_datadir}/doc/ustr-devel-%{version}/ ------------------------------------------------------ * Usually the dependency for other subpackages must be version-release specific. i.e. -devel package should have Requires: %{name} = %{version}-%{release}, for example
* Use %_includedir for /usr/include.
* I have not installed ustr yet, however would you check the dependencies between subpackages? For example, why does -debug subpackage require -static subpackage? (well, this is a question)
* For summary (In reply to comment #2)
- summary seems all the same for all packages??
This is common for libraries, no? For instance glib2 and glib2-devel have the same summary but different descriptions ...
Strange... Anyway as you can try "rpmdev-newspec libfoo" to create skeleton spec file, usually summary and description for -devel subpackage are like: -------------------------------------------------------- %package devel Summary: Development files for %{name} Group: Development/Libraries Requires: %{name} = %{version}-%{release}
%description devel The %{name}-devel package contains libraries and header files for developing applications that use %{name}. ----------------------------------------------------------
* And please increase release number (Perhaps you want to set release number as 1 when review is done
and wants to set release number 0 during review, but please don't. At least please increase release number as 0.1, 0.2, ...)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-27 13:53 EST ------- (Assuming that you are the upstream of this package and you have not yet released 1.0.1 formally)
Yes.
* For /sbin/ldconfig, usually we don't write Requires(post) and so on.
Why? What is best practice, no deps. or just a normal requires?
* rpm (sub)packages which contains pkgconfig .pc files should have "Requires: pkgconfig"
This is true even if it's not required. From the upstream POV it isn't required, it can be used if you find pkg-config easier to use ... or you can just do -lustr etc. Obviously I can add it to the rpm anyway, if you want though.
* mock build log says that fedora specific compilation flags are not honored.
Ok, I thought:
CFLAGS="${CFLAGS:-%optflags}" ; export CFLAGS ;
...was enough, as that's what the %configure macro seems to be using. I can't find any documentation on what I should be calling here (there is no ./configure).
* The following directories are not owned by any packages. ------------------------------------------------------ %{_datadir}/ustr-%{version}/ %{_datadir}/doc/ustr-devel-%{version}/ ------------------------------------------------------
My bad, I assumed dir/* got dir too.
* Usually the dependency for other subpackages must be version-release specific. i.e. -devel package should have Requires: %{name} = %{version}-%{release}, for example
Fixed.
* Use %_includedir for /usr/include.
In the %files section? Fixed.
* I have not installed ustr yet, however would you check the dependencies between subpackages? For example, why does -debug subpackage require -static subpackage? (well, this is a question)
I can't remember :(. I've changed debug to just depend on devel, and debug-static to depend on debug.
* For summary (In reply to comment #2)
- summary seems all the same for all packages??
This is common for libraries, no? For instance glib2 and glib2-devel have the same summary but different descriptions ...
Strange... Anyway as you can try "rpmdev-newspec libfoo" to create skeleton spec file, usually summary and description for -devel subpackage are like:
Summary: Development files for %{name}
Ok, I guess the other stuff just hasn't been fixed yet. Fixed.
* And please increase release number (Perhaps you want to set release number as 1 when review is done
and wants to set release number 0 during review, but please don't. At least please increase release number as 0.1, 0.2, ...)
Yeh, I didn't want rel=1 until it's 1.0.1 is released upstream, I'll upload a 0.2 version as soon as I can find out what to do about:
requires for pkg-config requires for ldconfig CFLAGS
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-07-27 14:31 EST ------- (In reply to comment #9)
- For /sbin/ldconfig, usually we don't write Requires(post) and so on.
Why? What is best practice, no deps. or just a normal requires?
- The most unkind answer is that "it is as written on packaging guideline". http://fedoraproject.org/wiki/Packaging/ScriptletSnippets I guess /sbin/ldconfig is so common?
Note: while I don't know if rpm adds the dependency for Requires(post) or etc, at least if %post -p <program> is written, rpm seems to add automatically the <program> to Requires.
- rpm (sub)packages which contains pkgconfig .pc files should have "Requires: pkgconfig"
Obviously I can add it to the rpm anyway, if you want though.
- Well, though I wrote "should", this is a "MUST" item of the review http://fedoraproject.org/wiki/Packaging/ReviewGuidelines (As written, the reason is directory ownership and usability for .pc file)
- mock build log says that fedora specific compilation flags are not honored.
Ok, I thought: CFLAGS="${CFLAGS:-%optflags}" ; export CFLAGS ; ...was enough, as that's what the %configure macro seems to be using. I can't find any documentation on what I should be calling here (there is no ./configure).
%optflags is autually the flags we want to use. It is just CFLAGS you set is not honored by this way for this package (i.e. If normal way is not used to honor %optflags, you have to make %optflags honored *somehow*. Perhaps you have to investigate Makefile how compilation flags are used.)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-27 19:29 EST ------- Hopefully this fixes all of the above:
Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.1-0.2.fc7.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |mtasaka@ioa.s.u-tokyo.ac.jp Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-07-28 02:10 EST ------- Well, I just tried to rebuild -0.2 on koji target dist-f8:
* Still fedora specific compilation flags are not honored (partially). http://koji.fedoraproject.org/koji/getfile?taskID=80296&name=build.log * Rebuild seems to fail on ppc http://koji.fedoraproject.org/koji/taskinfo?taskID=80293
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-28 17:22 EST -------
* Still fedora specific compilation flags are not honored (partially). http://koji.fedoraproject.org/koji/getfile?taskID=80296&name=build.log
On what? The unit tests? Or the fact that the -debug version of the library turns some of the optimizations down? I don't see anything else, but maybe I'm just missing it.
* Rebuild seems to fail on ppc http://koji.fedoraproject.org/koji/taskinfo?taskID=80293
Damn, I'll see if I can reproduce this on a fedora-devel-i386 mock build.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-28 20:56 EST ------- It doesn't but I can reproduce the failure with:
koji build --scratch --arch-override=ppc dist-f8 /home/james/work/build/ustr/ustr-1.0.1-0.2.fc7.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-28 23:07 EST -------
Well I've "fixed" the ppc problem.
http://koji.fedoraproject.org/koji/taskinfo?taskID=80882
Although the fix implies I need to fix a bunch of other points. Anyway, if you can let me know about the build flags I'll do an another update.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-07-29 12:37 EST ------- Well, now ustr is rebuilt on all archs as: http://koji.fedoraproject.org/koji/taskinfo?taskID=80889
Then for -0.6:
- Still fedora specific compilation flags are not honored (partially).
On what? The unit tests?
- Oh, yes, for tests. Perhaps this can be ignored.
* File entry - I just noticed that: ------------------------------------------------- $ rpm -qlp ustr-*rpm | sort | uniq -d /usr/include/ustr-conf-debug.h /usr/include/ustr-debug.h ------------------------------------------------- ? Perhaps it is better that all header files are hidden under /usr/include/ustr.
* Compilation flags ? On compiling debugging source code: ------------------------------------------------- Compiling for A DBG lib: ustr-io-dbg-code.c cc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -W -Wall -Wundef -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wwrite-strings -Waggregate-return -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wnested-externs -Wno-format-zero-length -Wformat-nonliteral -Wformat-security -O1 -ggdb3 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -o ustr-cmp-code-a-dbg.o -c ustr-cmp-dbg-code.c ------------------------------------------------- the fedora compilation flags (-O2) is overwritten by -O1. Is this intentional?
* File name ? Well, it is okay, however would you rename the file which contains white space in its name?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-30 03:19 EST ------- ? Perhaps it is better that all header files are hidden under /usr/include/ustr.
...if I did that the users would have to alter their -I build flags to use the headers, saying that I think I'll be forced to move at least ustr-conf.h out to /usr/lib/ and /ustr/lib64/ if multilib. development is to be supported. I'll have to think about this more.
? the fedora compilation flags (-O2) is overwritten by -O1. Is this intentional?
Yes, GCC/GDB gets confused at -O2 so for a "debug mode" you want to reduce that confusion as much as possible.
? Well, it is okay, however would you rename the file which contains white space in its name?
This is part of upstream, is it policy to rename upstream files? I guess I can change the spaces to dashes at install time, but it means html documentation pointing to the file won't work unless I run that through sed etc.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From rc040203@freenet.de 2007-07-30 04:01 EST ------- (In reply to comment #17)
? the fedora compilation flags (-O2) is overwritten by -O1. Is this intentional?
Yes, GCC/GDB gets confused at -O2 so for a "debug mode" you want to reduce that confusion as much as possible.
Either this package is obeying RPM_OPT_FLAGS, or ... if it can't, it has to be considered broken or you are triggering a bug in GCC.
In any case, this is a MUSTFIX.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-30 13:52 EST -------
Ralf, I'm not sure if you know the context for the above. The Fedora flags are, roughly:
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
...and you appear to be saying that adding other options is fine, as long as you don't change the behaviour of any of the above? (although I'd be shocked if nothing did -Wno-* which turned off a -Wall option). I can sort of understand it for all of them apart from -O1, but what is the rationale for requiring optimizations at that specific level? This is esp. true given that we are talking about _sub-package_ specifically for development, with the main package using the exact CFLAGS you want. You are requiring making the debugging worse in Fedora, for no good reason that I can see. If you are going to hold to that line, due to it being policy, I'd like to know how I can go about getting this policy fixed.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From rc040203@freenet.de 2007-07-31 00:03 EST ------- (In reply to comment #19)
Ralf, I'm not sure if you know the context for the above. The Fedora flags are, roughly:
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
...and you appear to be saying that adding other options is fine, as long as you don't change the behaviour of any of the above?
More or less, yes, that's what I am saying.
The core behind this is: A package's CFLAGS must not use a different ABI.
(although I'd be shocked if nothing did -Wno-* which turned off a -Wall option).
-W* options are warnings. They don't change the ABI.
I can sort of understand it for all of them apart from -O1, but what is the rationale for requiring optimizations at that specific level?
-O* flags imply many other flags and have many side-effects (such as inlining, certain optimizations, certain arch-dependent optimizations etc.), which implicitly gradually change over time.
I.e. what you currently think about -O1 might be true on your current setup, but is not unlikely not apply in 2 years and on other archs. Therefore, using a consistent set of optimizations (RPM_OPT_FLAGS) and not to play tricks with them is important for a distro's quality.
That said, unless there is an inevitable technical requirement (which I don't see in this case), things like -ggdb3 are just silly. Requiring -O1 in almost all cases means a major bug inside of a package and questions its usability.
You are requiring making the debugging worse in Fedora, for no good reason that I can see. If you are going to hold to that line, due to it being policy, I'd like to know how I can go about getting this policy fixed.
In this case, I am going to be hard - Fix the package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-31 02:02 EST -------
The core behind this is: A package's CFLAGS must not use a different ABI.
Your definition of ABI is vastly different to mine, apparently.
-O* flags imply many other flags and have many side-effects (such as inlining, certain optimizations, certain arch-dependent optimizations etc.), which implicitly gradually change over time.
Indeed they do, none of those change the Application Binary Interface though. And, as I would normally presume you'd know, they do affect things like hampering debugging of code (which, being the -debug package might be kind of the point).
You are requiring making the debugging worse in Fedora, for no good reason that I can see. If you are going to hold to that line, due to it being policy, I'd like to know how I can go about getting this policy fixed.
In this case, I am going to be hard - Fix the package.
As I said I can make the package worse so it passes review (I'll upload a 0.7 package tomorrow), but again I'd like to know how I get this problem with policy fixed ... so at some point in the future I can unbreak the package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From rc040203@freenet.de 2007-07-31 02:31 EST ------- (In reply to comment #21)
-O* flags imply many other flags and have many side-effects (such as inlining, certain optimizations, certain arch-dependent optimizations etc.), which implicitly gradually change over time.
Indeed they do, none of those change the Application Binary Interface though.
* -ggdb3 does. If GCC doesn't support it, your package won't build nor will it be usable.
And, as I would normally presume you'd know, they do affect things like hampering debugging of code (which, being the -debug package might be kind of the point).
Yes, -O2 always reduces debug-ability of the code, so what is the problem you are trying to solve?
You are requiring making the debugging worse in Fedora, for no good
reason that
I can see. If you are going to hold to that line, due to it being policy, I'd like to know how I can go about getting this policy fixed.
In this case, I am going to be hard - Fix the package.
As I said I can make the package worse so it passes review (I'll upload a 0.7 package tomorrow), but again I'd like to know how I get this problem with policy fixed ... so at some point in the future I can unbreak the package.
I guess you will understand, that I consider your answer to be non-acceptable and inappropriate.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-07-31 18:13 EST -------
- -ggdb3 does. If GCC doesn't support it, your package won't build nor will it
be usable
You are saying -ggdb3 Changes the ABI? I don't think so. Yes, that is somewhat of a GCC specific option. As are all the -W flags. If that's the only objection, I'm not that bothered about droping that.
Yes, -O2 always reduces debug-ability of the code, so what is the problem you are trying to solve?
Ok, so explaining again:
. The srpm currently produces 6 packages, the most relevant to this are the two "main" ones:
ustr-1.0.1-0.7.fc7.x86_64.rpm ustr-debug-1.0.1-0.7.fc7.x86_64.rpm
...the first is the library built for "production", and is what all the people using a program that uses the library will see. The second is _the same code_ built using as much debugging as possible, so (expensive) internal consistency checks are called often; optimizations are turned down; etc. The idea being that anyone developing code using the library can use the debugging version while doing development and it will actively alert them to errors. Forcing build flags that make debugging harder when using this package is not doing to help anyone.
I guess you will understand, that I consider your answer to be non-acceptable and inappropriate.
Actually I didn't, and still don't, understand that ... I said I would do what you wanted to follow policy, even though it was wrong, and I repeatedly asked how I could fix policy. Is one not allowed to point out that policy is wrong?
I can't imagine what I said that was inappropriate.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-08-02 01:24 EST ------- Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.1-0.8.fc7.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-02 10:51 EST ------- (In reply to comment #16)
Then for -0.6:
- File entry
- I just noticed that:
$ rpm -qlp ustr-*rpm | sort | uniq -d /usr/include/ustr-conf-debug.h /usr/include/ustr-debug.h
- This is not yet fixed on -0.8 . (i.e. both -devel and -debug package contain the files above.)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-08-03 03:15 EST ------- Ok, hopefully for the final time...
Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.1-0.9.fc7.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-03 08:23 EST ------- Ah.. http://koji.fedoraproject.org/koji/taskinfo?taskID=87418
Note: You can use %exclude . i.e. for example: ----------------------------------------------- %files devel .............. %{_includedir}/ustr.h %{_includedir}/ustr-*.h %exclude %{_includedir}/ustr*debug.h .............. %files debug .............. %{_includedir}/ustr*debug.h .............. ---------------------------------------------------- Perhaps this should be okay
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-03 08:33 EST ------- More one point: - As we can see on the -devel list, the license tag guideline is changed. For this package, please use: --------------------------------------------- MIT or LGPLv2 or BSD --------------------------------------------- (the seperator seems changed to "or", and for LGPL now we have to specify version, i.e. LGPLv2 or LGPLv2+)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-08-03 12:49 EST ------- Ok, sorry, I thought my mock builds were checking for installed but not covered files. Thanks for the except tip, I didn't kjnow about that. Also fixed the license:
Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.1-0.10.fc7.src.rpm
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-03 14:34 EST ------- One question (for clarification)
You seemed to choose LGPLv2+ (not LGPLv2), however where can I found that this can be licensed as LGPL "and later"?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-08-03 15:25 EST ------- Well the LICENSE file refers to LICENSE_LGPL which says:
If the Library does not specify a license version number, you may choose any version ever published by the Free Software Foundation.
...and version 2 was the oldest version of LGPL. If that's not enough does an upstream change of:
diff --git a/LICENSE b/LICENSE index 5d03476..c1faf88 100644 --- a/LICENSE +++ b/LICENSE @@ -1,6 +1,6 @@ This code is multi Licensed under all/any one of:
-LGPL - http://www.and.org/ustr/LICENSE_LGPL +LGPLv2+ - http://www.and.org/ustr/LICENSE_LGPL New Style BSD (2 clause) - http://www.and.org/ustr/LICENSE_BSD MIT - http://www.and.org/ustr/LICENSE_MIT
...make this clearer?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-03 19:47 EST ------- Yes, I think it makes the license clearer.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From james.antill@redhat.com 2007-08-04 01:53 EST ------- Ok, official upstream release 1.0.1 vers. 1:
Spec URL: http://people.redhat.com/jantill/fedora/ustr.spec SRPM URL: http://people.redhat.com/jantill/fedora/ustr-1.0.1-1.fc7.src.rpm
...also has the LICENSE changes.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-04 03:38 EST ------- Please add the last entry "1.0.1-1" to %changelog before you commit to CVS.
------------------------------------------------------------- This package (ustr) is APPROVED by me -------------------------------------------------------------
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
james.antill@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
------- Additional Comments From james.antill@redhat.com 2007-08-04 11:51 EST ------- N/p, on the changelog thing (although I assume it's ok if I use 1.0.1-2, given that's probably the first version I'll upload?).
New Package CVS Request ======================= Package Name: ustr Short Description: String library, very very low memory overhead, simple to import Owners: james.antill@redhat.com Branches: FC-6 F-7 EL-4 EL-5 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-04 12:30 EST ------- (In reply to comment #35)
(although I assume it's ok if I use 1.0.1-2, given that's probably the first version I'll upload?).
No problem.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
------- Additional Comments From kevin@tummy.com 2007-08-04 14:13 EST ------- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-08-06 09:49 EST ------- Please close this bug as NEXTRELEASE when rebuild is done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Review Request: ustr - String library, very low memory overhead, simple to import
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=248231
james.antill@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
package-review@lists.fedoraproject.org