[parted-devel] Parted on GNU Hurd based systems

[Debarshi 'Rishi' Ray]

>> +dnl Checks for #defines.
>> +have_s390=no
>> +AC_CHECK_FUNC([__s390__], have_s390=yes, )
>> +AC_CHECK_FUNC([__s390x__], have_s390=yes, )

> __s390__ isn't really a *function*, is it?
> Everywhere else, it's used like a cpp symbol.
> If so, you need a different test (and different variable name):

I think it is something like __linux__, __gnu__ which is pre-defined
by the compiler. I tried AC_CHECK_DEFINE
(http://autoconf-archive.cryp.to/ax_check_define.html) but it seems to
have become obsolete. I will use AC_EGREP_CPP now.

> a *proper* autoconf-style test would not check for __s390__,
> but rather for the existence of "struct fdasd_anchor" in whatever
> header defines it.  Then the #if above would be
>
>   #if HAVE_STRUCT_FDASD_ANCHOR

But 'struct fdasd_anchor' is a struct defined in include/parted/fdasd.h.

>
> > +AM_CONDITIONAL([HAVE_S390], [test "$have_s390" = yes])
> > +
> >  dnl check for "check", unit testing library/header
> >  PKG_CHECK_MODULES([CHECK], [check >= 0.9.3], have_check=yes, have_check=no)
> >  if test "$have_check" != "yes"; then
> > diff --git a/include/parted/Makefile.am b/include/parted/Makefile.am
> > index 13df6c0..79e08cb 100644
> > --- a/include/parted/Makefile.am
> > +++ b/include/parted/Makefile.am
> > @@ -1,4 +1,13 @@
> > +if HAVE_S390
> > +S390_HEADERS = fdasd.h vtoc.h
> > +else
> > +S390_HEADERS =
> > +endif

> Unless you have a good reason to exclude headers here, please don't.
> Listing them here should be ok, even when not used.

My first patch did not remove any. I thought David preferred to have
them removed for consistency. Maybe I misunderstood. David?

> >  partedincludedir      =      $(includedir)/parted
> > +# Dummy-- to prevent error: "S390dir not defined".
> > +S390dir               = $(partedincludedir)

> Why is the above necessary?
> I don't see any other use of that symbol.

Automake throws a message during bootstrapping:
include/parted/Makefile.am:4: `S390_HEADERS' is used but `S390dir' is undefined
include/parted/Makefile.am:2: `S390_HEADERS' is used but `S390dir' is undefined
Is that alright?

> >  partedinclude_HEADERS = gnu.h                \
> >                       linux.h         \
> >                       constraint.h    \
> > @@ -11,9 +20,8 @@ partedinclude_HEADERS = gnu.h               \
> >                       natmath.h       \
> >                       timer.h         \
> >                       unit.h          \
> > -                     parted.h    \
> > -                     vtoc.h          \
> > -                     fdasd.h
> > +                     parted.h        \
> > +                        $(S390_HEADERS)

> Inconsistent indentation.
> In Makefiles, always indent with TAB.

Even when we are preferring space over tabs in the source files?

Happy hacking,
Debarshi
-- 
GPG key ID: 63D4A5A7
Key server: pgp.mit.edu