[SSSD] [PATCHES][DING-LIBS] Fix two distcheck issues with libini_config

Jan Zelený jzeleny at redhat.com
Mon May 14 13:31:09 UTC 2012


> 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 at 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?

Because only foo.conf serves you as file for testing of the access permissions. 
Basically make distcheck led to another set of access rights the file could 
have, leading to failure of the test. The solution was to move this file to 
builddir during make distcheck and change access rights there. There is no 
need to do this for other config files.

Jan

> >>> 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20120514/1bd680c5/attachment.sig>


More information about the sssd-devel mailing list