[SSSD] [PATCH v3] Remove trailing whitespace

Nikolai Kondrashov Nikolai.Kondrashov at redhat.com
Wed Aug 26 09:37:55 UTC 2015


On 08/18/2015 03:31 PM, Nikolai Kondrashov wrote:
> On 08/17/2015 08:42 PM, Lukas Slebodnik wrote:
>> ----- Original Message -----
>>> From: "Nikolai Kondrashov" <Nikolai.Kondrashov at redhat.com>
>>> To: "Development of the System Security Services Daemon" <sssd-devel at lists.fedorahosted.org>
>>> Sent: Monday, August 17, 2015 6:40:36 PM
>>> Subject: [SSSD] [PATCH v3] Remove trailing whitespace
>>>
>>> On 08/17/2015 09:37 AM, Lukas Slebodnik wrote:
>>>> On (13/08/15 15:48), Nikolai Kondrashov wrote:
>>>>> On 08/13/2015 02:57 PM, Lukas Slebodnik wrote:
>>>>>> On (13/08/15 14:47), Nikolai Kondrashov wrote:
>>>>>>> +if GIT_CHECKOUT
>>>>>>> +export GIT_DIR=$(abs_top_srcdir)/.git
>>>>>>> +export GIT_WORK_TREE=$(abs_top_srcdir)
>>>>>>> +endif
>>>>>> It's not good idea to export such in general code.
>>>>>> you might greak something. I didn't try but you might break
>>>>>> something in fedpkg + local build.
>>>>>>
>>>>>> Better solution would to to export make variable abs_top_srcdir
>>>>>> as ABS_TOP_SRCDIR in TESTS_ENVIRONMENT
>>>>>>
>>>>>> and print warnig/error from test if this variable is not defined.
>>>>>
>>>>> Yeah, it's not nice. I doubt that it will break anything as it is very
>>>>> specific as to where the git repo is, but I wouldn't vouch for it.
>>>>>
>>>> But on the other hand ABS_TOP_SRCDIR is more abstract
>>>> and there is higher probability it will be used in other tests.
>>>>
>>>>> We'll need to pass it both to the TESTS_ENVIRONMENT and to *srpm targets
>>>>> as
>>>>> well, which will make it uglier, but I'll do it.
>>>>>
>>>> You can still export GIT_DIR and GIT_WORK_TREE inside script if
>>>> ABS_TOP_SRCDIR is defined. Otherwise test can be skipped
>>>> https://www.gnu.org/software/automake/manual/html_node/Scripts_002dbased-Testsuites.html
>>>
>>> Alright here's another version.
>>>
>>> I didn't change it much. Simply moved export of GIT_DIR and GIT_WORK_TREE
>>> lower, to TESTS_ENVIRONMENT and *rpm* targets. I didn't add exporting of
>>> ABS_TOP_SRCDIR, because that way the whitespace test would always require it,
>>> making manual execution inconvenient, or would need additional logic with
>>> assumptions there. This way we avoid having another variable and a way to
>>> pass
>>> build parameters around.
>>>
>>> BTW, Lukas, could you please describe what kind of problems you expect in
>>> "fedpkg + local build"? Perhaps it's not a problem after all?
>>>
>>
>> fedpkg compile
>>
>> usage: fedpkg compile [-h] [--arch ARCH] [--short-circuit]
>>
>> This command calls rpmbuild to compile the source. By default the prep and configure
>> stages will be done as well, unless the short-circuit option is used
>>
>> And where I can see a problem.
>> fedpkg is a git wrapper. The git contains patches + spec file.
>>
>> If you call "fedpkg compile" then rpmbuild will extract tarball and apply patches.
>> configure will detect there is a git and the test will fail because
>> patches contains trailing whitespaces.
>>
>> So GIT_WORK_TREE is not an equivalent to ABS_TOP_SRCDIR.
>> This is an example of for fedora.
>> There might be other distribution where it can cause troubles as well.
>> So it's better to be defensive.
>
> Thank you, Lukas. I understand your concern. However, the second patch in the
> series specifically makes configure detect git repo root at the srcdir only.
> So, if srcdir is in a subdirectory of a git repo and is not a git repo itself
> (which would be fine, BTW), configure won't detect it.
>
> The only problem I see at the moment is Debian. They store the packaging
> together with the source repo [1]. However, we can't do anything about that
> regardless whether we use ABS_TOP_SRCDIR or not, and they should be able to
> add "debian/.*" to the exclude patterns of the whitespace test.
>
> Nick
>
> [1] git://anonscm.debian.org/pkg-sssd/sssd.git

Lukas, does my answer prove we'll be fine with the patch set as is?

Nick


More information about the sssd-devel mailing list