Re: [RFC PATCH v3 1/1 Hurd]hurd: Add a libstore library without libparted

[Zhaoming Luo]

On Sat, Jan 11, 2025 at 11:39:46AM +0100, Samuel Thibault wrote:
> Hello,

Thanks for the review.

> 
> Zhaoming Luo, le sam. 11 janv. 2025 12:56:33 +0800, a ecrit:
> > diff --git a/libstore/Makefile b/libstore/Makefile
> > index c7af958b..d0f06450 100644
> > --- a/libstore/Makefile
> > +++ b/libstore/Makefile
> > @@ -20,10 +20,17 @@
> >  #   along with this program; if not, write to the Free Software
> >  #   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111, USA.
> >  
> > -dir := libstore
> > +# EXCLUDE_PARTED is defined in the Makefile of libstore-noparted
> 
> Negations in names often lead to hard-to-read double-negations. Better
> use USE_PARTED instead.

OK

> 
> > +ifeq ($(EXCLUDE_PARTED), 1)
> > +  dir := libstore-noparted
> > +  libname := libstore-noparted
> > +else
> > +  dir := libstore
> > +  libname := libstore
> > +endif
> 
> Why not using ?= in libstore to let it overridded by libstore-noparted?

OK. I supposed using `ifeq` can make it easier to understand. Now I know ?= and
a line of comment is better :).

> 
> > -libstore.so-LDLIBS += $(PARTED_LIBS) -ldl
> > +ifeq ($(EXCLUDE_PARTED), 1)
> > +  libstore-noparted.so-LDLIBS += -ldl
> > +else
> > +  libstore.so-LDLIBS += $(PARTED_LIBS) -ldl
> 
> Better use 
> 
> $(libname).so-LDLIBS += -ldl
> ifeq ($(USE_PARTED), 1)
>   $(libname).so-LDLIBS += $(PARTED_LIBS)
> endif

OK. Then `USE_PARTED ?= 1` will need to be added in the libstore Makefile.

Zhaoming