[SSSD] [PATCH v3] Remove trailing whitespace

Lukas Slebodnik lslebodn at redhat.com
Thu Aug 27 08:29:55 UTC 2015


On (18/08/15 14:47), Michal Židek wrote:
>On 08/18/2015 02: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
>
>Honestly I think it is OK if this test is properly
>executed on Fedora only and disabled elsewhere.
>The code is the same everywhere so if there are no
>trailing whitespaces on Fedora then there are no
>trailing whitespaces on other systems as well.
>
>This is different from testing functionality where we
>want to check if something is broken on different
>platforms.
>
>Ofcourse if making it multiplatform is easy, then go
>for it, but I would not invest too much effort in making
>this test multiplatform because I do not see additional
>value here.
>
Package maintainers in other distribution usually do not spend
time with troubleshooting tests. The result is disabled tests at all.
It's not our goal.

LS


More information about the sssd-devel mailing list