[pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. When running `git diff --check`, Git will complain about this as a whitespace error, saying "new blank line at EOF." Remove the empty line after sections. Replace the empty echo with a placeholder `true` call in case in the future, we do want to close the section with something. --- scripts/libmakepkg/srcinfo.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 6e783279..e7b5c4be 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -31,7 +31,7 @@ srcinfo_open_section() { } srcinfo_close_section() { - echo + true # nothing to be done } srcinfo_write_attr() { -- 2.27.0.rc1.69.g3229fa6e15
On 6/3/20 7:26 PM, Denton Liu wrote:
When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. When running `git diff --check`, Git will complain about this as a whitespace error, saying "new blank line at EOF."
git diff --check isn't necessarily our problem, though... But it will only be reported once, the very first time you ever commit this .SRCINFO
Remove the empty line after sections. Replace the empty echo with a placeholder `true` call in case in the future, we do want to close the section with something.
Now you've also removed the blank line in the middle of the file, separating the pkgbase section from each pkgname section. That is more than just the blank line terminating the final pkgname section.
--- scripts/libmakepkg/srcinfo.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 6e783279..e7b5c4be 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -31,7 +31,7 @@ srcinfo_open_section() { }
srcinfo_close_section() { - echo + true # nothing to be done }
srcinfo_write_attr() {
-- Eli Schwartz Bug Wrangler and Trusted User
On 6/3/20 8:04 PM, Eli Schwartz wrote:
On 6/3/20 7:26 PM, Denton Liu wrote:
When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. When running `git diff --check`, Git will complain about this as a whitespace error, saying "new blank line at EOF."
git diff --check isn't necessarily our problem, though...
But it will only be reported once, the very first time you ever commit this .SRCINFO
Also FWIW, I use git diff --check in my pre-commit githook for AUR packages, but I also generate the .SRCINFO in that very hook, AFTER I check for whitespace issues. Hence I don't see whitespace issues for the .SRCINFO file, even for the very first commit. So I'm really not motivated to change this. For more details see https://github.com/eli-schwartz/aurpublish/#hooks -- Eli Schwartz Bug Wrangler and Trusted User
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Hi, On Wed, 3 Jun 2020, Eli Schwartz wrote:
On 6/3/20 8:04 PM, Eli Schwartz wrote:
On 6/3/20 7:26 PM, Denton Liu wrote:
When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. When running `git diff --check`, Git will complain about this as a whitespace error, saying "new blank line at EOF."
git diff --check isn't necessarily our problem, though...
But it will only be reported once, the very first time you ever commit this .SRCINFO
Also FWIW, I use git diff --check in my pre-commit githook for AUR packages, but I also generate the .SRCINFO in that very hook, AFTER I check for whitespace issues. Hence I don't see whitespace issues for the .SRCINFO file, even for the very first commit. So I'm really not motivated to change this.
In case my voice counts anything in this discussion: I would prefer the empty lines between the sections and at the end to stay. I regularly parse .SRCINFOs (or the output of `makepkg --printsrcinfo`) and terminate parsing of sections on the empty line (think along the lines `sed '/^\S/,/^$/ {...}'`).
For more details see https://github.com/eli-schwartz/aurpublish/#hooks
-- Eli Schwartz Bug Wrangler and Trusted User
regards, Erich -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEE3p92iMrPBP64GmxZCu7JB1Xae1oFAl7Yf/QACgkQCu7JB1Xa e1oQuhAArWd0GHhWTk6F4bNS+Vw6PFR7UyJmgUzeipnwBFpXhntUwv4SiygGCaY4 XQFcEByjlhMz6xfL7t2ZfBfxpudH0TlypFdHUV9H2ujgJHi82+1XM13H9R/KMY56 I+YLO5iahPCVu/54licMWiSeb8Vu9ZBOgK5M4bm2YPfcADFaS0FiqZ8bwp3LCe3y JLiXZ3NOJj4U94OLXA6K1gMUkbn1ZX4TExS+qLGPqLo7W0rPCfZo+FyushNHGjv1 MUUKzDVgXMF06pgN1vCkHWuEmV9HKPibCD1265jbHcgXACzl9qv8wB/AJx21oN6G UgfzkTxu4LfaoVHsGA0be2vHHcdYYDbQLYJNEVm5DfNj7zeYwp1GFsGIarLz6BQT FnTpXkni2RFNvVJskDnEAXBsgv0u1Ml5CBJegrPBPhuaFXZ8sk9HVwkb+26DtSQr /5NyouwErASPDU2kDWnXJ8coJGfqGTmTzAAg1Y2S9/1/uVC9xx9Trya83MBxJ7eF 0bIDWngQNtCuEceEugXtM8W4DqkEFGrWGWiNwX632XhqPmLDuEZBz9HppgNYioHK ae+rklION7jOCHgu+kHH0DQKwL/V6QViLtL8FYn6M0hHe6ncjxUxta+3FYpmdnTR Xr98++0JRG9FtWIVOPCGnGU0Qfpig8IJk+dYYv0FNhKvahGn5q4= =vQdo -----END PGP SIGNATURE-----
On 6/4/20 1:00 AM, Erich Eckner wrote:
In case my voice counts anything in this discussion: I would prefer the empty lines between the sections and at the end to stay. I regularly parse .SRCINFOs (or the output of `makepkg --printsrcinfo`) and terminate parsing of sections on the empty line (think along the lines `sed '/^\S/,/^$/ {...}'`).
I should probably clarify that the empty lines as section separators are part of the current format specification and as far as I'm concerned, removing them is simply not up for discussion. Any proposed patch *must* retain those empty lines. Your use case is a good example of why. And I have no reason to think you're the only person in the entire userbase who is parsing srcinfo content with this assumption. Removing the *final* trailing newline could in theory be done (any srcinfo parser would presumably terminate at EOF anyway) and that, I'm "merely unconvinced" about implementing. I still don't think a compelling case has been made for it, but I'm willing to be convinced. -- Eli Schwartz Bug Wrangler and Trusted User
Hi Eli, Sorry for the late reply. On Wed, Jun 03, 2020 at 08:04:59PM -0400, Eli Schwartz wrote:
On 6/3/20 7:26 PM, Denton Liu wrote:
When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. When running `git diff --check`, Git will complain about this as a whitespace error, saying "new blank line at EOF."
git diff --check isn't necessarily our problem, though...
But it will only be reported once, the very first time you ever commit this .SRCINFO
You are correct that it's only reported once. I guess my problem isn't with `git diff --check` in particular but my problem is that the tool is generating files that has a whitespace error. (I consider a blank line at EOF to be an instance of trailing whitespace which should be considered a whitespace error.)
Remove the empty line after sections. Replace the empty echo with a placeholder `true` call in case in the future, we do want to close the section with something.
Now you've also removed the blank line in the middle of the file, separating the pkgbase section from each pkgname section. That is more than just the blank line terminating the final pkgname section.
This was intentional. I assumed that the blank line separators were merely aesthetic since in the wiki, it says "Blank lines [...] are also ignored." Since the .SRCINFO is supposed to be machine-parsed, I assumed that the aesthetics didn't really matter and we could follow an approach similar to how .gitconfig's are laid out with sections squashed together without any blank lines in between. Anyway, this is a moot point since downthread, Erich mentioned that he uses the blank lines in his parsing script and I'd hate to break backwards compatibility. If you accept the above justification of a trailing newline at EOF being a whitespace error, my next reroll of the patch will only remove the _final_ blank line, not the ones in between. Thanks for the feedback, Denton
On 4/6/20 9:26 am, Denton Liu wrote:
When the .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. When running `git diff --check`, Git will complain about this as a whitespace error, saying "new blank line at EOF."
Remove the empty line after sections. Replace the empty echo with a placeholder `true` call in case in the future, we do want to close the section with something. --- scripts/libmakepkg/srcinfo.sh.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 6e783279..e7b5c4be 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -31,7 +31,7 @@ srcinfo_open_section() { }
srcinfo_close_section() { - echo + true # nothing to be done }
This is a very strange approach to implementing your idea. Now the function is useless and should have been removed completely. And now there is zero separate between sections - not just the last line was removed. Anyway, the justification for this patch is missing. Why does makepkg care about a minor warning from "git diff --check"?
srcinfo_write_attr() {
When a .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. This is considered a trailing whitespace error. In fact, `git diff --check` will about this saying "new blank line at EOF." Instead of closing each section off with an empty line, use the empty line to separate sections, omitting the empty line at the end of the file. --- scripts/libmakepkg/srcinfo.sh.in | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/scripts/libmakepkg/srcinfo.sh.in b/scripts/libmakepkg/srcinfo.sh.in index 6e783279..d1e39f7d 100644 --- a/scripts/libmakepkg/srcinfo.sh.in +++ b/scripts/libmakepkg/srcinfo.sh.in @@ -30,7 +30,7 @@ srcinfo_open_section() { printf '%s = %s\n' "$1" "$2" } -srcinfo_close_section() { +srcinfo_separate_section() { echo } @@ -94,7 +94,6 @@ srcinfo_write_global() { srcinfo_open_section 'pkgbase' "${pkgbase:-$pkgname}" srcinfo_write_section_details '' - srcinfo_close_section } srcinfo_write_package() { @@ -104,7 +103,6 @@ srcinfo_write_package() { srcinfo_open_section 'pkgname' "$1" srcinfo_write_section_details "$1" - srcinfo_close_section } write_srcinfo_header() { @@ -118,6 +116,7 @@ write_srcinfo_content() { srcinfo_write_global for pkg in "${pkgname[@]}"; do + srcinfo_separate_section srcinfo_write_package "$pkg" done } -- 2.27.0.107.g134631ef42
Sorry, small typo: On Wed, Jun 10, 2020 at 10:14:07PM -0400, Denton Liu wrote:
When a .SRCINFO file is generated via `makepkg --printsrcinfo`, each section is concluded with an empty line. This means that at the end of the file, an empty line remains. This is considered a trailing whitespace error. In fact, `git diff --check` will about this saying
s/will/will warn/
"new blank line at EOF."
Instead of closing each section off with an empty line, use the empty line to separate sections, omitting the empty line at the end of the file.
participants (4)
-
Allan McRae
-
Denton Liu
-
Eli Schwartz
-
Erich Eckner