On 05/14/2012 07:13 AM, Stephen Gallagher wrote:
On Mon, 2012-05-14 at 09:19 +0200, Jan Zelený wrote:
>> On 05/11/2012 04:21 PM, Dmitri Pal wrote:
>>> On 05/11/2012 10:04 AM, Stephen Gallagher wrote:
>>>> Patch 0001: The unit tests for libini_config need to be able to validate
>>>> specific permissions on some sample data. However, 'make
distcheck'
>>>> always removes the 'write' permissions on data in the $srcdir in
order
>>>> to test that the build does not modify the contents of $srcdir. So
we'll
>>>> solve this by moving the sample data being tested to the $builddir.
>>>>
>>>> Patch 0002: There was an issue where, if the repo was cloned as root,
>>>> the sample data did not have the proper permissions and would fail the
>>>> checks. Previously, we had a hackish workaround where we would just
>>>> check to see if it matched the root-necessary permissions. Now we'll
>>>> just explicitly set them properly beforehand.
>>>>
>>>>
>>>> _______________________________________________
>>>> sssd-devel mailing list
>>>> sssd-devel(a)lists.fedorahosted.org
>>>>
https://fedorahosted.org/mailman/listinfo/sssd-devel
>>> I do not like the first patch.
>>> I think it will fail when you build in tree. Have you checked?
>> Ok. I checked it builds fine. However there are other places where the
>> same change needs to be implemented:
>>
>> ini_config_ut.c: srcdir = getenv("srcdir");
>> ini_parse_ut.c: srcdir = getenv("srcdir");
>> ini_parse_ut.c: srcdir = getenv("srcdir");
> I don't think we need to do that since those instances are for different files.
> The only file affected by this change is foo.conf and Stephen correctly changed
> all instances of the srcdir for that file.
foo.conf is no different that other test config files that reside in the
ini.d directory. I do not understand why the change needed for one and
not for the others. Can you please elaborate?
>> I am also not sure whether we need to change the build dir.
This is used
>> to compare generated files that are created during the build with the
>> files that are stored in the source.
>>
>> ini_parse_ut.c: builddir = getenv("builddir");
For what it's worth, this operation is guaranteed to be useless. In all
versions of autoconf that support $builddir, it's actually hard-coded to
be './'
It's mostly there as a convenience variable to make the code easier to
read.
> I'm not sure what do you mean. I think the test is ok, since it copies the file
> to a different dir, tests it there and then removes it.
>
> ACK from me, just please consider one small change - use memcpy() instead of
> sprintf(), there is no need for sprintf() since the string is static. I'm
> attaching proposed change.
Ack.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
--
Thank you,
Dmitri Pal
Sr. Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/