Let journey tests fail without find and enforce LF line endings
#1676
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adjustes `.gitattributes` to normalize line endings to LF (Unix style line endings) for all shell scripts in the project, not just shell scripts that are fixture scripts. Previously, shell scripts related to journey tests were not normalized this way. This is simpler to write, and `bash` and POSIX shell scripts are required to have LF line endings to be correct in general. On Windows, some `bash` builds (including the MSYS2 `bash` provided as part of the Git Bash environment) are patched to automatically accept Windows-style line endings, but I believe they are still considered technically incorrect. In practice, the effect of not using LF line endings when working with shell scripts on Windows may usually be where other tools are involved: ShellCheck, and at least some language servers for `bash` script editing, complain if CRLF line endings are used.
This removes all uses of `with_program`, which is no longer used with any command other than `find`. The rationale is: - `find` is a standard command. Minimal systems may omit it, but they omit numerous other software that is needed for building and testing gitoxide. `find` is standardized and required by POSIX and is present on all common general-purpose installations of Unix-like operating systems. It is also present in the Git Bash environment (and most other MSYS2 environments) that may be used for running the test suite on Windows. - One purpose of `with_program` is to check for missing commands where, if not checked for, then tests might run but wrongly pass due to assertions that commands fail (where they should fail for a different reason). However, that does not seem to apply to any `with_program find`: all assertions with `find` in code to which that had been applied are using `expect_run_sh $SUCCESSFULLY`. `with_program` is typically applied in a `(` `)` subshell, which itself can have an important effect in isolating changes made within it. In this case, however, the code was either still enclosed equivalently, or appeared at the end of a larger `(` `)` scope. So this removes `(` `)` subshells that only existed to facilitate the deleted `with_program` uses and dedents.
This removes the journey test helper function `with_program`, which now has zero uses. (This is done in its own commit to make it easy to revert or examine later if a use case for it ever arises in the future.)
Byron
approved these changes
Nov 14, 2024
EliahKagan
added a commit
to EliahKagan/gitoxide
that referenced
this pull request
Nov 18, 2024
In c957ab8 (GitoxideLabs#1340), I intended to mark the contents of `gix-packetline-blocking/src` as auto-generated for github-linguist, but I did not do so correctly, because giving the directory as a path does not specify the `linguist-generated` attribute as being unset for the contents. This fixes that. The bug, and fix, is revealed by comparing the output of `github-linguist --breakdown` before and after this: --- a 2024-11-18 15:45:57.376285107 -0500 +++ b 2024-11-18 15:46:44.808328857 -0500 @@ -1,6 +1,6 @@ -94.14% Rust -4.58% Shell -1.19% HTML +94.08% Rust +4.63% Shell +1.20% HTML 0.09% Makefile Makefile: @@ -682,23 +682,6 @@ gix-pack/tests/pack/multi_index/mod.rs gix-pack/tests/pack/multi_index/verify.rs gix-pack/tests/pack/multi_index/write.rs -gix-packetline-blocking/src/decode.rs -gix-packetline-blocking/src/encode/async_io.rs -gix-packetline-blocking/src/encode/blocking_io.rs -gix-packetline-blocking/src/encode/mod.rs -gix-packetline-blocking/src/lib.rs -gix-packetline-blocking/src/line/async_io.rs -gix-packetline-blocking/src/line/blocking_io.rs -gix-packetline-blocking/src/line/mod.rs -gix-packetline-blocking/src/read/async_io.rs -gix-packetline-blocking/src/read/blocking_io.rs -gix-packetline-blocking/src/read/mod.rs -gix-packetline-blocking/src/read/sidebands/async_io.rs -gix-packetline-blocking/src/read/sidebands/blocking_io.rs -gix-packetline-blocking/src/read/sidebands/mod.rs -gix-packetline-blocking/src/write/async_io.rs -gix-packetline-blocking/src/write/blocking_io.rs -gix-packetline-blocking/src/write/mod.rs gix-packetline/src/decode.rs gix-packetline/src/encode/async_io.rs gix-packetline/src/encode/blocking_io.rs (The test was done on Ubuntu 18.04 LTS ESM with `github-linguist` provided by the `ruby-github-linguist` package, version 5.3.3-1.) In addition to fixing that, this also simplifies `.gitattributes` files throughout the repository. This simplification includes removing redundant `clrf=input` (discussed in GitoxideLabs#1676) and a spurious unrecognized `-eof` (see GitoxideLabs#1689), changing lone `**` to `*` where equivalent, and some other changes. This also makes the style of `.gitattributes` files more consistent, using `foo`/`-foo` rather than `foo=true`/`foo=false` everywhere even where both work, and using whitespace more consistently.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
f1a171b (#1674) included the conservative step of correcting the last remaining
with_program treetowith_program findand no longer installingtreeon CI. However, we can go further: any environment withoutfindis unsuitable for running journey tests (and, more broadly, broken for general purpose usage).findis required by POSIX, is in practice present on all but the most minimal Unix-like systems (and installable on those), and is present in Git Bash.with_programskips tests when the named tool is not found, so using it does not prevent bugs from being concealed. More importantly, all the assertions covered underwith_progam findwereexpect_run_sh $SUCCESSFULLYassertions, so iffindwere absent, they would fail, revealing the problem. I think that it is better for journey tests that usefindto fail, when run in strange environments that don't havefind.This removes all such calls in bc6e4be. As detailed in that commit message, even with scoping considerations in mind, the subshells introduced with
with_findcalls are no longer needed either, so this removes those and dedents. (Thus, the changes here are significantly less than a +/- line count would suggest.) These were also the only remaining calls towith_program, so 05e176c removes that function definition.While beginning to work on this, I noticed that shell scripts other than those related to fixtures don't have LF line endings enforced in
.gitattributesand would end up being checked out with CRLF line endings on Windows. This is undesirable even ifbashon Windows typically tolerates it, as detailed in 1a3f4e3, which therefore simplifies that part of.gitattributesto force LF for all shell scripts.