Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo-dist include directive _may_ be having problems archiving subdirectories? #873

Closed
simonsan opened this issue Mar 24, 2024 · 14 comments · Fixed by axodotdev/axoasset#94
Labels
bug Something isn't working

Comments

@simonsan
Copy link

simonsan commented Mar 24, 2024

I'm actually not sure, where the problem lies. I opened this issue with scoop first, because it got apparent when I extracted a cargo-dist packaged app: ScoopInstaller/Scoop#5845

But thinking about it, cargo-dist was zipping the file with the following include directive:

# Include files in the dist package
include = [
  "./config/",
  "./docs/",
  "./templates/",
]

So I wonder if there are some flags being used, that make it incompatible to be unpacked with 7-Zip?

A zip archive packed by cargo-dist like this:

│   CHANGELOG.md
│   LICENSE
│   pace.exe
│   README.md

├───config
│       pace.toml
│       projects.pace.toml
│       README.md
│       tasks.pace.toml

├───docs
│       LICENSE
│       pace-dev-docs.pdf
│       pace-user-docs.pdf
│       README.md

└───templates
    │   pace_report_json.html
    │   README.md

    └───reflections
            basic.html
            basic.md

is being extracted like this on Win11-x64 with 7-Zip: 23.01-x64

│   CHANGELOG.md
│   configpace.toml
│   configprojects.pace.toml
│   configREADME.md
│   configtasks.pace.toml
│   docsLICENSE
│   docspace-dev-docs.pdf
│   docspace-user-docs.pdf
│   docsREADME.md
│   install.json
│   LICENSE
│   manifest.json
│   pace.exe
│   README.md
│   templatespace_report_json.html
│   templatesREADME.md
│   templatesreflectionsbasic.html
│   templatesreflectionsbasic.md

├───config                       # persisted from before
│       pace.toml                # persisted from before
│       pace.toml.bak            # persisted from before

├───config.original
├───docs                         # persisted from before
├───logs                         # persisted from before
├───templates
└───templatesreflections

I'm talking about this archive: https://github.com/pace-rs/pace/releases/download/pace-rs-v0.15.1/pace-rs-x86_64-pc-windows-msvc.zip

I tried to get to the ground of it, but got lost in the Github Actions file. If you could direct to the root of this, I would be able to investigate further. Thank you!

@mistydemeo mistydemeo added the bug Something isn't working label Mar 25, 2024
@mistydemeo
Copy link
Contributor

Wow, that's weird. I'm seeing the same thing with unar and unzip -l; the forward slash is part of the filenames instead of the proper directory structure being included. I'll look into it.

@mistydemeo
Copy link
Contributor

It looks like the tarballs are fine; only the zips are affected. I'm looking into the generation to identify the issue.

@simonsan
Copy link
Author

Oh wow, thanks. I wanted to confirm, first, what's going on. Thank you a lot for looking into it. <3

mistydemeo added a commit to axodotdev/axoasset that referenced this issue Mar 26, 2024
ZIP expects unix-style paths here, and passing Windows paths will cause
incorrect and broken files to be added to the directory.

The `zip` crate's `start_file_from_path` and `add_directory_from_path` normalize
these, but they were deprecated without replacement so we need to do the work
ourselves.

Fixes axodotdev/cargo-dist#873.
mistydemeo added a commit to axodotdev/axoasset that referenced this issue Mar 26, 2024
ZIP expects unix-style paths here, and passing Windows paths will cause
incorrect and broken files to be added to the directory.

The `zip` crate's `start_file_from_path` and `add_directory_from_path` normalize
these, but they were deprecated without replacement so we need to do the work
ourselves.

Fixes axodotdev/cargo-dist#873.
@sorokya
Copy link

sorokya commented Mar 31, 2024

Will this fix be released soon? My users are mostly on windows and the broken zip is pretty annoying.

Thanks

@mistydemeo
Copy link
Contributor

It's a holiday weekend at the moment, so I've been out of the office a little longer than usual. The next release will most likely be this coming week.

@sorokya
Copy link

sorokya commented Mar 31, 2024

It's a holiday weekend at the moment, so I've been out of the office a little longer than usual. The next release will most likely be this coming week.

No rush! Just wanted to confirm. Thanks. Happy Easter 😊

@mistydemeo
Copy link
Contributor

This fix is now available in the 0.12.1 bugfix release.

@simonsan
Copy link
Author

simonsan commented Apr 4, 2024

Thanks a lot!

@sorokya
Copy link

sorokya commented Apr 7, 2024

I think there might still be an issue with sub directories.

In my project I'm trying to include pub/items but in the zip I get a folder named pub (with the readme file inside as expected! This was fixed)

But instead of a subdirectory in pub for items I get a folder in the root of the zip named pub\items and another folder named just items

Can look at the latest release of my repo to see the issue.

reoserv

It might be the way I'm including the paths actually. Should it "just work" to put the top most directory name? I'm specifying the sub directories and files.

# Extra static files to include in each App (path relative to this Cargo.toml's dir)
include = ["./Config.toml", "./Arenas.ron", "./Commands.ron", "./Formulas.ron", "./PacketRateLimits.ron", "./db-init/", "./news.txt", "./lang/", "./maps/", "./pub/", "./pub/inns/", "./pub/npcs/", "./pub/items/", "./pub/shops/", "./pub/spells/", "./pub/classes/", "./pub/skill_masters/", "./quests/"]

@mistydemeo mistydemeo reopened this Apr 10, 2024
@mistydemeo mistydemeo reopened this Apr 10, 2024
@mistydemeo
Copy link
Contributor

I'll test with that repo and see if I can identify the issue. Sorry about the inconvenience!

@mistydemeo
Copy link
Contributor

I believe this should be fixed with the just-released 0.13.2; I tested with reoserv on Windows and the ZIP structure looks good to me now. Can you confirm if this is working for you now?

@sorokya
Copy link

sorokya commented Apr 19, 2024

Thanks! I'll test this soon

@sorokya
Copy link

sorokya commented Apr 25, 2024

I believe this should be fixed with the just-released 0.13.2; I tested with reoserv on Windows and the ZIP structure looks good to me now. Can you confirm if this is working for you now?

@mistydemeo This is working now for me! thanks so much. Can close issue

@mistydemeo
Copy link
Contributor

You're welcome! Glad it's working for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants