FreeBSD has quite unfortunate behaviour when dealing with keywords that
are missing in one of the manifests being compared -- namely, they
ignore these instances.
Commit 21723a3974 ("*: fix comparison of missing keywords") re-added
this behaviour after the introduction of the Compare API, but
unfortunately it was implemented in the Compare API itself -- meaning
that library users (which didn't want this behaviour) were silently
opted into it.
This patch moves the behaviour to the command-line, where it belongs
(a future patch in this series will allow users to opt-out of this
unfortunate behaviour, as well as some other unfortunate FreeBSD
compatibility behaviours).
Fixes: 21723a3974 ("*: fix comparison of missing keywords")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This is fairly similar to the compareEntry modernisation, but there was
no need to use maps.Insert or maps.Collect since we want to pre-allocate
the map sizes anyway.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
The previous logic was overly complicated and can be simplified a lot
(especially now that we have helpful generic stdlib packages for dealing
with maps).
There are still several bugs in this, but the intention of this patch is
to just modernise the code without making any functional changes.
Bugfixes will come later.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
At the moment, filtering out keyword changes from an InodeDelta after
doing Compare is a little complicated and error-prone. The simplest
solution is to allow for callers to access a pointer to the underlying
slice so it can be modified properly.
The filtering logic in the gomtree command-line implicitly depends on
the behaviour of InodeDelta.Diff -- DiffPtr would be a far more
appropriate replacement.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Rather than parsing the value as a float and then truncating it, just
parse it as an integer in the first place (this also adds some
validation that we are parsing a reasonable-looking value).
While we're at it, add some integration tests for this code to make sure
this quite complicated special-case behaviour doesn't regress.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
In certain circumstances, if a manifest with "time" keywords was
compared to another manifest with "tar_time" keywords, differences were
not actually reported despite there being logic for it.
Unfortunately, even though commit 26ff922da6 ("compare: implement
mtree.DirectoryHierarchy comparisons") explicitly included logic to
handle the necessary "time" -> "tar_time" conversions, the same commit
also made it so that Compare() could be instructed to only consider a
subset of keywords and failed to take into account said "time" ->
"tar_time" remapping.
Most users have likely not noticed this because gomtree will re-use the
keywords from a manifest when doing a straightforward "gomtree validate"
invocation, but if you explicitly requested "time" when validating a
"tar_time" manifest (and *not* the other way around) then any time
changes would be undetected.
Fixes: 26ff922da6 ("compare: implement mtree.DirectoryHierarchy comparisons")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
I have a use case where I'd like to know the files that are the same in the
tree, as well as the differences.
I could do this with a separate walk and excluding the paths that were
different, but since mtree is already doing all of this for me, it makes
sense to include it here. I've added a new function so that the behavior
stays the same for existing users of Compare(), since I assume mostly this
will be slower given that most files stay the same. I'd be happy to merge
it into one, though.
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
The core issue comes about when you consider a trivial example of a path
like "./README". This path is lexically equivalent within mtree to
"README", but a simple string comparison will yield the wrong result.
Instead you need to lexically clean the path first (filepath.Clean isn't
enough here -- you need to prepend a "/" and then do filepath.Clean).
In gomtree we generate spec files in the same style of FreeBSD's
mtree(8), so you would be very hard-pressed to find an example of such
an inconsistency. However casync's mtree implementation does not
generate leading "./" for root paths which results in "missing" entries.
The implementation of CleanPath was written by me for umoci originally,
then later I copied it to runc for other uses, and now I've copied it
here. Since I'm the sole author I'm effectively dual-licensing it under
this project's license to avoid having to relicense go-mtree for no good
reason (or deal with the multiple-license hassle).
Signed-off-by: Aleksa Sarai <asarai@suse.de>
This allows people to create synthetic InodeDeltas, which is something
that umoci would like to be able to do in order to nicely create 'umoci
insert' layers.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
During the rework of how xattr fields are handled, the comparison code
was not correctly updated. As a result, changes to xattrs would not be
detected in any form. This was detected in the umoci integration suite.
In addition, fix the dh.UsedKeywords logic so auto-detection works
correctly with prefix-based xattrs.
Fixes: ed464af779 ("*: xattr can Update()")
Signed-off-by: Aleksa Sarai <asarai@suse.de>
This is a gnarly patchset that has been mashed together.
It uncovered that some aspects of Check were never really working
correctly for `xattr` keywords, but also the `Update()` had been left
undone for a while.
This includes some API changes around the `Keyword` and `KeyVal` types.
Also I would like to update the signature for the `UpdateKeywordFunc` to
just accept a `KeyVal` as an argugment, rather than a keyword AND the
value. with this context there would be no need to guess on the value of
what's passed to the xattr update function of whether it needs or
already is base64 encoded.
Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
KeyVal specific functions can be a part of the struct.
Also add tests and fix the NewValue functions for suffixes
Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
This allows for restoring some attributes of files from the state in an
mtree Manifest
Reported-by: Matthew Garrett <Matthewgarrett@google.com>
Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
Because of how xattr works (it will not be set on all files, but it's
possible for it to be added to a file without changing any other key)
it's necessary that we _always_ compute a diff when we hit an inode that
has xattr keys set.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Adding another test validated from the FreeBSD workflow.
Just because the keywords requested to be validated are not present in
the manifest, it is not an error.
Also, if the keywords from a new manifest are not present in a prior
manifest, then only compare the common keywords.
Fixes https://github.com/vbatts/go-mtree/issues/86
Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
This is part of a patchset that refactors all of the checking logic into
comparison operations. Essentially, provide a Compare(...) function that
allows for two different manifests to be compared. Extra and missing
entries are supported in addition to the standard modified entry, and by
implementing as a manifest comparison there is no double-scanning of the
manifest source.
The main annoyance is that we have to also include tar_time handling,
which has not been abstracted inside keywords.go. This is a bit ugly
right now, but works fine for the moment.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
