Bug 253750 - sysutils/cpdup description is inaccurate
Summary: sysutils/cpdup description is inaccurate
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Guido Falsi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-22 00:24 UTC by dewayne
Modified: 2021-04-24 21:57 UTC (History)
4 users (show)

See Also:
madpilot: maintainer-feedback+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description dewayne 2021-02-22 00:24:55 UTC
Based on the desciption (& hope) I installed cpdup 1.22.  I created two folders and...
# echo hello > a/hi
# setfacl -m u:www:r a/hi.txt
# setextattr user t "a line" a/hi
# cpdup a b
# getextattr user t a/hi b/hi
a/hi    a line
getextattr: b/hi: failed: Attribute not found
# getfacl b/hi.txt
# file: b/hi.txt
# owner: root
# group: wheel
user::rwx
group::r--
other::---
(missing www acl)

I then searched the source for calls to acl_*, extattr_* and mac_*, there are no system calls included in the source tree.

The description of this port is inaccurate because it does not provide a comprehensive backup or mirror, due to its inability to copy ACL's, extended ACL's or MAC file attributes.  (MAC I can understand not doing to some extent, but acl's & extacl's should be included for the assertion to be true)

Would you please correct the description.  Suggest at least, the addition of:
"Note: This port does not copy (or mirror) FreeBSD ACLs, extended ACLs nor MAC labels."

Aside: For the interested:  rsync -aAX a/* b # does perform as expected.
Comment 1 Guido Falsi freebsd_committer 2021-04-23 14:46:37 UTC
Had a conversation with the maintainer who gave me instructions about me and asked me to commit the changes.

Will act on this shortly, please stand by.
Comment 2 commit-hook freebsd_committer 2021-04-23 17:01:44 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=f6f44b6cfe29d62a254a2deac5b7271fae1797cf

commit f6f44b6cfe29d62a254a2deac5b7271fae1797cf
Author:     Guido Falsi <madpilot@FreeBSD.org>
AuthorDate: 2021-04-23 17:00:31 +0000
Commit:     Guido Falsi <madpilot@FreeBSD.org>
CommitDate: 2021-04-23 17:01:28 +0000

    sysutils/cpdup: Update pkg-plist and WWW

    PR:             253750
    Approved by:    gmarco@giovannelli.it (maintainer)

 sysutils/cpdup/pkg-descr | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
Comment 3 Guido Falsi freebsd_committer 2021-04-23 17:02:18 UTC
Note added to pkg-descr.

Thanks!
Comment 4 Harald Schmalzbauer 2021-04-23 18:18:51 UTC
(In reply to commit-hook from comment #2)

Hi, I guess "Update pkg-plist and WWW" was meant to read "Update pkg-descr and WWW": No pkg-plist change reported by git.

And regarding the pkg-descr itself: "extended ACL's" is incorrect, as is FreeBSD ACLs.
The former is quiet sure just some kind of typo, meaning "Extended Attributes" (a.k.a EAs).
The latter is probably meant to read "NFSv4 ACLs'.
But there's posix ACL's too.
I guess cpudup(1) doesn't support anything else but POSIX permission bits.

I strongly suggest to either make sure to mention the distinct, single ACL type not supported, or change it to something like "any FreeBSD supported ACL types".
And of courde the EA correction.

Thanks!
-harry
Comment 5 Guido Falsi freebsd_committer 2021-04-23 18:26:51 UTC
(In reply to Harald Schmalzbauer from comment #4)

I committed what the maintainer asked me to commit.

I got distracted and meant pkg-descr, sorry for the lapsus.
Comment 6 Gianmarco Giovannelli 2021-04-24 10:27:10 UTC
I think the correct pkg-descr should be something like:



"This is the distribution of "cpdup", a powerful filesystem mirroring and
backup utility which also has file integrity testing features, supports
incremental backups, and remote mirroring via ssh.

Note: This port does not copy (or mirror) any FreeBSD supported ACL types and Extended Attributes"



I have written also to Matt Dillon (the original author) asking to him about this and him replied to me:


"I didn't add support for ACLs but cpdup uses a fairly robust extension mechanism to the protocol.
You can definitely add the feature if you would like.
ACLs are something we never threw into dragonfly so I didn't code it up at the time.

In anycase, if you would like to add support it shouldn't be too hard.  You would want to bump the protocol version and then implement the front-end and back-end messages to read and set the ACLs.   e.g. in hcproto.c there are various frontend functions like hc_stat() and backend functions like rc_stat().  There is also the dispatch table for the backend near the top.  You would use a protocol version test to check for remote support of the new messages and you would also want to handle error responses when the remote end supports the new messages but the target filesystem does not have ACLs.   The maximum message size is 65536 bytes, so if encoding all the ACLs in one message would exceed that you would have to break it up into multiple messages.

If the ACLs affect the destination end's ability to make adjustments to the file, you may have to be careful about how you order the messages.

Also conditionalize compilation if the OS doesn't have the system calls, e.g. #ifdef on something like ACL_MAX_ENTRIES, and conditionalize support for various tags and types and such.

If you (or anyone) want to do this work, I would be happy to incorporate it into the master copy of cpdup.  It would be nice if it supported ACLs.

-Matt"



So if someone wants to provide some patches we can test and try to implement them ... and then send to Matt.
If not the pkg-descr I have sent is enough correct.

Is it ok ?!


Thanks.
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2021-04-24 10:31:18 UTC
(In reply to Gianmarco Giovannelli from comment #6)

@Gianmarco Please open a new issue if you'd like to suggest additional changes, or contact the maintainer to suggest those changes.
Comment 8 Gianmarco Giovannelli 2021-04-24 11:02:27 UTC
I am the mantainer :)
Comment 9 Gianmarco Giovannelli 2021-04-24 11:08:29 UTC
I have written here because a lot of people were involved in this simple public request pkg-descr change. 

Probably these changes should not been written there too, but their best place are the cpdup man or in post install notes.

Btw the discussion was started and perhaps we can finish it if anyone involved agree in this:

"This is the distribution of "cpdup", a powerful filesystem mirroring and
backup utility which also has file integrity testing features, supports
incremental backups, and remote mirroring via ssh.

Note: This port does not copy (or mirror) any FreeBSD supported ACL types and Extended Attributes"

Thanks.
Comment 10 dewayne 2021-04-24 21:57:54 UTC
(In reply to Gianmarco Giovannelli from comment #9)
Thank-you Gianmarco.  I think that description will help people who use various ACL types to consider this tool appropriately.