[RFH] NetBSD 6?

[RFH] NetBSD 6?

[Junio C Hamano]

I would appreciate if somebody with more familiarlity with the
platform can suggest a better alternative than applying the
following patch to our Makefile.  Right now I have an equivalent of
this change in config.mak locally when building on the said
platform.

The "2.7" bit certainly looks fishy, as users should be able to
choose between "2.6" and "2.7" (and possibly "3.0"), IIUC.

 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index b0df41b..2ca6047 100644
--- a/Makefile
+++ b/Makefile
@@ -1163,8 +1163,11 @@ ifeq ($(uname_S),NetBSD)
 	ifeq ($(shell expr "$(uname_R)" : '[01]\.'),2)
 		NEEDS_LIBICONV = YesPlease
 	endif
+	PYTHON_PATH = /usr/pkg/bin/python2.7
+	PERL_PATH = /usr/pkg/bin/perl
 	BASIC_CFLAGS += -I/usr/pkg/include
 	BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib
+	OLD_ICONV = YesPlease
 	USE_ST_TIMESPEC = YesPlease
 	NO_MKSTEMPS = YesPlease
 	HAVE_PATHS_H = YesPlease

Re: [RFH] NetBSD 6?

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]


Junio C Hamano <gitster@pobox.com> writes:

> [query about NetBSD-6] 

> The "2.7" bit certainly looks fishy, as users should be able to
> choose between "2.6" and "2.7" (and possibly "3.0"), IIUC.
>
> +	PYTHON_PATH = /usr/pkg/bin/python2.7
> +	PERL_PATH = /usr/pkg/bin/perl

(I am one of the people who maintain the git package in pkgsrc.)

(Strictly, this is not really about NetBSD, but about all systems where
the standard approach to get python is via pkgsrc.  So that include
DragonflyBSD as well.  (pkgsrc runs on many other systems, but it isn't
the standard approach, so from the git viewpoint that's irrelevant.))

You are entirely right that on e.g. NetBSD 6 the view is that users
should be able to choose the python version.

pkgsrc can install multiple versions of python at the same time, to cope
with python-using packages that need different versions.  pkgsrc chooses
not to have a 'python' program, because that would result in installed
packages changing their binding of which python version to use when the
default was changed.  The default python version is currently 2.7, so
/usr/pkg/bin/python2.7 is the best guess for finding python on a NetBSD
system, if you're only allowed one guess.  A user can set a
PYTHON_VERSION_DEFAULT variable to choose the version they want; each
package expresses which versions will work.

This isn't relevant for git, not being a pure python library, but pkgsrc
supports installing multiple versions of some packages, so one can have
two versions installed at once:
  py27-expat-0nb6     Python interface to expat
  py26-expat-0nb6     Python interface to expat
The git package just depends on one version; by default the git package
depends on python (but one can tell it not to).

The python.m4 macro that comes with automake seems to find one of the
various pythonX.Y binaries in $PATH just fine.

pkgsrc has an entry for git (at 1.8.0.1).
The key line for handling python is:
  MAKE_FLAGS+=	PYTHON_PATH=${PYTHONBIN}
and there PYTHONBIN is set up by pkgsrc infrastructure for the right
prefix (99.9% but not always /usr/pkg) and version.  After this,
everything seems to come out right:

  > head -1 /usr/pkg/libexec/git-core/git-p4
  #!/usr/pkg/bin/python2.7

So I'd say that if PYTHON_PATH is set in the environment to configure,
it should behave as it does now.  And if not, it would be nice if the
highest pythonX.Y found (that is known to work with git) is used.

> +	PYTHON_PATH = /usr/pkg/bin/python2.7
> +	PERL_PATH = /usr/pkg/bin/perl

So it would be nice to make these work as ?=, letting an environment
variable win if set.

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFH] NetBSD 6?

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]


Junio C Hamano <gitster@pobox.com> writes:

> I would appreciate if somebody with more familiarlity with the
> platform can suggest a better alternative than applying the
> following patch to our Makefile.  Right now I have an equivalent of
> this change in config.mak locally when building on the said
> platform.

I realized after sending my previous reply that you are probably trying
to have a way to build and run tests on NetBSD-6 from a git checkout as
part of development testing.

One approach I've taken with other programs is to have a README.NetBSD
file which is actually an executable /bin/sh script with comments,
explaining the prereqs in terms of pkgsrc and invoking configure to get
dependencies from pkgsrc (-I/usr/pkg/include plus -L/-R).

So in the git case, this could set PYTHON_PATH in the environment.

I realize a README.foo file for N different systems could be clutter,
but having these checked in would provide the concise help that people
on any of those platforms need.

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFH] NetBSD 6?

[Junio C Hamano]

Greg Troxel <gdt@ir.bbn.com> writes:

> I realize a README.foo file for N different systems could be clutter,
> but having these checked in would provide the concise help that people
> on any of those platforms need.

Our Makefile documents knobs people on various platforms can tweak
(PYTHON_PATH and OLD_ICONV are two examples of them), sets default
values for them based on $(uname -S), then includes config.mak file
the user can optionally create to override these platform defaults.
This infrastructure is used across platforms, not just for NetBSD.

The part shown in the patch was to update the platform default for
NetBSD.  The setting we have been shipping in our Makefile seemed to
be different from what I needed on my NetBSD 6 install, and I was
wondering if we have no real users of Git on the platorm (which
would explain why we didn't get any complaints or patches to update
this part).  Or there are some optional packages all real NetBSD
users install, but being a NetBSD newbie I didn't, that makes the
values I showed in the patch inappropriate for them (e.g. Perhaps
there is a mechanism other than pkgsrc that installs perl and python
under /usr/bin?  Perhaps an optional libi18n package that gives
iconv(3) with new function signature?), in which case I definitely
should not apply that patch to my tree, as it would only be an
improvement for one person and a regression for existing users at
the same time.

Re: [RFH] NetBSD 6?

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 2791 bytes --]


Junio C Hamano <gitster@pobox.com> writes:

> Greg Troxel <gdt@ir.bbn.com> writes:
>
>> I realize a README.foo file for N different systems could be clutter,
>> but having these checked in would provide the concise help that people
>> on any of those platforms need.
>
> Our Makefile documents knobs people on various platforms can tweak
> (PYTHON_PATH and OLD_ICONV are two examples of them), sets default
> values for them based on $(uname -S), then includes config.mak file
> the user can optionally create to override these platform defaults.
> This infrastructure is used across platforms, not just for NetBSD.

If you have to choose a single PYTHON_PATH, the one you picked is right
(for now, and likely will be right for a long time).

But as a general rule, I think configure tests are preferable to
OS-specific variables.

> The part shown in the patch was to update the platform default for
> NetBSD.  The setting we have been shipping in our Makefile seemed to
> be different from what I needed on my NetBSD 6 install, and I was
> wondering if we have no real users of Git on the platorm (which
> would explain why we didn't get any complaints or patches to update
> this part).  Or there are some optional packages all real NetBSD
> users install, but being a NetBSD newbie I didn't, that makes the
> values I showed in the patch inappropriate for them (e.g. Perhaps
> there is a mechanism other than pkgsrc that installs perl and python
> under /usr/bin?  Perhaps an optional libi18n package that gives
> iconv(3) with new function signature?), in which case I definitely
> should not apply that patch to my tree, as it would only be an
> improvement for one person and a regression for existing users at
> the same time.

A large number of people on NetBSD use git, but almost all of them get
it via pkgsrc (which is also where they get perl, emacs, svn, and
everything else you didn't find in /usr/bin).  The exception would be
people that want to hack on git itself.

People who want gnu libiconv can install the libiconv package from
pkgsrc.  (I'm guessing OLD_ICONV means "POSIX iconv", without GNU
extensions (iconvctl?).)  The git package doesn't depend on GNU iconv,
though (perhaps it should but we tend to avoid dependencies that arren't
really needed).

There are no mechanisms to install python/perl in /usr/bin, and doing so
would be viewed as bizarre.  /usr/bin belongs to the base system,
/usr/pkg to pkgsrc and /usr/local to the user.

So, it doesn't matter too much for pkgsrc what you change, because it
can be patched anyway (once, for all users).  It's probably better to
make a straightforward build from source come out right.
But, if the build respects the PYTHON_PATH environment variable, that's
easier (and more robust against changes) than patching.

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFH] NetBSD 6?

[Junio C Hamano]

Greg Troxel <gdt@ir.bbn.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Greg Troxel <gdt@ir.bbn.com> writes:
>>
>>> I realize a README.foo file for N different systems could be clutter,
>>> but having these checked in would provide the concise help that people
>>> on any of those platforms need.
>>
>> Our Makefile documents knobs people on various platforms can tweak
>> (PYTHON_PATH and OLD_ICONV are two examples of them), sets default
>> values for them based on $(uname -S), then includes config.mak file
>> the user can optionally create to override these platform defaults.
>> This infrastructure is used across platforms, not just for NetBSD.
>
> If you have to choose a single PYTHON_PATH, the one you picked is right
> (for now, and likely will be right for a long time).
>
> But as a general rule, I think configure tests are preferable to
> OS-specific variables.

I forgot to mention that we also ship configure (and keep track of
configure.ac) so that optionally people can let autoconf machinery
to create config.mak.autogen to be included at the same place as
handcrafted config.mak in their build process.  I do not offhand
know if we do "for p in python python2.6 python2.7; do ..." kind of
thing, though.

> A large number of people on NetBSD use git, but almost all of them get
> it via pkgsrc (which is also where they get perl, emacs, svn, and
> everything else you didn't find in /usr/bin).  The exception would be
> people that want to hack on git itself.

Yeah, that much I figured ;-)

> People who want gnu libiconv can install the libiconv package from
> pkgsrc.  (I'm guessing OLD_ICONV means "POSIX iconv", without GNU
> extensions (iconvctl?).)

It refers to the type of the second parameter to iconv(); OLD_ICONV
makes it take "const char *", as opposed to "char *", the latter of
which matches

  http://pubs.opengroup.org/onlinepubs/9699919799/functions/iconv.html

> So, it doesn't matter too much for pkgsrc what you change, because it
> can be patched anyway (once, for all users).

Yes.  The values in the Makefile are fallback defaults, and matters
only to people who build from the source.

Re: [RFH] NetBSD 6?

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]


Junio C Hamano <gitster@pobox.com> writes:

> I forgot to mention that we also ship configure (and keep track of
> configure.ac) so that optionally people can let autoconf machinery
> to create config.mak.autogen to be included at the same place as
> handcrafted config.mak in their build process.  I do not offhand
> know if we do "for p in python python2.6 python2.7; do ..." kind of
> thing, though.

pkgsrc uses the configure method, but it seems not to output a
PYTHON_PATH.  It looks like automake's python.m4 is not used by git's
configure.ac.  But pkgsrc passes PYTHON_PATH in the environment to make,
so it works out currently.

> It refers to the type of the second parameter to iconv(); OLD_ICONV
> makes it take "const char *", as opposed to "char *", the latter of
> which matches
>
>   http://pubs.opengroup.org/onlinepubs/9699919799/functions/iconv.html

Thanks - I now see our extra const and am looking into it.

[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFH] NetBSD 6?

[Stefano Lattarini]

On 01/03/2013 07:27 PM, Greg Troxel wrote:
> 
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> I forgot to mention that we also ship configure (and keep track of
>> configure.ac) so that optionally people can let autoconf machinery
>> to create config.mak.autogen to be included at the same place as
>> handcrafted config.mak in their build process.  I do not offhand
>> know if we do "for p in python python2.6 python2.7; do ..." kind of
>> thing, though.
> 
> pkgsrc uses the configure method, but it seems not to output a
> PYTHON_PATH.  It looks like automake's python.m4 is not used by git's
> configure.ac.
>
That is not surprising, since Git build system doesn't use Automake :-)

In addition, it's worth nothing that Automake's python support (both
in its m4 and make components) is intended as an help to install python
*modules* from Autotools-based packages; using it for mere stand-alone
scripts would be overkill.

In the case of Git, an adapted version of something like this might
be enough:

  AC_CHECK_PROGS([PYTHON], [python python2.7 python2.6 python2.5])
  if test -z "$PYTHON"; then
    AC_MSG_WARNING([python not found])
  fi

  $PYTHON -c 'check-not-py3k' || AC_MSG_WARNING([Python 3 is not supported])
  $PYTHON -c 'check-its-version' || AC_MSG_WARNING([python is too old])

(Automake itself uses, in its own build system, a similar idiom to
look for a Perl interpreter at configure runtime).

Not my itch so far, but I will happily review a patch if anyone is
willing to write it.

> But pkgsrc passes PYTHON_PATH in the environment to make,
> so it works out currently.
> 
>> It refers to the type of the second parameter to iconv(); OLD_ICONV
>> makes it take "const char *", as opposed to "char *", the latter of
>> which matches
>>
>>   http://pubs.opengroup.org/onlinepubs/9699919799/functions/iconv.html
> 
> Thanks - I now see our extra const and am looking into it.

Regards,
  Stefano

Re: [RFH] NetBSD 6?

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]


Junio C Hamano <gitster@pobox.com> writes:

>>  [OLD_ICONV]

> It refers to the type of the second parameter to iconv(); OLD_ICONV
> makes it take "const char *", as opposed to "char *", the latter of
> which matches
>
>   http://pubs.opengroup.org/onlinepubs/9699919799/functions/iconv.html

I just wanted to follow up on this.  It turns out that the old POSIX
standard was buggy (header file and function spec were different), and
they resolved it in favor of non-const.  NetBSD followed the const way,
and just now documented that with links to the standards email archives.

Interestingly, GNU iconv 1.14 seems to define it as const also:

  https://www.gnu.org/savannah-checkouts/gnu/libiconv/documentation/libiconv-1.14/iconv.3.html

(which matches man/iconv.3 in the tarball).

When I build libiconv-1.14, it produces a .h with const.  But it has a
configure test to check if there is a host include file with const, and
puts the const in the built header file or not to match!
In include/iconv.h.in, there is:

  extern size_t iconv (iconv_t cd,
      @ICONV_CONST@ char* * inbuf, size_t *inbytesleft,
       char* * outbuf, size_t *outbytesleft);

Someday, it would be nice to have the configure test not fail an iconv
implementation just because of the const, unless the presence of const
is causing a real problem.  But I can understand that no one thinks
that's important enough to get around to.



[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]

Re: [RFH] NetBSD 6?

[Junio C Hamano]

Greg Troxel <gdt@ir.bbn.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>>>  [OLD_ICONV]
>
>> It refers to the type of the second parameter to iconv(); OLD_ICONV
>> makes it take "const char *", as opposed to "char *", the latter of
>> which matches
>>
>>   http://pubs.opengroup.org/onlinepubs/9699919799/functions/iconv.html
>
> I just wanted to follow up on this.  It turns out that the old POSIX
> standard was buggy (header file and function spec were different), and
> they resolved it in favor of non-const.  NetBSD followed the const way,
> and just now documented that with links to the standards email archives.
>
> Interestingly, GNU iconv 1.14 seems to define it as const also:
>
>   https://www.gnu.org/savannah-checkouts/gnu/libiconv/documentation/libiconv-1.14/iconv.3.html
>
> (which matches man/iconv.3 in the tarball).
>
> When I build libiconv-1.14, it produces a .h with const.  But it has a
> configure test to check if there is a host include file with const, and
> puts the const in the built header file or not to match!
> In include/iconv.h.in, there is:
>
>   extern size_t iconv (iconv_t cd,
>       @ICONV_CONST@ char* * inbuf, size_t *inbytesleft,
>        char* * outbuf, size_t *outbytesleft);
>
> Someday, it would be nice to have the configure test not fail an iconv
> implementation just because of the const, unless the presence of const
> is causing a real problem.  But I can understand that no one thinks
> that's important enough to get around to.

Interesting.

Don't get too offended by the "OLD_" prefix to that symbol, by the
way.  I do not think "old" means "old and broken hence fixed in
newer version and you are low life if you live on a platform that
has to define it" ;-).

We just needed to have a boolean to tell which variant it is to let
the compiler build objects without complaining, and we named that
switch as OLD_ICONV.

Re: [RFH] NetBSD 6?

[Greg Troxel]

[-- Attachment #1: Type: text/plain, Size: 841 bytes --]


Junio C Hamano <gitster@pobox.com> writes:

> Don't get too offended by the "OLD_" prefix to that symbol, by the
> way.  I do not think "old" means "old and broken hence fixed in
> newer version and you are low life if you live on a platform that
> has to define it" ;-).

Thanks - it did throw me at the beginning, because I expected that it
lead to using a copy of GNU iconv and not using the native one.
But it will probably confuse few enough people that changing to
CONST_ICONV is not warranted...

> We just needed to have a boolean to tell which variant it is to let
> the compiler build objects without complaining, and we named that
> switch as OLD_ICONV.

I get that, now that I read utf8.c.  It's amusing that git's own
function is const, and on non-OLD_ICONV platforms has to cast away the
const for standards-compliant iconv.


[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]