all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Filip Schauer <f.schauer@proxmox.com>
To: Wolfgang Bumiller <w.bumiller@proxmox.com>
Cc: pbs-devel@lists.proxmox.com, Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pbs-devel] [PATCH v3 proxmox 2/3] compression: Add support for symlinks in zip files
Date: Thu, 21 Dec 2023 12:37:42 +0100	[thread overview]
Message-ID: <086153c2-0458-409d-a241-2a2b02bbbb34@proxmox.com> (raw)
In-Reply-To: <ccc4uaslsrj67zrjeixmjwvofnqos4vuyp5e3a6niljmaovr4e@wl4dq2mjueiv>

On 20/12/2023 14:20, Wolfgang Bumiller wrote:
> With the link of a symlink being encoded in the contents, I wonder if we
> should just do the same in the code. Not normally what I'd go for in
> rust, but...
>
> On Thu, Dec 14, 2023 at 03:48:21PM +0100, Filip Schauer wrote:
>> Add support for symlinks to ZipEntry. A symlink is encoded by or-ing its
>> attributes with S_IFLNK as seen in the kernel in
>> include/uapi/linux/stat.h
>>
>> Signed-off-by: Filip Schauer<f.schauer@proxmox.com>
>> ---
>>   proxmox-compression/src/zip.rs | 32 +++++++++++++++++++++++++-------
>>   1 file changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/proxmox-compression/src/zip.rs b/proxmox-compression/src/zip.rs
>> index 069e8bc..a3b2346 100644
>> --- a/proxmox-compression/src/zip.rs
>> +++ b/proxmox-compression/src/zip.rs
>> @@ -74,6 +74,7 @@ fn epoch_to_dos(epoch: i64) -> (u16, u16) {
>>   pub enum FileType {
>>       Directory,
>>       Regular,
>> +    Symlink(OsString),
> ... then this enum could be #[repr(u32)] using the values from the hunk
> below as discriminants here directly.
> And without the OsString in there this could be all of `Clone + Copy +
> Eq + PartialEq`, turning all the `matches!()` in this series into
> comparisons with `==`.


Do you want to pass the symlink target as an additional argument to
add_entry? If so, add_entry would take an enum FileType, an Option
content and an Option symlink_target. Depending on the FileType, at
least one of the Options would be None. This approach does not seem very
clean to me.

I just thought of another way:

How about inserting the content into FileType::Regular. This would
eliminate the need to pass None to add_entry. Therefore, it would be
impossible to pass conflicting parameters to the function, such as
FileType::Regular with content = None. This change would not get rid of
the hunk you mentioned, but the code seems cleaner to me this way.




  reply	other threads:[~2023-12-21 11:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-14 14:48 [pbs-devel] [PATCH v3 many] fix #4995: Include symlinks in zip file restore Filip Schauer
2023-12-14 14:48 ` [pbs-devel] [PATCH v3 proxmox 1/3] compression: Add a FileType enum to ZipEntry Filip Schauer
2023-12-14 14:48 ` [pbs-devel] [PATCH v3 proxmox 2/3] compression: Add support for symlinks in zip files Filip Schauer
2023-12-20 13:20   ` Wolfgang Bumiller
2023-12-21 11:37     ` Filip Schauer [this message]
2023-12-21 12:03       ` Wolfgang Bumiller
2023-12-21 12:15         ` Filip Schauer
2023-12-21 12:11       ` Wolfgang Bumiller
2024-01-24 10:19         ` Filip Schauer
2023-12-14 14:48 ` [pbs-devel] [PATCH v3 proxmox 3/3] compression: Add unit tests for the ZipEncoder Filip Schauer
2023-12-14 14:48 ` [pbs-devel] [PATCH v3 backup 1/2] pxar: Adopt FileType enum when creating a ZipEntry Filip Schauer
2023-12-14 14:48 ` [pbs-devel] [PATCH v3 backup 2/2] fix #4995: pxar: Include symlinks in zip file creation Filip Schauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=086153c2-0458-409d-a241-2a2b02bbbb34@proxmox.com \
    --to=f.schauer@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=w.bumiller@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal