r-lib / archive

R bindings to libarchive, supporting a large variety of archive formats
https://archive.r-lib.org/
Other
145 stars 17 forks source link

Add password #73

Open cielavenir opened 2 years ago

cielavenir commented 2 years ago

closes #8 (unless you meant providing callback)

DanChaltiel commented 2 years ago

I tested this PR on my computer and this works perfectly.

Unfortunately, the logs have been deleted so we cannot know why the checks failed. Is there anything we could do to help so that this PR could be accepted?

cielavenir commented 2 years ago

@gaborcsardi could you rerun roxygen2 onto this branch? I don't know how.

gaborcsardi commented 2 years ago

/document

cielavenir commented 2 years ago

Thank you @gaborcsardi

@DanChaltiel now doc is sane, much closer to the goal

But why windows CI is failing? Is this libarchive binary ('s password) issue or not...?

gaborcsardi commented 2 years ago

Is is possible that the libarchive Windows build from rtools42 is missing something for password support?

cielavenir commented 2 years ago

I tried something but failing so far

cielavenir commented 2 years ago

Another interesting thing is that https://github.com/cielavenir/miniarc can handle encryption on Windows build. Note that I loaded libarchive-13.dll from https://packages.msys2.org/package/mingw-w64-x86_64-libarchive , which should be the same source as rwinlib libarchive.

gaborcsardi commented 2 years ago

AFAICT we do not currently use rwinlib on the CI, but use the libarchive static lib from rtools42. Many dependencies of libarchive are optional, so maybe that build does not have everything that is needed?

cielavenir commented 1 year ago

Let's revisit after https://github.com/r-lib/archive/pull/80

cielavenir commented 1 year ago

It is now ready.

cielavenir commented 1 year ago

seems like I'm still new to R binding (well, I just took a look on this as an extension from https://github.com/r-lib/archive/pull/72 ).

cpp11::strings password;
  if (password.size() > 0) {
    call(archive_write_set_passphrase, out, std::string(password[0]).c_str());
  }

umm that is when password is character(), not NA_character_.

cielavenir commented 10 months ago

@gaborcsardi @jimhester

Let me explain the situation again.

I can add the password feature, but the test does not pass on Windows because archive_write_files is broken. I asked you to fix it but I don't get replies. I fixed it by my own but it is not accepted due to coding style. But what I do is correcting broken coding style of r-lib/archive:master.

Do note that currently test-archive_write_files.R is wrongly passing on Windows.

You don't fix, my fix is not accepted, hence now I think of skipping password test on Windows. Please perform some action(s). I'm serious.

cielavenir commented 8 months ago

I think of skipping password test on Windows

done. really heartbreak.