Skip to content

fix: remove bad .gitattributes#977

Merged
JeromeMartinez merged 1 commit intomipops:mainfrom
acuteaangle:bad_gitattributes
Nov 18, 2025
Merged

fix: remove bad .gitattributes#977
JeromeMartinez merged 1 commit intomipops:mainfrom
acuteaangle:bad_gitattributes

Conversation

@acuteaangle
Copy link
Copy Markdown
Contributor

@acuteaangle acuteaangle commented Mar 6, 2025

The current .gitattributes file marks all bash scripts as binary files, which prevents most git commands like git diff from working properly unless a --text argument is given, and entirely breaks the default merge strategy and conflict markers during manual merges.


Closes: gh-979

@dericed
Copy link
Copy Markdown
Contributor

dericed commented Mar 6, 2025

@ElderOrb, can you remind me why we did this?

@dericed
Copy link
Copy Markdown
Contributor

dericed commented Mar 6, 2025

IIRC this was a workaround for preserving EOL changes in a windows environment, but I don't think it's still needed.

@ElderOrb
Copy link
Copy Markdown
Collaborator

ElderOrb commented Mar 6, 2025

IIRC this was a workaround for preserving EOL changes in a windows environment, but I don't think it's still needed.

Right, it was done to avoid EOL auto-conversion on fetching changes into Windows-hosted git repo. It might be not needed or still needed, dependently on how Windows builds are produced. So makes sense to produce windows build from this PR and check if scripts are still functional

@acuteaangle
Copy link
Copy Markdown
Contributor Author

acuteaangle commented Mar 7, 2025

I believe .gitattributes also supports <pattern> text eol=lf to always checkout files with LF line endings, which may be closer to the intended result in this case.

acuteaangle added a commit to acuteaangle/dvrescue that referenced this pull request Mar 7, 2025
The current `.gitattributes` file marks all bash scripts as binary
files, which prevents most `git` commands like `git diff` from working
properly unless a `--text` argument is given, and entirely breaks
the default merge strategy and conflict markers during manual merges.

This was originally done to ensure bash scripts would keep their LF
line-endings on Windows; however, this is better accomplished by the
`text eol=lf` attributes instead. `text eol=lf` instructs git to always
checkout specified text files with LF line-endings, regardless of what
line-endings are considered 'native' on the current system.

This commit changes the `binary` attribute to `text eol=lf` for shell
scripts, continuing to ensure they are checked out with LF line-endings
on Windows, while also allowing `git` utilities like `git diff`, `git log
-p`, and merges to work as expected.

Closes: mipopsgh-977
@acuteaangle
Copy link
Copy Markdown
Contributor Author

I have opened an alternative PR to replace binary with text eol=lf.

I've marked both PRs as 'Closes:' each other, so if either is accepted the alternative will hopefully be cleaned up automatically.

The current `.gitattributes` file marks all bash scripts as binary
files, which prevents most `git` commands like `git diff` from working
properly unless a `--text` argument is given, and entirely breaks
the default merge strategy and conflict markers during manual merges.
@JeromeMartinez JeromeMartinez merged commit c158503 into mipops:main Nov 18, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants