threads / discuss / 28971

Re: t/t1304: avoid -d option to setfacl (db82657)

Subject: Re: t/t1304: avoid -d option to setfacl (db82657)

## tl;dr

3 messages between Nov 18, 2011 and Nov 29, 2011.

replies: 2people: 3as markdown or json

Brandon Casey· Nov 18, 2011, 05:37 UTC · lore

[cc git@vger.kernel.org since that is the appropriate place for this discusion]

On Thu, Nov 17, 2011 at 8:59 PM, wsp <reply+c-729354-3460aca0fa61e627f9d1a271cf70a99d5c1e7e4e-921167@reply.github.com> wrote:

> Could this test case be reviewed again? It fails on FreeBSD where the appropriate way to specify default ACL's is with the "-d" option. The "d[efault]:" syntax is invalid on FreeBSD.

Well, I'm not sure there is a right answer here.

   Linux accepts -d or "d[efault]"
   Solaris only accepts "d[efault]"
   FreeBSD only accepts -d
   a quick search shows z/OS only accepts "d[efault]"
   other?

I think most everything else rolls their own implementation into chmod or chacl or something like that.

The abandoned POSIX draft does actually specify the FreeBSD behavior.

So I think it's kind of a toss-up. Which option we choose should probably depend on whether we get more test coverage by using the "d[efault]" notation or by using the -d option. That depends on whether there are more Solaris users compiling git or whether there are more FreeBSD users. I don't know the answer to that either. I tend to think there are very few of either.

-Brandon

Scott Parrish· Nov 19, 2011, 20:28 UTC · re: Brandon Casey · lore

Hi Brandon,

Thanks for posting my question. I realized shortly after my post to github that I should have posted to the mailing list instead.

On Nov 18, 2011, at 12:37 AM, Brandon Casey wrote:

Show 17 quoted lines

> [cc git@vger.kernel.org since that is the appropriate place for this discusion]
> 
> On Thu, Nov 17, 2011 at 8:59 PM, wsp
> <reply+c-729354-3460aca0fa61e627f9d1a271cf70a99d5c1e7e4e-921167@reply.github.com>
> wrote:
>> Could this test case be reviewed again? It fails on FreeBSD where the appropriate way to specify default ACL's is with the "-d" option. The "d[efault]:" syntax is invalid on FreeBSD.
> 
> Well, I'm not sure there is a right answer here.
> 
>   Linux accepts -d or "d[efault]"
>   Solaris only accepts "d[efault]"
>   FreeBSD only accepts -d
>   a quick search shows z/OS only accepts "d[efault]"
>   other?
> 
> I think most everything else rolls their own implementation into chmod
> or chacl or something like that.

OS X does indeed roll ACL management into chmod. The test passes there because the prereq isn't met (i.e. the setfacl command does not exist) and thus the tests are skipped.

Show 9 quoted lines

> 
> The abandoned POSIX draft does actually specify the FreeBSD behavior.
> 
> So I think it's kind of a toss-up.  Which option we choose should
> probably depend on whether we get more test coverage by using the
> "d[efault]" notation or by using the -d option.  That depends on
> whether there are more Solaris users compiling git or whether there
> are more FreeBSD users.  I don't know the answer to that either.  I
> tend to think there are very few of either.

Is there a reason conditional logic can't be used (perhaps keying off of `uname -s` or the like) so that we have coverage in all cases?

-S

Show 6 quoted lines

> 
> -Brandon
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jeff King· Nov 29, 2011, 12:23 UTC · re: Scott Parrish · lore

On Sat, Nov 19, 2011 at 03:28:49PM -0500, Scott Parrish wrote:

Show 11 quoted lines

> > The abandoned POSIX draft does actually specify the FreeBSD behavior.
> > 
> > So I think it's kind of a toss-up.  Which option we choose should
> > probably depend on whether we get more test coverage by using the
> > "d[efault]" notation or by using the -d option.  That depends on
> > whether there are more Solaris users compiling git or whether there
> > are more FreeBSD users.  I don't know the answer to that either.  I
> > tend to think there are very few of either.
> 
> Is there a reason conditional logic can't be used (perhaps keying off
> of `uname -s` or the like) so that we have coverage in all cases?

It's nice if we can auto-detect such things based on behavior, and then there's no need for maintaining a "uname -s" guess. Perhaps we can check at the beginning of t1304 which one works (or neither), and set a flag appropriately? It looks like we already check whether setfacl works at all, so this would be a natural extension.

I started to do a "how about this" patch, but I realized that in order to see whether "-d" worked as "default" versus "delete", we'll have to check the output of "getfacl" to see if it reports the default entry. And then I realized I have no idea what FreeBSD shows in such a case (surely not "d:*", right?).

-Peff