From: Carl Worth <cworth@cworth.org>
To: Tomas Carnecky <tom@dbservice.com>, notmuch@notmuchmail.org
Subject: Re: [RFC] Add support for the Solaris platform
In-Reply-To: <1272654024-5254-1-git-send-email-tom@dbservice.com>
References: <1272654024-5254-1-git-send-email-tom@dbservice.com>
User-Agent: Notmuch/0.3.1-22-g1d528f8 (http://notmuchmail.org) Emacs/23.1.1
	(i486-pc-linux-gnu)
Date: Thu, 03 Jun 2010 17:22:47 -0700
Message-ID: <87ocfreje0.fsf@yoom.home.cworth.org>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha1; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Fri, 30 Apr 2010 21:00:24 +0200, Tomas Carnecky <tom@dbservice.com> wrot=
e:
> Like on Mac OS X, the linker doesn't automatically resolve
> dependencies.

Thanks for this fix, Tomas!

I like the fix described above, but you seem to have sent a patch with
three independent changes, only one of which was described. See below
for what I've done...

>  notmuch: $(notmuch_client_modules) lib/libnotmuch.a
> -	$(call quiet,CXX $(CFLAGS)) $^ $(FINAL_LIBNOTMUCH_LDFLAGS) -o $@
> +	$(call quiet,CXX $(CFLAGS)) $^ $(FINAL_NOTMUCH_LDFLAGS) -o $@

That seems to be correct. Could you describe what problem you ran into
that motivated you to find this fix? I have not committed this piece.

> -printf "Checking for Mac OS X (for shared library)... "
> +printf "Checking which platform we are on... "
>  if [ `uname` =3D "Darwin" ] ; then
> -    printf "Yes.\n"
> -    mac_os_x=3D1
> +    printf "Mac OS X.\n"
> +    platform=3DMACOSX
...
> -ifeq ($(MAC_OS_X),1)
> +ifeq ($(PLATFORM),MACOSX)
>  LIBRARY_SUFFIX =3D dylib
>  # On OS X, library version numbers go before suffix.
>  LINKER_NAME =3D libnotmuch.$(LIBRARY_SUFFIX)

That hunk was all described by your original commit message. So I've
applied this part with your original commit message and will be pushing
it soon.

> +/* Helper functions to test if a given dirent is of a certain type
> + */
> +static int
> +_is_reg(const char *path, struct dirent *entry)
...
> @@ -372,7 +426,7 @@ add_files_recursive (notmuch_database_t *notmuch,
>=20=20
>  	    if (! S_ISREG (st.st_mode))
>  		continue;
> -	} else if (entry->d_type !=3D DT_REG) {
> +	} else if (!_is_reg(path, entry)) {
>  	    continue;
>  	}

This all might be well and good, but is also not described by the commit
message. So I have not committed this either. If you resend with an
actual description of the change, I'll be happy to take a closer look.

Thanks again,

=2DCarl

=2D-=20
carl.d.worth@intel.com

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iD8DBQFMCEdX6JDdNq8qSWgRAnFmAJ9c/8QJJhK0TCg28QXFycXfO/3s1gCfcSn1
qESwlVSHeNEXHAgO1yTKAlk=
=/Co/
-----END PGP SIGNATURE-----
--=-=-=--