Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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
Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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
signature.asc
Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
-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-
Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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 signature.asc Description: OpenPGP digital signature
Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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() {
>
Re: [pacman-dev] [PATCH] srcinfo.sh: remove trailing newline
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
signature.asc
Description: OpenPGP digital signature
[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
