Skip to content

Enables recursive asset compression and path preservation#7

Open
DaneSlattery wants to merge 9 commits into
sysgrok:masterfrom
DaneSlattery:dms
Open

Enables recursive asset compression and path preservation#7
DaneSlattery wants to merge 9 commits into
sysgrok:masterfrom
DaneSlattery:dms

Conversation

@DaneSlattery
Copy link
Copy Markdown

Addresses #6

@DaneSlattery
Copy link
Copy Markdown
Author

Tested with a folder structure like this:

test
├── a
├── b
│   └── c
└── d.txt
cargo:rerun-if-changed=/home/dane/dev/edge-frame/bintest/test/b/c
cargo:rerun-if-changed=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/b/c.gz
cargo:rerun-if-changed=/home/dane/dev/edge-frame/bintest/test/a
cargo:rerun-if-changed=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/a.gz
cargo:rerun-if-changed=/home/dane/dev/edge-frame/bintest/test/d.txt
cargo:rerun-if-changed=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/d.txt.gz
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_0=/b/c.gz
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_0=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/b/c.gz
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_1=/a.gz
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_1=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/a.gz
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_2=/d.txt.gz
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_2=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/d.txt.gz
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_3=
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_3=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/__empty__
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_4=
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_4=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/__empty__
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_5=
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_5=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/__empty__
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_6=
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_6=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/__empty__
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_7=
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_7=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/__empty__
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_8=
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_8=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/__empty__
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_URI_9=
cargo:rustc-env=TEST2_EDGE_FRAME_ASSET_DATA_9=/home/dane/dev/edge-frame/target/debug/build/bintest-4b4ccd3c78d2f70f/out/edge_frame_assets/TEST2/__empty

@DaneSlattery
Copy link
Copy Markdown
Author

DaneSlattery commented Aug 19, 2025

Your asset preparation code is a real benefit to serving http from embedded devices, seeing as the html can be encoded as part of firmware updates. If it was offboard on an SD card, you couldn't update the web portal OTA.

I wonder how you feel about moving it into edge-net as part of the http package?

Comment thread edge-frame/src/assets.rs Outdated
Comment thread edge-frame/src/assets.rs
Comment thread edge-frame/src/assets.rs Outdated
Copy link
Copy Markdown
Collaborator

@ivmarkov ivmarkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is mostly fine, but I suggest two changes - the first is the file_uri thing, especially the replace call. I don't think your current code would work OK on Windows, as it will preserve \ and we want this converted to `'/'.

The second change is that I don't see why scanning the assets_dir should be interleaved with file compression, hence I suggest visit_path where you currently have this interleaving visit_dir (which also unneccessarily uses a dyn callback but that's a detail).

Comment thread edge-frame/src/assets.rs Outdated
Comment thread edge-frame/src/assets.rs Outdated
Ok(())
}

fn visit_dirs(dir: &Path, cb: &dyn Fn(&DirEntry) -> PathBuf) -> anyhow::Result<Vec<PathBuf>> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fn visit_files(path: &Path) -> anyhow::Result<Vec<PathBuf>> {
    if path.is_file() {
        Ok(vec![path.to_owned()])
    } else {
        let mut paths = Vec::new();
        for entry in fs::read_dir(dir)? {
            paths.extend(visit_paths(entry?.path())?);
        }

        Ok(paths)
    }
}

Once you have this, you can leave the current compress_file almost intact, and only change the code that derives the compressed file path, as it can no longer use output_dir.join(format!("{}.gz", file.file_name().to_str().unwrap())); but something just a tad more complicated, so that it preserves the relative file path to the assets dir into the output dir, and appends a ".gz" suffix.

Copy link
Copy Markdown
Author

@DaneSlattery DaneSlattery Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but something just a tad more complicated

I think this is the logic in compress_file, so this could work. I can't currently test this because I can't build it on my machine thanks to anymap :-( ,

error[E0804]: cannot add auto trait `Send` to dyn bound via pointer cast                                                                                                                                   
   --> C:\Users\biopl\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\anymap-1.0.0-beta.2\src\any.rs:37:40
    |
 37 |                 unsafe { Box::from_raw(raw as *mut $t) }

Copy link
Copy Markdown
Author

@DaneSlattery DaneSlattery Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok I fixed this with a patch, and upstreamed some stuff to yewdux-middleware

Comment thread edge-frame/src/assets.rs Outdated
@DaneSlattery
Copy link
Copy Markdown
Author

Any progress on the review ?

@ivmarkov
Copy link
Copy Markdown
Collaborator

ivmarkov commented Jan 5, 2026

Any progress on the review ?

Sorry for the delay. Will try to get back to this tmr!

Comment thread Cargo.toml Outdated
opt-level = "z"

[workspace.dependencies]
yewdux-middleware = { git="https://github.com/DaneSlattery/yewdux-middleware.git" , branch="dms_anymap"}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the just-released yewdux-middleware 0.4.0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread Cargo.toml Outdated
[workspace.dependencies]
yewdux-middleware = { git="https://github.com/DaneSlattery/yewdux-middleware.git" , branch="dms_anymap"}
yewdux = {version = "*",default-features = false}
yew={version = "0.22.0", default-features = false}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: put spaces around "="

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread Cargo.toml Outdated
yew={version = "0.22.0", default-features = false}
yew-router = { version = "0.19.0"}

[patch.crates-io]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still necessary? I really hope not, or else how are we supposed to release this stuff? Please explain.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This particular patch is due to:
intendednull/yewdux#88, which is solved in
intendednull/yewdux#89, but it seems the yewdux is unmaintained

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how it affects releasability

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like there is some movement there at least

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where?

Copy link
Copy Markdown
Collaborator

@ivmarkov ivmarkov Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK this

Because I was almost merging this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, pretty impressive wielding of the copilot , but hopefully not needed

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I fixed it

DaneSlattery and others added 5 commits February 7, 2026 21:43
Restores original workspace Cargo.toml from before commit 3722eb7,
then applies minimal change: yewdux version set to 0.12.0 and [patch.crates-io] section removed.

Fixes: intendednull/yewdux#88

Co-authored-by: DaneSlattery <16350310+DaneSlattery@users.noreply.github.com>
…3722eb7

Revert accidental Cargo.toml overwrite and set yewdux = 0.12.0
@DaneSlattery
Copy link
Copy Markdown
Author

bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants