[Bug 1203749] Review Request: dssp - Protein secondary structure assignment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #13 from Antonio Trande <anto.trande@xxxxxxxxx> ---
(In reply to Dave Love from comment #12)
> I could have sworn I'd responded to this, but obviously didn't press save
> or something.  Anyhow:
> 
> I'm not sure who I should be responding to, and why half this stuff matters
> when it's not in any guidelines and differs in other packages.
> 

They are just advices; if you think they are useless (and it's not any official
guidelines), you can do what you prefer.

> > > 
> > > I don't understand why BOOST_{INC,LIB}_DIR, DEBUG, and INSTALL need setting.
> > 
> > Boost must be set in Make.config file; better if set manually in the SPEC file 
> > in case of changes in future.
> > Debugging is considered by Make but **explicitly** not required during building > process.
> 
> Boost clearly doesn't need to be set there; it builds fine and it seems
> wrong explicitly to set the default paths -- what's special about boost, and
> why don't other packages need to do that?

Boost paths point to default directories of potentially bundled files; as you
can see in your SPEC file, Boost lib_suffix is right for Boost-1.41.0 (named
'boost141' in EPEL5 and 'boost' in CentOS6 or Rhel6) but not for Boost-1.48.0
(named 'boost148' in EPEL6 and 'boost' in Fedora):

@echo "#BOOST_LIB_SUFFIX = -mt" >> make.config
@echo "#BOOST_LIB_DIR    = $(HOME)/projects/boost/lib" >> make.config
@echo "#BOOST_INC_DIR    = $(HOME)/projects/boost/include" >> make.config

Here is why (to me) you SHOULD set explicitely Boost paths in the rpm building. 

> Sorry, I just don't understand the comment about DEBUG.

Make provides a DEBUG variable that could be contemplated for debugging builds.

> 
> > %build
> > ...
> > echo "LDOPTS=%{__global_ldflags}" >>make.config
> > make %{?_smp_mflags}
> 
> __global_ldflags isn't defined in EPEL6, and it's not clear to me that the
> flags are generally a good idea for computational programs, but I added it
> conditionally.

Define a macro so:

%if 0%{?epel} < 7
%{!?__global_ldflags: %global __global_ldflags -Wl,-z,relro}
%endif

> 
> > Since the package contains just one binary, please use
> > 
> > %{_bindir}/mkdssp
> > 
> > instead of
> > 
> > %_bindir/*
> 
> Why?  I've not seen that in any guidelines, and it doesn't seem to achieve
> anything useful, but I did it.

Because SPEC file is more legible in my opinion; using an aggregation symbol
just for one binary name is not make sense.

> 
> It's wanted on our el5 system, and the spec now supports that.
> 
> New
> SRPM URL: https://loveshack.fedorapeople.org/review/dssp-2.2.1-5.el5.src.rpm
> Spec URL: https://loveshack.fedorapeople.org/review/dssp.spec

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]