[PATCH 0/5] Some autoconf patches

[PATCH 0/5] Some autoconf patches

[Jakub Narebski]

This is set of a few autoconf patches, based on 'master' 
  (commit 6b4fd65cce00d4adb5dd3ed2960ba355057c5a97)
What is left is check if "libc/libiconv properly support iconv.",
i.e. NO_ICONV check.

Summary:
---
 config.mak.in |    2 ++
 configure.ac  |   22 ++++++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)


Shortlog:
 * [PATCH 1/5] autoconf: Add some commented out variables to config.mak.in
 * [PATCH 2/5] autoconf: Add -liconv to LIBS when NEEDS_LIBICONV
 * [PATCH 3/5] autoconf: Preliminary check for working mmap
 * [PATCH 4/5] autoconf: Check for subprocess.py
 * [PATCH 5/5] autoconf: Quote AC_CACHE_CHECK arguments
-- 
Jakub Narebski
ShadeHawk on #git
Poland

[PATCH 1/5] autoconf: Add some commented out variables to config.mak.in

[Jakub Narebski]

Add commented out variables for the features which can be detected by
autoconf, but are not implemented yet, to config.mak.in file.

This way we can easily see which tests are not implemented yet.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 config.mak.in |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 369e611..2947560 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -37,4 +37,6 @@ NO_C99_FORMAT=@NO_C99_FORMAT@
 NO_STRCASESTR=@NO_STRCASESTR@
 NO_STRLCPY=@NO_STRLCPY@
 NO_SETENV=@NO_SETENV@
+#NO_MMAP=@NO_MMAP@
+#NO_ICONV=@NO_ICONV@
 
-- 
1.4.1.1

[PATCH 2/5] autoconf: Add -liconv to LIBS when NEEDS_LIBICONV

[Jakub Narebski]

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Just in case; it could matter for testing if iconv is properly
supported (NO_ICONV test).

 configure.ac |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 36f9cd9..fc5b813 100644
--- a/configure.ac
+++ b/configure.ac
@@ -147,6 +147,7 @@ AC_CHECK_LIB([c], [iconv],
 [NEEDS_LIBICONV=],
 [NEEDS_LIBICONV=YesPlease])
 AC_SUBST(NEEDS_LIBICONV)
+test -n "$NEEDS_SOCKET" && LIBS="$LIBS -liconv"
 #
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
-- 
1.4.1.1

[PATCH 3/5] autoconf: Preliminary check for working mmap

[Jakub Narebski]

Use AC_FUNC_MMAP check to check if the `mmap' function exists and
works correctly.  (It only checks private fixed mapping of
already-mapped memory.)

Still it is better than having no mmap check at all.
Attention: uses implementation detail of AC_FUNC_MMAP!

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch sent earlier in other patch series and dropped,
as git uses private mapping, not private fixed. I think
that this check is better than no check at all...

 config.mak.in |    2 +-
 configure.ac  |    7 +++++++
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/config.mak.in b/config.mak.in
index 2947560..2c8fd2c 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -37,6 +37,6 @@ NO_C99_FORMAT=@NO_C99_FORMAT@
 NO_STRCASESTR=@NO_STRCASESTR@
 NO_STRLCPY=@NO_STRLCPY@
 NO_SETENV=@NO_SETENV@
-#NO_MMAP=@NO_MMAP@
+NO_MMAP=@NO_MMAP@
 #NO_ICONV=@NO_ICONV@
 
diff --git a/configure.ac b/configure.ac
index fc5b813..799321e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -249,6 +249,13 @@ AC_CHECK_FUNC(setenv,
 AC_SUBST(NO_SETENV)
 #
 # Define NO_MMAP if you want to avoid mmap.
+AC_FUNC_MMAP
+if test $ac_cv_func_mmap_fixed_mapped != yes; then
+	NO_MMAP=YesPlease
+else
+	NO_MMAP=
+fi
+AC_SUBST(NO_MMAP)
 #
 # Define NO_ICONV if your libc does not properly support iconv.
 
-- 
1.4.1.1

[PATCH 4/5] autoconf: Check for subprocess.py

[Jakub Narebski]

Add custom test for checking if Python comes with subprocess.py, or
should we use our own subprocess.py by defining WITH_OWN_SUBPROCESS_PY.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Taken from Makefile.

Perhaps we would get rid of Python dependency soon.

 configure.ac |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/configure.ac b/configure.ac
index 799321e..67a7aa9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -268,6 +268,16 @@ # Define NO_SYMLINK_HEAD if you never wa
 # Enable it on Windows.  By default, symrefs are still used.
 #
 # Define WITH_OWN_SUBPROCESS_PY if you want to use with python 2.3.
+AC_CACHE_CHECK(for subprocess.py,
+ ac_cv_python_has_subprocess_py,
+[if $PYTHON_PATH -c 'import subprocess' 2>/dev/null; then
+	ac_cv_python_has_subprocess_py=yes
+else
+	ac_cv_python_has_subprocess_py=no
+fi])
+if test $ac_cv_python_has_subprocess_py != yes; then
+	GIT_CONF_APPEND_LINE([WITH_OWN_SUBPROCESS_PY=YesPlease])
+fi
 #
 # Define NO_ACCURATE_DIFF if your diff program at least sometimes misses
 # a missing newline at the end of the file.
-- 
1.4.1.1

[PATCH 5/5] autoconf: Quote AC_CACHE_CHECK arguments

[Jakub Narebski]

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Just in case.

 configure.ac |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index 67a7aa9..85317a3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -205,8 +205,8 @@ # Define NO_C99_FORMAT if your formatted
 # do not support the 'size specifiers' introduced by C99, namely ll, hh,
 # j, z, t. (representing long long int, char, intmax_t, size_t, ptrdiff_t).
 # some C compilers supported these specifiers prior to C99 as an extension.
-AC_CACHE_CHECK(whether formatted IO functions support C99 size specifiers,
- ac_cv_c_c99_format,
+AC_CACHE_CHECK([whether formatted IO functions support C99 size specifiers],
+ [ac_cv_c_c99_format],
 [# Actually git uses only %z (%zu) in alloc.c, and %t (%td) in mktag.c
 AC_RUN_IFELSE(
 	[AC_LANG_PROGRAM([AC_INCLUDES_DEFAULT],
@@ -268,8 +268,8 @@ # Define NO_SYMLINK_HEAD if you never wa
 # Enable it on Windows.  By default, symrefs are still used.
 #
 # Define WITH_OWN_SUBPROCESS_PY if you want to use with python 2.3.
-AC_CACHE_CHECK(for subprocess.py,
- ac_cv_python_has_subprocess_py,
+AC_CACHE_CHECK([for subprocess.py],
+ [ac_cv_python_has_subprocess_py],
 [if $PYTHON_PATH -c 'import subprocess' 2>/dev/null; then
 	ac_cv_python_has_subprocess_py=yes
 else
-- 
1.4.1.1

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> This patch sent earlier in other patch series and dropped,
> as git uses private mapping, not private fixed. I think
> that this check is better than no check at all...

I think the next major change that is needed fairly soon is to
be able to mmap parts of a large pack file (even with 32-bit
offset).  I haven't loooked into it deeply enough but it may or
may not turn out to be beneficial if we map at fixed location so
the requirements of "working mmap()" might change.  I'd be
happier if you held onto this part of the patch before that
happens.

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Jakub Narebski]

Junio C Hamano wrote:

> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> This patch sent earlier in other patch series and dropped,
>> as git uses private mapping, not private fixed. I think
>> that this check is better than no check at all...
> 
> I think the next major change that is needed fairly soon is to
> be able to mmap parts of a large pack file (even with 32-bit
> offset).  I haven't loooked into it deeply enough but it may or
> may not turn out to be beneficial if we map at fixed location so
> the requirements of "working mmap()" might change.  I'd be
> happier if you held onto this part of the patch before that
> happens.

Well, this patch perhaps not always would set NO_MMAP when it should 
(some broken mmap that does private+fixed but not private), but I guess that
if it would set NO_MMAP then mmap is broken (or doesn't exist at all).

I have noticed AC_FUNC_MMAP test, and thought why not to use it?
The simplest check if mmap exist would be to check libc for mmap function.


BTW the patch was reworked from previous version to reflect changes
in the compilation variables management (ability to unset variable).
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Shawn Pearce]

Jakub Narebski <jnareb@gmail.com> wrote:
> Junio C Hamano wrote:
> 
> > Jakub Narebski <jnareb@gmail.com> writes:
> > 
> >> This patch sent earlier in other patch series and dropped,
> >> as git uses private mapping, not private fixed. I think
> >> that this check is better than no check at all...
> > 
> > I think the next major change that is needed fairly soon is to
> > be able to mmap parts of a large pack file (even with 32-bit
> > offset).  I haven't loooked into it deeply enough but it may or
> > may not turn out to be beneficial if we map at fixed location so
> > the requirements of "working mmap()" might change.  I'd be
> > happier if you held onto this part of the patch before that
> > happens.
> 
> Well, this patch perhaps not always would set NO_MMAP when it should 
> (some broken mmap that does private+fixed but not private), but I guess that
> if it would set NO_MMAP then mmap is broken (or doesn't exist at all).
> 
> I have noticed AC_FUNC_MMAP test, and thought why not to use it?
> The simplest check if mmap exist would be to check libc for mmap function.

Are there really mmap implementations that will only do
private+fixed?  Sick.

I'm maybe only 1/3 of the way through the sliding window mmap
implementation.  I've got a good chunk of sha1_file.c converted but I
still have to deal with the copying in pack-objects.c and the verify
code in verify-pack.c.  I'm hoping I can send a preliminary patch
series tomorrow as I'm going to work on it more tonight and tomorrow.

-- 
Shawn.

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> I'm maybe only 1/3 of the way through the sliding window mmap
> implementation.  I've got a good chunk of sha1_file.c converted but I
> still have to deal with the copying in pack-objects.c and the verify
> code in verify-pack.c.  I'm hoping I can send a preliminary patch
> series tomorrow as I'm going to work on it more tonight and tomorrow.

Thanks -- I was tempted to do this myself after finishing the
index_64 change in "pu" branch, but have resisted the temptation
myself so far.  Being lazy, the less I have to code the better,
naturally ;-).
 

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > I'm maybe only 1/3 of the way through the sliding window mmap
> > implementation.  I've got a good chunk of sha1_file.c converted but I
> > still have to deal with the copying in pack-objects.c and the verify
> > code in verify-pack.c.  I'm hoping I can send a preliminary patch
> > series tomorrow as I'm going to work on it more tonight and tomorrow.
> 
> Thanks -- I was tempted to do this myself after finishing the
> index_64 change in "pu" branch, but have resisted the temptation
> myself so far.  Being lazy, the less I have to code the better,
> naturally ;-).

I thought that might be the case.  I should be able to finish it
up tomorrow.  :-)

I don't know if I've made this more complex than I really need to
but I've permitted multiple windows per pack.  There is just one
LRU of all windows across all packs and a maximum amount of address
space to use for pack mappings.  Least recently used window gets
tossed when we need a different window.  This permits us to keep
say a window active on the front of a pack (near the commits) and
another different active window closer to the back (near the blobs).

That multiple window feature made it a slightly non-trivial copy
and paste from fast-import but I think its worth it for tree walking
type applications.

-- 
Shawn.

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> I don't know if I've made this more complex than I really need to
> but I've permitted multiple windows per pack.  There is just one
> LRU of all windows across all packs and a maximum amount of address
> space to use for pack mappings.  Least recently used window gets
> tossed when we need a different window.  This permits us to keep
> say a window active on the front of a pack (near the commits) and
> another different active window closer to the back (near the blobs).

Sounds good. That is exactly what I was expecting it to be done.

Re: [PATCH 2/5] autoconf: Add -liconv to LIBS when NEEDS_LIBICONV

[Jonas Fonseca]

Jakub Narebski <jnareb@gmail.com> wrote Tue, Sep 05, 2006:
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> Just in case; it could matter for testing if iconv is properly
> supported (NO_ICONV test).
> 
>  configure.ac |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 36f9cd9..fc5b813 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -147,6 +147,7 @@ AC_CHECK_LIB([c], [iconv],
>  [NEEDS_LIBICONV=],
>  [NEEDS_LIBICONV=YesPlease])
>  AC_SUBST(NEEDS_LIBICONV)
> +test -n "$NEEDS_SOCKET" && LIBS="$LIBS -liconv"

I see that this has entered already, but it looks like it needs a
s/NEEDS_SOCKET/NEEDS_LIBICONV/

>  #
>  # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
>  # Patrick Mauritz).

-- 
Jonas Fonseca

[PATCH] autoconf: Fix copy'n'paste error

[Jakub Narebski]

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 configure.ac |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure.ac b/configure.ac
index 85317a3..482c849 100644
--- a/configure.ac
+++ b/configure.ac
@@ -147,7 +147,7 @@ AC_CHECK_LIB([c], [iconv],
 [NEEDS_LIBICONV=],
 [NEEDS_LIBICONV=YesPlease])
 AC_SUBST(NEEDS_LIBICONV)
-test -n "$NEEDS_SOCKET" && LIBS="$LIBS -liconv"
+test -n "$NEEDS_LIBICONV" && LIBS="$LIBS -liconv"
 #
 # Define NEEDS_SOCKET if linking with libc is not enough (SunOS,
 # Patrick Mauritz).
-- 
1.4.2

Re: [PATCH 2/5] autoconf: Add -liconv to LIBS when NEEDS_LIBICONV

[Junio C Hamano]

Jonas Fonseca <fonseca@diku.dk> writes:

> Jakub Narebski <jnareb@gmail.com> wrote Tue, Sep 05, 2006:
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>> ---
>> Just in case; it could matter for testing if iconv is properly
>> supported (NO_ICONV test).
>> 
>>  configure.ac |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>> 
>> diff --git a/configure.ac b/configure.ac
>> index 36f9cd9..fc5b813 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -147,6 +147,7 @@ AC_CHECK_LIB([c], [iconv],
>>  [NEEDS_LIBICONV=],
>>  [NEEDS_LIBICONV=YesPlease])
>>  AC_SUBST(NEEDS_LIBICONV)
>> +test -n "$NEEDS_SOCKET" && LIBS="$LIBS -liconv"
>
> I see that this has entered already, but it looks like it needs a
> s/NEEDS_SOCKET/NEEDS_LIBICONV/

Eek, eye-ballo-o.  Thanks for noticing.

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
> 
> > I don't know if I've made this more complex than I really need to
> > but I've permitted multiple windows per pack.  There is just one
> > LRU of all windows across all packs and a maximum amount of address
> > space to use for pack mappings.  Least recently used window gets
> > tossed when we need a different window.  This permits us to keep
> > say a window active on the front of a pack (near the commits) and
> > another different active window closer to the back (near the blobs).
> 
> Sounds good. That is exactly what I was expecting it to be done.

So I have this implemented but its against the 32 bit index.
It passes the full test suite and appears to be working as intended.

I am going to rebase the changes to the 64 bit index in `pu`
and clean up my history.  Its currently one massive commit with
lots of changes that should be broken down into slightly more
digestable chunks.

I'll try to do that tonight and get a patch series out.  My email
is currently proving to be unstable so you may not get the series
until later on Wed.

-- 
Shawn.

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Shawn Pearce <spearce@spearce.org> writes:
>> 
>> > I don't know if I've made this more complex than I really need to
>> > but I've permitted multiple windows per pack.  There is just one
>> > LRU of all windows across all packs and a maximum amount of address
>> > space to use for pack mappings.  Least recently used window gets
>> > tossed when we need a different window.  This permits us to keep
>> > say a window active on the front of a pack (near the commits) and
>> > another different active window closer to the back (near the blobs).
>> 
>> Sounds good. That is exactly what I was expecting it to be done.
>
> So I have this implemented but its against the 32 bit index.
> It passes the full test suite and appears to be working as intended.
>
> I am going to rebase the changes to the 64 bit index in `pu`
> and clean up my history.  Its currently one massive commit with
> lots of changes that should be broken down into slightly more
> digestable chunks.
>
> I'll try to do that tonight and get a patch series out.  My email
> is currently proving to be unstable so you may not get the series
> until later on Wed.

Take your time.  My usual git day is Wednesday but this week it
is shot and I won't be doing much gitting until later this week.

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Junio C Hamano]

Shawn Pearce <spearce@spearce.org> writes:

> Junio C Hamano <junkio@cox.net> wrote:
>> Shawn Pearce <spearce@spearce.org> writes:
>> 
>> > I don't know if I've made this more complex than I really need to
>> > but I've permitted multiple windows per pack.  There is just one
>> > LRU of all windows across all packs and a maximum amount of address
>> > space to use for pack mappings.  Least recently used window gets
>> > tossed when we need a different window.  This permits us to keep
>> > say a window active on the front of a pack (near the commits) and
>> > another different active window closer to the back (near the blobs).
>> 
>> Sounds good. That is exactly what I was expecting it to be done.
>
> So I have this implemented but its against the 32 bit index.
> It passes the full test suite and appears to be working as intended.

Well, if you are talking about git.git t/ directory, I bet that
"the full test suite" easily fits in a single window so much of
the new code is not exercised, unless you have a runtime option
to make use of very tiny window (32kB would be nice) for testing
;-).

> I am going to rebase the changes to the 64 bit index in `pu`
> and clean up my history.  Its currently one massive commit with
> lots of changes that should be broken down into slightly more
> digestable chunks.

Breaking things up would be nice.  Do you have a good testcase
out of Mozilla repository that _needs_ 64-bit index?  The code
in "pu" uses 64-bit index only 32-bit offset cannot describe the
pack, so I only tested with an artificial case with the kernel
archive with .idx converted with convert-idx to 64-bit.

Re: [PATCH 3/5] autoconf: Preliminary check for working mmap

[Shawn Pearce]

Junio C Hamano <junkio@cox.net> wrote:
> Well, if you are talking about git.git t/ directory, I bet that
> "the full test suite" easily fits in a single window so much of
> the new code is not exercised, unless you have a runtime option
> to make use of very tiny window (32kB would be nice) for testing
> ;-).

Heh.  t/ passed which meant basic rev listing and verify-pack
appeared OK.  I hardcoded the window size to be 2*getpagesize() (8192
bytes on my Mac OS X system) and started beating up Git against its
own 8 MiB pack.  The window code opened a ton of windows at once as
the PACK_MAX_SZ was 128 MiB.  So I hardcoded that to be 16384 bytes.
And the code worked right against the 8 MiB Git pack by opening
only 4 windows at once and closing the least-recently-used one.

I can easily parameterize both values in .git/config; there's no
real reason for them to be hardcoded.  That should make it easier
to craft some new test cases that push the windowing code.

> > I am going to rebase the changes to the 64 bit index in `pu`
> > and clean up my history.  Its currently one massive commit with
> > lots of changes that should be broken down into slightly more
> > digestable chunks.
> 
> Breaking things up would be nice.  Do you have a good testcase
> out of Mozilla repository that _needs_ 64-bit index?  The code
> in "pu" uses 64-bit index only 32-bit offset cannot describe the
> pack, so I only tested with an artificial case with the kernel
> archive with .idx converted with convert-idx to 64-bit.
> 

Not really.  We know the Mozilla pack decompressed is around 20 GiB.
I could decompress it and repack it using no deltas; that should
exceed 4 GiB.  It will take quite some time to run the decompression
and delta inflates, especially with the highly-compressed 450 MiB
pack Jon Smirl sent me.  I can do that kind of testing maybe later
next week after I get my new amd64 system built.

-- 
Shawn.