anton-rs / op-up

Composable OP Stack Orchestration
https://stack.anton.systems
MIT License
41 stars 2 forks source link

perf: download monorepo instead of cloning from git #52

Closed merklefruit closed 11 months ago

merklefruit commented 11 months ago

Improves the UX of using op-up by downloading a compressed archive instead of cloning the op monorepo. The directory source is set to MonorepoSource::Tarball by default, but in the future this can be extended.

I tested the download with this test:

mod tests {
    #[test]
    fn test_download_and_uncompress_tar() {
        let pwd = std::env::current_dir().unwrap();
        super::download_file(&pwd, super::OP_MONOREPO_TAR_URL, "optimism.tar.gz").unwrap();
        super::unzip_tarball(&pwd, "optimism.tar.gz").unwrap();
        super::mv_dir(&pwd, "optimism-develop", super::MONOREPO_DIR).unwrap();

        assert!(pwd.join(super::MONOREPO_DIR).exists());

        std::fs::remove_file(pwd.join("optimism.tar.gz")).unwrap();
        std::fs::remove_dir_all(pwd.join(super::MONOREPO_DIR)).unwrap();
    }
}

But it's a silly test to keep because it would download stuff on every test run – so I removed it from the pr.

refcell commented 11 months ago

Should we put this config option in the stack.toml?

I definitely think the option should be in the stack toml config. Now that I think about it - could probably also add a configurable (as maybe another field like monorepo_url) to allow for someone to effectively "point" at a different monorepo - this is useful in modified fork cases for example. This should probably be done under a [monorepo] toml table. The monorepo toml table would also contain the aforementioned source=tarball | git field. Though the isolate = true field for github monorepos should probably still live under a [stages] table.

This PR adds the following dependencies: curl, tar. AFAIK they come preinstalled with all Unix systems though. we could use the curl crate which embeds the libcurl bindings for us, I don't think that's necessary for now.

I think for this, we should just make a note in the https://github.com/anton-rs/op-up/issues/7 issue which I started to implement in the Dependency manager pr #36

roninjin10 commented 11 months ago

Does this also fix this issue too? https://github.com/anton-rs/op-up/issues/56

merklefruit commented 11 months ago

Does this also fix this issue too? #56

Not yet, I'm gonna add this in now