Remove generated testbed/new.icc from the tree - #588
Closed
sallustfire wants to merge 1 commit into
Closed
Conversation
Running the test suite from an out-of-tree build of a read-only source tree segfaults at "Checking Check MetaTag": both out-of-tree flows copy the .icc corpus into the writable build directory preserving file modes (configure_file copy in meson, cp in the autotools VPATH check rule), and new.icc is the only "corpus" file a test opens for writing. fopen() of the read-only copy returns NULL and the following fwrite() dereferences it. It is the only file that can fail this way because it is not corpus at all: the Check MetaTag test reserializes ibm-t61.icc to memory, writes the blob to new.icc and reopens it to re-read the meta dictionary. It is written fresh on every run before it is ever read, so the tracked copy is never used as input, and unlike every other scratch profile the harness creates it is never removed afterwards. It is also absent from EXTRA_DIST, and being dirty after every run it has been swept into unrelated commits by accident several times (2022-06-05, 2023-01-03, 2024-03-11) - including into the meson iccs copy list when testbed runs moved to the build directory, where the stray file was indistinguishable from real test data. Delete the file, drop it from the meson icc copy list (configure_file fails on a missing input), and gitignore it so it is not re-committed. The testbed produces byte-identical output with the file absent, and the read-only out-of-tree run passes.
Owner
Author
|
Thanks for the quick fix. Agreed that removing it rather than ignoring it a better approach. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
I was hermetically building this and stumbled across testbed/new.icc, which is output of the test suite, not test data. The "Check MetaTag" test writes it fresh before ever reading it, and it's the only scratch profile the harness never cleans up with remove(). Being left behind after every run, it got committed by accident (a few times since 2022), and from there into the meson icc copy list, where it now breaks read-only checkouts:
configure_file copies the corpus into the build dir preserving file modes, so the copied new.icc is read-only; fopen("new.icc", "wb") returns NULL and the unchecked fwrite crashes. A VPATH make check does the same, since cp also preserves the mode. As far as I can tell, writable checkouts never see any of this. There the test just silently rewrites the tracked file, which is why CI stays green and the only visible symptom has been the perpetually dirty tree.
This deletes the file, drops it from the meson list (configure_file errors on a missing input), and gitignores it. Verified: test output is byte-identical with the file absent, and the read-only meson run above passes after the change. The file isn't in EXTRA_DIST, so make dist tarballs never shipped it.
The leftover could also be eliminated at the source by having CheckMeta remove("new.icc") like every other test does with its scratch file. Then, nothing would ever leave the file behind, gitignored or not. Happy to add that here if you'd prefer; I left it out to keep this a pure deletion.