Skip to content

Lint for filename case collisions and enforce lowercase on new assets#4145

Draft
dannon wants to merge 1 commit into
galaxyproject:mainfrom
dannon:lint-filename-case
Draft

Lint for filename case collisions and enforce lowercase on new assets#4145
dannon wants to merge 1 commit into
galaxyproject:mainfrom
dannon:lint-filename-case

Conversation

@dannon

@dannon dannon commented Jul 20, 2026

Copy link
Copy Markdown
Member

Follow-up to #4143. That was the second time a case-only filename collision (Keynote.jpg vs keynote.jpg) slipped in, so this adds a lint to catch the class of problem going forward.

check-filename-case.mjs joins the content:lint pipeline with two checks:

  1. Case-collision (always, repo-wide): fails if any two tracked paths are equal when lowercased. These collide on case-insensitive filesystems (macOS, Windows) -- only one file lands in the working tree, git reports a permanent phantom modification, and branch switches flip which one is present. The repo is clean today, so this passes.
  2. New-asset lowercase (forward-only): fails if a PR adds an image/media/doc asset with an uppercase basename. It only looks at files added relative to the PR base, so the ~1,600 existing uppercase files are untouched -- no mass rename. The content-lint job now fetches full history and passes the base SHA so the diff works.

Includes unit tests and a CONTRIBUTING note. I deliberately didn't try to enforce lowercase on all existing files -- that's a much bigger change and a separate discussion if we want it.

Adds check-filename-case.mjs to the content:lint pipeline. It fails on any
two tracked paths that differ only in case -- the Keynote.jpg/keynote.jpg
problem that's bitten us twice, which collides on macOS and Windows and
leaves git reporting a phantom modification. It also fails on newly added
image/media assets with uppercase names, while leaving the ~1600 existing
uppercase files alone (the new-asset check only looks at files added
relative to the PR base). Because that diff needs the base, the content-lint
job now fetches full history and passes the base SHA. Includes unit tests
and a CONTRIBUTING note.

@bgruening bgruening left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@itisAliRH itisAliRH left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed early since it's a draft; nice idea, and grandfathering old files is the right call. Two spots where the lint passes when it shouldn't:

Renames slip past. listAddedFiles uses git diff --diff-filter=A, and git detects renames by default, so git mv keynote.jpg Keynote.jpg shows up as R, not A. That's a likely way this bug comes back.

git(['diff', '--no-renames', '--diff-filter=A', '--name-only', '-z', base])

The CLI guard can quietly do nothing.

if (import.meta.url === `file://${process.argv[1]}`) {

import.meta.url is percent-encoded, so if the checkout path has a space or a non-ASCII character, this is false, main() never runs, and the script exits 0 with no output. CI goes green having checked nothing.

import { pathToFileURL } from 'url';
if (import.meta.url === pathToFileURL(process.argv[1]).href) {

Smaller notes:

  • base.sha is the base tip from the last sync, not the merge-base. An uppercase file merged to main after that point can get blamed on this PR. Rare, but confusing.
  • fetch-depth: 0 pulls every image blob on each lint run. A shallow checkout plus git fetch --depth=1 origin <base.sha> gets the same thing cheaper.
  • /[A-Z]/ misses accented capitals like Ä.jpg; /\p{Lu}/u if you want it tight.

Rest looks good — exit codes, messages, and helper tests all line up with the other lints.

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.

3 participants