add ide-specific settings to .gitignore and simply implement 7z #393

MisileLab · 2023-04-02T11:05:20Z

fix #164

Description

implement 7z and add .vscode and .idea to .gitignore

todo

implement 7z list
return error not using unwrap

marcospb19 · 2023-04-03T17:57:34Z

Thanks for your contribution! A review will come once one of us finds the time to take a look.

marcospb19

Thanks for your PR, there are some changes needed for it to be merged.

In the title you wrote "simply implement 7z", so maybe your plan was to leave this as it is right now, a simpler implementation, if that's the case, I'll ask you to leave the PR open so that anyone can pick from where you left it and finish it, otherwise, you're more than welcome to finish it yourself.

src/commands/mod.rs

src/commands/decompress.rs

src/commands/list.rs

src/commands/compress.rs

src/commands/list.rs

src/commands/decompress.rs

marcospb19 · 2023-04-20T00:24:46Z

I've asked for a review from Figsoda because unfortunately I've been busy (and kinda exhausted), but he's also kinda busy so maybe this will have to wait.

Mic92

I looked at the code and it looks pretty simple and straight forward. If I was the project maintainer I would probably also want to see some lines added in ./tests/integration.rs

marcospb19

This is the final change request.

As @Mic92 mentioned, it's necessary to add 7z to the integration tests file.

src/commands/compress.rs

src/commands/decompress.rs

src/commands/list.rs

MisileLab · 2023-05-11T11:31:15Z

I looked at the code and it looks pretty simple and straight forward. If I was the project maintainer I would probably also want to see some lines added in ./tests/integration.rs

idk how to implement test, I'll add tests/integration.rs when I implement it or someone implement test and comment it

marcospb19 · 2023-05-11T20:12:34Z

Thanks for sticking to this! It's looking great!

EDIT: removed reference, it looks misleading.

figsoda · 2023-05-24T14:55:46Z

added 7z to tests, but I kept running into this error

thread \'main\' panicked at \'StripPrefix Failed: StripPrefixError(())\', src/archive/sevenz.rs:17:26

marcospb19 · 2023-06-01T19:43:35Z

src/archive/sevenz.rs

+pub fn compress_sevenz(files: Vec<PathBuf>, output_path: &Path) -> crate::Result<bool> {
+    let mut writer = sevenz_rust::SevenZWriter::create(output_path).map_err(crate::Error::SevenzipError)?;
+
+    for filep in files.iter() {
+        writer
+            .push_archive_entry::<std::fs::File>(
+                sevenz_rust::SevenZWriter::<std::fs::File>::create_archive_entry(
+                    filep,
+                    filep
+                        .strip_prefix(current_dir()?)
+                        .expect("StripPrefix Failed")
+                        .as_os_str()
+                        .to_str()
+                        .unwrap()
+                        .to_string(),
+                ),
+                None,
+            )
+            .map_err(crate::Error::SevenzipError)?;
+    }
+
+    writer.finish()?;
+    Ok(true)


It's in line 17 of this big oneliner.

MisileLab · 2023-06-11T15:05:22Z

i don't know how to fix it and I have a no time for now, anyone can contribute this pull request!

marcospb19 · 2023-06-11T17:31:30Z

@MisileLab thanks for your hard-work! I might be able to pick it up soon.

AntoniosBarotsis · 2023-08-18T18:42:39Z

I saw that tar uses strip_cur_dir instead of something like filep.strip_prefix(current_dir()?) where the issue was in the 7z code.

Switching to that does solve the panic (I can now manually compress and decompress 7z archives from the cli) however it does not fix all the tests. Using strip_cur_dir in my case did not change the file path but instead kept it the same (again, the resulting behaviour worked as expected though). I'm not sure if this was not used on purpose or if using it is a bad idea for some reason so do let me know, if its not then I can push the commit 😅

Another thing that was surprising was when decompressing an archive that was compressed by ouch I get the following log:

$ cargo r -- d tmp.7z
[INFO] Could not detect the extension of `tmp`

This was interesting because it did decompress correctly. The bytes here seem to agree with Wikipedia so not sure why it fails to detect it (but somehow still work?).

The following tests are failing:

test single_empty_file ... FAILED
test single_file ... FAILED
test multiple_files ... FAILED

Edit: On closer inspection, upon decompressing a 7z archive, an empty folder with the same name as the initial archive is created in the same directory

Logs:

$ cargo r -- d tmp.7z
[INFO] Could not detect the extension of `tmp`
[INFO] Created temporary directory C:\Users\anton\Documents\Github\ouch\tmp\.\.tmpRCeHQ4 to hold decompressed elements.
[INFO] Successfully moved C:\Users\anton\Documents\Github\ouch\tmp\.\.tmpRCeHQ4 to .\tmp.
[INFO] Successfully decompressed archive in current directory.
[INFO] Files unpacked: 1

Signed-off-by: MisileLab <[email protected]>

zzzsyyy · 2023-09-19T10:47:24Z

will it work？ Whats the block so far?

marcospb19 · 2023-09-19T17:13:48Z

will it work？ Whats the block so far?

I believe it will. The current block is: PR is not ready yet, we're waiting for someone to pick it up and spend more time finishing it (and maybe opening another PR based on this branch, but rebased to solve conflicts).

The one who picks it up needs to debug the errors, solve them, and make the tests pass.

tip: .tar and .zip implementations can be used as a reference.

Flat · 2023-11-25T00:24:13Z

I've started working on this PR hoping to fix the last issues and get it merged, however I seem to be stuck around the same place after re-basing. If we use the original code from this PR, 7zip will fail tests that contain any additional compression on 7z (bz, bz2 gzip, etc) this is the BadSignature error, it has the header bytes of bzip, which suggests it's being passed to the 7z decompression/unpack before it has been fully decompressed from the other algorithms.

If we try the temp file creation that rar uses, nothing is written from the reader to the temp file, which is confusing since attaching lldb I see the reader has data, but it isn't copied.

marcospb19 · 2023-11-25T13:18:21Z

@Flat

this is the BadSignature error, it has the header bytes of bzip, which suggests it's being passed to the 7z decompression/unpack before it has been fully decompressed from the other algorithms

Does it look like a problem with our code or the 7z crate code?

Flat · 2023-11-25T16:08:55Z

@Flat

this is the BadSignature error, it has the header bytes of bzip, which suggests it's being passed to the 7z decompression/unpack before it has been fully decompressed from the other algorithms

Does it look like a problem with our code or the 7z crate code?

I may be barking up the wrong tree on that info actually. Doing some more testing and it looks like regular 7z compression fails, due to a symlink I have setup, the file paths passed to archive/compression algorithms are different than the env:current_dir path used in strip_cur_dir. Once I figure that out I'll look more into the compression/decompression chain.

marcospb19 · 2023-11-25T22:56:57Z

Superseded by #555, thanks @MisileLab and others who gave input here.

MisileLab added 7 commits April 2, 2023 10:24

chore: add .vscode and .idea to .gitignore

33dd520

feat: simple impl 7zip and lot of todo

0122621

style: cargo fmt

15a2ff8

fix: why this is 72

9ef7f50

[ci skip] not ready

fdc1d50

fix: now recursivly compress 7z

d0939fb

[ci skip] add pr to changelog

277f374

MisileLab marked this pull request as ready for review April 3, 2023 13:53

marcospb19 requested changes Apr 17, 2023

View reviewed changes

marcospb19 reviewed Apr 17, 2023

View reviewed changes

src/commands/list.rs Outdated Show resolved Hide resolved

MisileLab added 4 commits April 17, 2023 13:23

feat: decompress with count

65fba63

feat: good error output

4265eb8

feat: good error handling

aa069ae

feat: cleanup codes and more error handling

a374b86

MisileLab requested a review from marcospb19 April 17, 2023 14:37

MisileLab added 2 commits April 18, 2023 04:05

feat: RefCell remove

49bde92

fix: Incorrect decompressed file count

8648161

marcospb19 reviewed Apr 20, 2023

View reviewed changes

src/commands/decompress.rs Outdated Show resolved Hide resolved

fix: Incorrect count

273ab88

Mic92 approved these changes May 8, 2023

View reviewed changes

marcospb19 requested changes May 8, 2023

View reviewed changes

src/commands/compress.rs Outdated Show resolved Hide resolved

src/commands/decompress.rs Outdated Show resolved Hide resolved

src/commands/list.rs Outdated Show resolved Hide resolved

impl except test

c08105b

MisileLab and others added 3 commits May 23, 2023 17:52

Merge branch 'ouch-org:main' into main

7e198e4

add 7z to tests

10cae99

small cleanups

656514a

marcospb19 reviewed Jun 1, 2023

View reviewed changes

MisileLab added 2 commits August 20, 2023 02:32

Merge branch 'main' of github.com:ouch-org/ouch

8c100e3

Signed-off-by: MisileLab <[email protected]>

idk i just merged and patch some

5d58f22

Signed-off-by: MisileLab <[email protected]>

MisileLab added 2 commits September 29, 2023 13:49

Merge branch 'main' into main

a629cb4

I just merge it

193bb60

Flat mentioned this pull request Nov 25, 2023

Implement 7z support, fix windows specific symlink issue and UI tests #555

Merged

marcospb19 closed this Nov 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add ide-specific settings to .gitignore and simply implement 7z #393

add ide-specific settings to .gitignore and simply implement 7z #393

MisileLab commented Apr 2, 2023 •

edited

Loading

marcospb19 commented Apr 3, 2023

marcospb19 left a comment

marcospb19 commented Apr 20, 2023

Mic92 left a comment

marcospb19 left a comment

MisileLab commented May 11, 2023

marcospb19 commented May 11, 2023 •

edited

Loading

figsoda commented May 24, 2023

marcospb19 Jun 1, 2023

MisileLab commented Jun 11, 2023

marcospb19 commented Jun 11, 2023

AntoniosBarotsis commented Aug 18, 2023 •

edited

Loading

zzzsyyy commented Sep 19, 2023

marcospb19 commented Sep 19, 2023

Flat commented Nov 25, 2023

marcospb19 commented Nov 25, 2023 •

edited

Loading

Flat commented Nov 25, 2023

marcospb19 commented Nov 25, 2023

add ide-specific settings to .gitignore and simply implement 7z #393

add ide-specific settings to .gitignore and simply implement 7z #393

Conversation

MisileLab commented Apr 2, 2023 • edited Loading

Description

todo

marcospb19 commented Apr 3, 2023

marcospb19 left a comment

Choose a reason for hiding this comment

marcospb19 commented Apr 20, 2023

Mic92 left a comment

Choose a reason for hiding this comment

marcospb19 left a comment

Choose a reason for hiding this comment

MisileLab commented May 11, 2023

marcospb19 commented May 11, 2023 • edited Loading

figsoda commented May 24, 2023

marcospb19 Jun 1, 2023

Choose a reason for hiding this comment

MisileLab commented Jun 11, 2023

marcospb19 commented Jun 11, 2023

AntoniosBarotsis commented Aug 18, 2023 • edited Loading

zzzsyyy commented Sep 19, 2023

marcospb19 commented Sep 19, 2023

Flat commented Nov 25, 2023

marcospb19 commented Nov 25, 2023 • edited Loading

Flat commented Nov 25, 2023

marcospb19 commented Nov 25, 2023

MisileLab commented Apr 2, 2023 •

edited

Loading

marcospb19 commented May 11, 2023 •

edited

Loading

AntoniosBarotsis commented Aug 18, 2023 •

edited

Loading

marcospb19 commented Nov 25, 2023 •

edited

Loading