Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

We have zero-line text files that aren't empty #9802

Closed
philderbeast opened this issue Mar 13, 2024 · 7 comments · Fixed by #9804
Closed

We have zero-line text files that aren't empty #9802

philderbeast opened this issue Mar 13, 2024 · 7 comments · Fixed by #9804

Comments

@philderbeast
Copy link
Collaborator

philderbeast commented Mar 13, 2024

Describe the bug
Investigating #9798, I wanted to find the number of single line project files. In doing this I found some were reported as having 0 lines.

On review, @geekosaur is catching the mistake I'm often making of dropping or forgetting these newlines.

Therefore, “lines” not ending in a newline character aren't considered actual lines. That's why some programs have problems processing the last line of a file if it isn't newline terminated.

The advantage of following this convention is that all POSIX tools expect and use it.
SOURCE: Why should text files end with a newline?

I have a script to add newlines when missing and can use it to fix up some files that don't conform, for instance *.hs and *.project files. I didn't find any non-conforming *.cabal files. There are only two legitimate zero line (empty) projects;

  • ./cabal-install/tests/fixtures/project-root/cabal.project
  • ./cabal-install/tests/fixtures/project-root/nix/cabal.project

To Reproduce

$ find -name '*.project' -exec wc -l {} \; | sort -k 1
0 ./cabal-install/tests/fixtures/project-root/cabal.project
0 ./cabal-install/tests/fixtures/project-root/nix/cabal.project
0 ./cabal-testsuite/PackageTests/ExtraProgPath/cabal.project
0 ./cabal-testsuite/PackageTests/HaddockArgs/cabal.project
0 ./cabal-testsuite/PackageTests/HaddockWarn/cabal.project
0 ./cabal-testsuite/PackageTests/NewBuild/CmdRun/Script/cabal.project
0 ./cabal-testsuite/PackageTests/NewBuild/CmdRun/ScriptLiterate/cabal.project
0 ./cabal-testsuite/PackageTests/NewHaddock/Fails/cabal.project
0 ./cabal-testsuite/PackageTests/NewSdist/DeterministicTrivial/cabal.project
0 ./cabal-testsuite/PackageTests/PkgConfigParse/cabal.project
0 ./cabal-testsuite/PackageTests/Regression/T5782Diamond/cabal.project
0 ./cabal-testsuite/PackageTests/SDist/Respect-Project-File/cabal.project
0 ./cabal-testsuite/PackageTests/SDist/T5195and5349/cabal.project
0 ./cabal-testsuite/PackageTests/WarnEarlyOverwrite/cabal.project
...

$ cat ./cabal-testsuite/PackageTests/WarnEarlyOverwrite/cabal.project
packages: .⏎

Expected behavior
We add the missing newlines (I have a script for this).

#!/bin/bash
# SEE: https://unix.stackexchange.com/questions/31947/how-to-add-a-newline-to-the-end-of-a-file
for f in $(git grep -Il '' -- "$1"); do
    echo "$f";
    tail -c1 < "$f" | read -r _ || echo >> "$f";
done

Stretch goal: We stop relying on @geekosaur spotting these mistakes and add a check for this in CI.

@ffaf1
Copy link
Collaborator

ffaf1 commented Mar 13, 2024

Mhhh I thought we already did it re: automating this bit of geekosaur

# This file contains the project-specific settings for `fix-whitespace` a tiny
# but useful tool to
#
# * Removes trailing whitespace.
# * Removes trailing lines containing nothing but whitespace.
# * Ensures that the file ends in a newline character.

@philderbeast
Copy link
Collaborator Author

philderbeast commented Mar 14, 2024

Looking at the blame for fix-whitespace.yaml, I see it was added along with .github/workflows/whitespace.yml in #8320. That workflow no longer exists and I can't find another one that still runs this check, that runs fix-whitespace --check.

@philderbeast
Copy link
Collaborator Author

With fix-whitespace we have these violations;

$ fix-whitespace --version
0.1

$ fix-whitespace --check
[ Violation detected ] .github/workflows/bootstrap.skip.yml
[ Violation detected ] .github/workflows/validate.skip.yml
[ Violation detected ] .github/workflows/validate.yml
[ Violation detected ] CONTRIBUTING.md
[ Violation detected ] Cabal-described/src/Distribution/Described.hs
[ Violation detected ] Cabal-described/src/Distribution/Utils/CharSet.hs
[ Violation detected ] Cabal-tests/tests/UnitTests/Distribution/Simple/Program/GHC.hs
[ Violation detected ] README.md
[ Violation detected ] bootstrap/src/Main.hs
[ Violation detected ] cabal-install/tests/IntegrationTests2/.gitignore
[ Violation detected ] cabal-install-solver/src/Distribution/Solver/Modular/Message.hs
[ Violation detected ] cabal-testsuite/PackageTests/Backpack/Includes2/setup-per-component.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/CmmSourcesExe/cmmexperiment.cabal
[ Violation detected ] cabal-testsuite/PackageTests/ExternalCommandSetup/setup.cabal.hs
[ Violation detected ] cabal-testsuite/PackageTests/NewBuild/CmdRepl/CustomSetup/foo/Setup.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.dot-uv.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.dot-uv.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.ignore-project.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.ignore-project.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.no-project.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.no-project.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.sub-pq.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.sub-pq.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.sub-rs.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-No/cabal.sub-rs.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.dot-uv.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.dot-uv.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.ignore-project.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.ignore-project.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.no-project.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.no-project.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.project.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.project.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.sub-pq.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.sub-pq.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.sub-rs.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/Projects-Default-Yes/cabal.sub-rs.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/README.md
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/cabal.ignore-project.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/cabal.ignore-project.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/cabal.no-project.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/SDist/Respect-Project-File/cabal.no-project.v2.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/VersionPriority/0-local.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/VersionPriority/1-local.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/VersionPriority/2-local.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/WarnEarlyOverwrite/clean-install-by-copy.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/WarnEarlyOverwrite/clean-install-by-symlink.test.hs
[ Violation detected ] cabal-testsuite/PackageTests/WarnEarlyOverwrite/dirty-install.test.hs
[ Violation detected ] doc/buildinfo-fields-reference.rst
[ Violation detected ] doc/cabal-commands.rst
[ Violation detected ] doc/getting-started.rst
[ Violation detected ] fourmolu.yaml

@philderbeast
Copy link
Collaborator Author

The configuration doesn't include cabal.project files;

- "cabal.project.*"

@philderbeast
Copy link
Collaborator Author

I find our naming for project files odd in that it doesn't have a consistent file extension, .project. Elsewhere I've used cabal.something.project rather than what we use here, cabal.project.something.

@philderbeast
Copy link
Collaborator Author

philderbeast commented Mar 14, 2024

@Kleidukos was there some bad interaction between fourmolu and fix-whitespace? The former doesn't cover all the file types of the later so perhaps we could revive some of fix-whitespace, deleted in 827ee18.

$ git log -p -- .github/workflows/whitespace.yml
commit 827ee18e0864d9774eb2f4d687e2eca69a61d478
Date:   Wed May 24 09:34:10 2023 +0200

    Formatting configuration

diff --git a/.github/workflows/whitespace.yml b/.github/workflows/whitespace.yml
deleted file mode 100644
index 1254f7ccc..000000000

@ulysses4ever
Copy link
Collaborator

I think we should put back the action running fix-whitespace and revise the tool's config to include more files and to print explanation for violations (it's a new flag in fix-whitespace), and maybe something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants