public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: "Torsten Bögershausen" <tboegi@web•de>
Cc: Ramsay Jones <ramsay@ramsay1•demon.co.uk>,
	git@vger•kernel.org, j6t@kdbg•org
Subject: Re: [RFC] test-lib.sh: No POSIXPERM for cygwin
Date: Tue, 19 Mar 2013 14:10:02 -0700	[thread overview]
Message-ID: <7vvc8ngkph.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7v620nhzle.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Tue, 19 Mar 2013 14:03:09 -0700")

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

> Torsten Bögershausen <tboegi@web•de> writes:
>
>> Use a compile switch IGNORECYGWINFSTRICKS to disable the usage
>> of cygwin_lstat_fn() only in path.c
>
> The analysis of the problem and the basic idea to disable the
> fast-but-lying fstricks in the code that matters may be good, but
> the implementation is questionable.
>
> What if we later need to move functions around, etc., so that some
> other calls in path.c still do want to use the fstricks bit while
> the existing ones in the file want the real lstat() information?
>
> Actually, that already is the case.  The call to lstat() in
> validate_headref() only cares about the S_ISXXX() type and can
> afford to use the fast-and-lying one, no?
>
> How about doing something like this in the generic codepath, and
> implement your own cygwin_true_mode_bits() function at the cygwin
> compatibility layer, and add
>
>     #define true_mode_bits cygwin_true_mode_bits
>
> in the compat/cygwin.h file?  The change has the documentation value
> to clarify what each lstat() is used for, too.
>
> Ideally, the "#ifndef true_mode_bits" part may want to become a
> generic helper function if there are lstat() calls in other files
> that cygwin wants to use the real lstat() not the fast-but-lying
> one, but one step at a time.
>
> Hrm?
>
>  path.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/path.c b/path.c
> index d3d3f8b..d0b31e5 100644
> --- a/path.c
> +++ b/path.c
> @@ -14,6 +14,21 @@
>  #include "strbuf.h"
>  #include "string-list.h"
>  
> +#ifndef true_mode_bits
> +/*
> + * The replacement lstat(2) we use on Cygwin is incomplete and
> + * lies about permission bits; most of the time we do not care,
> + * but the callsites of this wrpper do care.
> + */
> +static int true_mode_bits(const char *path, int *mode)
> +{
> +	struct stat st;
> +	if (lstat(path, &st) < 0)
> +		return -1;
> +	return st.st_mode;

Of course this should be more like

	*mode = st.st_mode;
        return 0;

but I think you got the idea ;-)

> +}
> +#endif
> +
>  static char bad_path[] = "/bad-path/";
>  
>  static char *get_pathname(void)
> @@ -400,9 +415,8 @@ int set_shared_perm(const char *path, int mode)
>  		return 0;
>  	}
>  	if (!mode) {
> -		if (lstat(path, &st) < 0)
> +		if (true_mode_bits(path, &mode) < 0)
>  			return -1;
> -		mode = st.st_mode;
>  		orig_mode = mode;
>  	} else
>  		orig_mode = 0;

next prev parent reply	other threads:[~2013-03-19 21:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-19 19:49 [RFC] test-lib.sh: No POSIXPERM for cygwin Torsten Bögershausen
2013-03-19 21:03 ` Junio C Hamano
2013-03-19 21:10   ` Junio C Hamano [this message]
  -- strict thread matches above, loose matches on Subject: below --
2013-03-23 12:40 Torsten Bögershausen
2013-03-24  2:49 ` Eric Sunshine
2013-03-25 15:53   ` Torsten Bögershausen
2013-01-27 14:57 Torsten Bögershausen
2013-02-06  9:34 ` Erik Faye-Lund
2013-02-06 20:16   ` Torsten Bögershausen
2013-02-07 18:25 ` Ramsay Jones
2013-02-07 19:35   ` Junio C Hamano
2013-02-08  6:08     ` Torsten Bögershausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vvc8ngkph.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=j6t@kdbg$(echo .)org \
    --cc=ramsay@ramsay1$(echo .)demon.co.uk \
    --cc=tboegi@web$(echo .)de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox