Skip to content

Fix test_eval.py committing no files if repo checkout is named crisp - #180

Open
fw-immunant wants to merge 2 commits into
mainfrom
fw/test_eval-crisp-glob
Open

Fix test_eval.py committing no files if repo checkout is named crisp#180
fw-immunant wants to merge 2 commits into
mainfrom
fw/test_eval-crisp-glob

Conversation

@fw-immunant

Copy link
Copy Markdown
Collaborator

I couldn't figure out why I had so much trouble getting the test_eval.py script to work locally. It turns out that it was passing a gitignore glob of crisp*, which included my whole checkout of the repo and all subdirectories, including the Test-Corpus directory and children.

Also raise an error explicitly if we try to crisp commit but commit no files.

I don't think this is ever the user's intent when using `crisp commit`
this ignores all files in paths starting with `crisp`, which leads to no files being committed if you happen to name your crisp-related directories "crisp" instead of Tractor-Crisp
@spernsteiner

Copy link
Copy Markdown
Contributor

The error on empty commit seems like a good idea.

Can you say more about what's going on with the crisp* filter? It sounds like it's looking at an absolute path like /home/user/crisp/Test-Corpus/B01/colourblind/test_case/src/main.c., seeing a segment in the middle that matches crisp*, and filtering it out? Maybe it should be taking the path of the file relative to the example directory, such as ./test_case/src/main.c, and comparing that against the filter instead. This would make it behave more like actual .gitignore files, where the filter paths are interpreted relative to the directory containing the .gitignore. Then we could keep the crisp* rule (which covers all the crisp-storage-20260501, crisp.toml.bad, and other junk I tend to accumulate during development) and have less risk of similar surprises in the future.

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.

2 participants