Re: Fwd: Re: [Platform-testers] new snapshot available: coreutils-8.12.178-df9cd

[Jim Meyering]

Markus Duft wrote:

> On 09/08/11 14:00, Jim Meyering wrote:
>> Markus Duft wrote:
>>> On 09/08/11 10:17, Jim Meyering wrote:
>>>> Markus Duft wrote:
>>>>
> [snip]
>>
>> Thanks.  I've pointed out a few issues, hoping you'll adjust
>> accordingly and resubmit.
>
> sure :) this exercised my git-rebase-foo a lot though ...
>
>>
>>> setgroups is not needed here, as it something different. i have 2
>>
>> I added that conjunct quite deliberately.
>> If we make the compromise of not detecting the getgr* performance
>> problems, then we need some other way to ensure that we use the
>> _nomembers functions only when needed.  Appending " && ! HAVE_SETGROUPS"
>> was my way of saying to use this kludge only on a system that also has
>> the defect of a missing setgroups function.  Yes, that deserves a
>> comment.
>
> hm. ok. i added and commented this, although it feels quite wrong for

As well it should.
As I mentioned initially, a better way would be to have
a configure-time run-test that would detect the losing getgr*
functions and set a macro like SLOW_GETGR__FUNCTIONS in that case.
Then, when/if Interix fixes them, that work-around code will stop
being used all by itself.  But run-tests are best avoided, since
they cause trouble when cross-compiling, and besides anything
that attempts to detect a performance problem like that is
doomed to fail some of the time.

So we compromise.

> me. is there any platform that has the _nomembers functions? if yes,
> would it do any harm to use them? idk...

If some other system has those _nomembers functions,
do you know that using them won't have some unpleasant side-effect?
What if some other system has working getgr* functions *and* the
_nomembers variants.  Would you still want to use the _nomembers ones?
I wouldn't.

> heh. while looking for a location to add the check, i realized there
> is one already! :) so i ommitted this step. hope thats ok, my build
> goes through, but for obvious reasons this is not significant (as i'll
> never have it defined). a quick grep over the build tree yields this:
>
> mduft coreutils-8.12.193-d8dc8 $ find . -type f | xargs grep SETGROUPS
> ./autom4te.cache/traces.0:m4trace:configure.ac:46: -1-
> AH_OUTPUT([HAVE_SETGROUPS], [/* Define to 1 if you have the
> setgroups\' function. */
> ./autom4te.cache/traces.0:@%:@undef HAVE_SETGROUPS])
> ./ChangeLog-2006: * src/setuidgid.c (main) [! HAVE_SETGROUPS]: Don't
> call setgroups.
> ./lib/config.h:/* #undef HAVE_SETGROUPS */
> ./lib/config.hin:#undef HAVE_SETGROUPS
> ./lib/config.hin~:#undef HAVE_SETGROUPS
> ./src/chroot.c:#if ! HAVE_SETGROUPS
> ./src/setuidgid.c:#if HAVE_SETGROUPS
> ./src/system.h:   other than interix, check for HAVE_SETGROUPS, as interix is
> ./src/system.h:#if ! HAVE_SETGROUPS
>
>>
> [snip]
>
> hope the new versions are acceptable. otherwise, i'll be happy to
> adjust once more, and resubmit tomorrow :)

Thanks.  I'll review one more time and apply these post-release.