-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Library cannot generate manifest entries for or validate bags with hidden (.
) payload files
#51
Comments
@ssciolla Thank you for reporting this! I agree with your reasoning and a PR would be awesome. Maybe we could add some metadata indicating that the bag has been created by a new version of the library in bag-info.txt and change the validation operation if it is the newer version? Happy to keep discussing this with you. |
Thanks for the response, @little9! I will start working on a PR. Your idea about keying off the version could work. My colleague also found a discussion in bagit-java about hidden files from years ago. I think this comment is interesting: LibraryOfCongress/bagit-java#31 (comment) We could play it safe for the immediate future by adding an option to |
Add optional hidden file detection (#51)
Hi, we seem to have discovered today when validating a bag generated by another BagIt implementation (bagit-python) that hidden files -- or those starting with
.
, such as.keep
-- are not handled by this library. Essentially, the bag included hidden files in the manifest and payload, but the library didn't find them, resulting in a failed completeness check. I believe the following line is the source of the issue. Because it's also used by themanifest!
method, hidden files would also not be included in the manifest when the bag is generated by the library.bagit/lib/bagit/bag.rb
Lines 39 to 41 in 4a7fb6d
A fix would likely involve using
Dir.glob
with a specific pattern that would include these; the Ruby docs suggest using'{*,.*}'
, so for this library we might use'**/{*,.*}'
.Dir[]
might also still work with the updated pattern. There is aFile::FNM_DOTMATCH
flag, but it would also cause.
and..
to be included (which we don't want).In reviewing the spec, I don't see any indication that these files should be ignored or not included. If people seem to agree this is an issue, I'm happy to open a PR with some added/updated tests. If I'm incorrect somewhere in this analysis, I'm also happy to be pointed in the right direction.
Resources
Update: In thinking more about this, this may be considered a breaking change for current users of the library, as the bags they previously generated using the library would now be considered invalid by the "fixed" version. That would have to be handled somehow, assuming this change is a wanted improvement.
The text was updated successfully, but these errors were encountered: