Hi everyone,
The attached patch fixes the issue of whitespace_test failing when no trailing whitespace was detected at all.
Nick
On (05/10/15 15:59), Nikolai Kondrashov wrote:
Hi everyone,
The attached patch fixes the issue of whitespace_test failing when no trailing whitespace was detected at all.
I do not understand the explanation. Trailing whitespaces are not detected with unchaged file po/ca.po either.
make -C po update-po changed the file "po/ca.po" and it should be ignored. $ make -C po update-po //snip $git status On branch python Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout -- <file>..." to discard changes in working directory)
modified: po/ca.po
no changes added to commit (use "git add" and/or "git commit -a") $ make check || echo FAILED //snip FAILED If I revert changes in po/ca.po (done by make -C po update-po) then whitespace test passes $ git checkout -f po/ $ make check && echo OK //snip OK
LS
On 10/05/2015 04:38 PM, Lukas Slebodnik wrote:
On (05/10/15 15:59), Nikolai Kondrashov wrote:
Hi everyone,
The attached patch fixes the issue of whitespace_test failing when no trailing whitespace was detected at all.
I do not understand the explanation.
Sorry, if I wasn't clear.
Trailing whitespaces are not detected with unchaged file po/ca.po either.
The trailing whitespace is there in the unchanged "po/ca.po" file. I.e. there *is* trailing whitespace in the source tree. However, we chose to ignore it. It is detected by "git grep" but suppressed by AWK, in whitespace_test.
With updated "po/ca.po" the trailing whitespace is removed. As a consequence, there is no more trailing whitespace in the whole source tree and "git grep" returns 1, as it hasn't found anything. That makes whitespace_test quit with status 1, failing the test.
The patch makes whitespace_test ignore exit status 1 from "git grep" (but still abort on other error codes), AWK happily processes empty output and all is fine.
Hope this makes it clearer.
Nick
On (05/10/15 16:49), Nikolai Kondrashov wrote:
On 10/05/2015 04:38 PM, Lukas Slebodnik wrote:
On (05/10/15 15:59), Nikolai Kondrashov wrote:
Hi everyone,
The attached patch fixes the issue of whitespace_test failing when no trailing whitespace was detected at all.
I do not understand the explanation.
Sorry, if I wasn't clear.
Trailing whitespaces are not detected with unchaged file po/ca.po either.
The trailing whitespace is there in the unchanged "po/ca.po" file. I.e. there *is* trailing whitespace in the source tree. However, we chose to ignore it. It is detected by "git grep" but suppressed by AWK, in whitespace_test.
With updated "po/ca.po" the trailing whitespace is removed. As a consequence, there is no more trailing whitespace in the whole source tree and "git grep" returns 1, as it hasn't found anything. That makes whitespace_test quit with status 1, failing the test.
The patch makes whitespace_test ignore exit status 1 from "git grep" (but still abort on other error codes), AWK happily processes empty output and all is fine.
I was not able to find anything about return codes in the man "git grep" but it seems that it's related to /usr/bin/grep
man grep says: EXIT STATUS Normally the exit status is 0 if a line is selected, 1 if no lines were selected, and 2 if an error occurred. However, if the -q or --quiet or --silent is used and a line is selected, the exit status is 0 even if an error occurred.
Patch work as expected. It was tested with: * trainling spaces in ignored files * without trailing spaces in git * with trailing spaces in C code.
It would be good to update either commit message or add comment to the code about this behaviour. So we would not wonder in future why there is 1.
LS
On 10/06/2015 09:10 AM, Lukas Slebodnik wrote:
On (05/10/15 16:49), Nikolai Kondrashov wrote:
On 10/05/2015 04:38 PM, Lukas Slebodnik wrote:
On (05/10/15 15:59), Nikolai Kondrashov wrote:
Hi everyone,
The attached patch fixes the issue of whitespace_test failing when no trailing whitespace was detected at all.
I do not understand the explanation.
Sorry, if I wasn't clear.
Trailing whitespaces are not detected with unchaged file po/ca.po either.
The trailing whitespace is there in the unchanged "po/ca.po" file. I.e. there *is* trailing whitespace in the source tree. However, we chose to ignore it. It is detected by "git grep" but suppressed by AWK, in whitespace_test.
With updated "po/ca.po" the trailing whitespace is removed. As a consequence, there is no more trailing whitespace in the whole source tree and "git grep" returns 1, as it hasn't found anything. That makes whitespace_test quit with status 1, failing the test.
The patch makes whitespace_test ignore exit status 1 from "git grep" (but still abort on other error codes), AWK happily processes empty output and all is fine.
I was not able to find anything about return codes in the man "git grep" but it seems that it's related to /usr/bin/grep
Yes, it is lacking in that regard.
man grep says: EXIT STATUS Normally the exit status is 0 if a line is selected, 1 if no lines were selected, and 2 if an error occurred. However, if the -q or --quiet or --silent is used and a line is selected, the exit status is 0 even if an error occurred.
"git grep" adds its own error codes too. E.g. it returns 128 if running outside a Git repo.
Patch work as expected. It was tested with:
- trainling spaces in ignored files
- without trailing spaces in git
- with trailing spaces in C code.
Thank you for testing and review, Lukas.
It would be good to update either commit message or add comment to the code about this behaviour. So we would not wonder in future why there is 1.
Sure. Would the attached patch do?
Nick
On (06/10/15 15:59), Nikolai Kondrashov wrote:
On 10/06/2015 09:10 AM, Lukas Slebodnik wrote:
On (05/10/15 16:49), Nikolai Kondrashov wrote:
On 10/05/2015 04:38 PM, Lukas Slebodnik wrote:
On (05/10/15 15:59), Nikolai Kondrashov wrote:
Hi everyone,
The attached patch fixes the issue of whitespace_test failing when no trailing whitespace was detected at all.
I do not understand the explanation.
Sorry, if I wasn't clear.
Trailing whitespaces are not detected with unchaged file po/ca.po either.
The trailing whitespace is there in the unchanged "po/ca.po" file. I.e. there *is* trailing whitespace in the source tree. However, we chose to ignore it. It is detected by "git grep" but suppressed by AWK, in whitespace_test.
With updated "po/ca.po" the trailing whitespace is removed. As a consequence, there is no more trailing whitespace in the whole source tree and "git grep" returns 1, as it hasn't found anything. That makes whitespace_test quit with status 1, failing the test.
The patch makes whitespace_test ignore exit status 1 from "git grep" (but still abort on other error codes), AWK happily processes empty output and all is fine.
I was not able to find anything about return codes in the man "git grep" but it seems that it's related to /usr/bin/grep
Yes, it is lacking in that regard.
man grep says: EXIT STATUS Normally the exit status is 0 if a line is selected, 1 if no lines were selected, and 2 if an error occurred. However, if the -q or --quiet or --silent is used and a line is selected, the exit status is 0 even if an error occurred.
"git grep" adds its own error codes too. E.g. it returns 128 if running outside a Git repo.
Patch work as expected. It was tested with:
- trainling spaces in ignored files
- without trailing spaces in git
- with trailing spaces in C code.
Thank you for testing and review, Lukas.
It would be good to update either commit message or add comment to the code about this behaviour. So we would not wonder in future why there is 1.
Sure. Would the attached patch do?
Nick
From 1b98c1305d3bb9e4e9fba717a39955193fe337de Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Mon, 5 Oct 2015 15:54:27 +0300 Subject: [PATCH] TESTS: Make whitespace_test pass without whitespace
Make whitespace_test pass if no trailing whitespace was detected at all. Add two comments explaining how searching and failure handling works.
ACK CI: http://sssd-ci.duckdns.org/logs/job/29/37/summary.html
There is still failed make check on rhel6 but it is unrelated to whitespace test.
LS
On (07/10/15 06:50), Lukas Slebodnik wrote:
On (06/10/15 15:59), Nikolai Kondrashov wrote:
On 10/06/2015 09:10 AM, Lukas Slebodnik wrote:
On (05/10/15 16:49), Nikolai Kondrashov wrote:
On 10/05/2015 04:38 PM, Lukas Slebodnik wrote:
On (05/10/15 15:59), Nikolai Kondrashov wrote:
Hi everyone,
The attached patch fixes the issue of whitespace_test failing when no trailing whitespace was detected at all.
I do not understand the explanation.
Sorry, if I wasn't clear.
Trailing whitespaces are not detected with unchaged file po/ca.po either.
The trailing whitespace is there in the unchanged "po/ca.po" file. I.e. there *is* trailing whitespace in the source tree. However, we chose to ignore it. It is detected by "git grep" but suppressed by AWK, in whitespace_test.
With updated "po/ca.po" the trailing whitespace is removed. As a consequence, there is no more trailing whitespace in the whole source tree and "git grep" returns 1, as it hasn't found anything. That makes whitespace_test quit with status 1, failing the test.
The patch makes whitespace_test ignore exit status 1 from "git grep" (but still abort on other error codes), AWK happily processes empty output and all is fine.
I was not able to find anything about return codes in the man "git grep" but it seems that it's related to /usr/bin/grep
Yes, it is lacking in that regard.
man grep says: EXIT STATUS Normally the exit status is 0 if a line is selected, 1 if no lines were selected, and 2 if an error occurred. However, if the -q or --quiet or --silent is used and a line is selected, the exit status is 0 even if an error occurred.
"git grep" adds its own error codes too. E.g. it returns 128 if running outside a Git repo.
Patch work as expected. It was tested with:
- trainling spaces in ignored files
- without trailing spaces in git
- with trailing spaces in C code.
Thank you for testing and review, Lukas.
It would be good to update either commit message or add comment to the code about this behaviour. So we would not wonder in future why there is 1.
Sure. Would the attached patch do?
Nick
From 1b98c1305d3bb9e4e9fba717a39955193fe337de Mon Sep 17 00:00:00 2001 From: Nikolai Kondrashov Nikolai.Kondrashov@redhat.com Date: Mon, 5 Oct 2015 15:54:27 +0300 Subject: [PATCH] TESTS: Make whitespace_test pass without whitespace
Make whitespace_test pass if no trailing whitespace was detected at all. Add two comments explaining how searching and failure handling works.
ACK CI: http://sssd-ci.duckdns.org/logs/job/29/37/summary.html
There is still failed make check on rhel6 but it is unrelated to whitespace test.
master: * d8899526551cbfe112e0ecc8280003a8349fc531
LS
sssd-devel@lists.fedorahosted.org
-
Lukas Slebodnik -
Nikolai Kondrashov