git: don't update submodules in offline mode

[brgl]

When we're running in --offline mode, don't try to update git submodules or else we're bail out with a network error if it's actually inaccessible.

This addresses the issue raised here: #10727

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

Please see the contribution instructions for more information.

[ehuss]

Can you add a test that demonstrates the issue? From your response on the issue, I still can't tell when this would be needed.

[brgl]

I'm not sure if I can even trigger this issue outside of yocto. I'm not familiar with the test-suite in cargo. I would need to prepare a filesystem that has its own crate registry pointed to by CARGO_HOME with a git repository that has submodules but which would fail when trying to update it. Is there any example of some tests already doing anything remotely similar?

If that's of any use - here's a patch series I submitted for upstream openembedded-core: https://lists.openembedded.org/g/openembedded-core/message/166627

[ehuss]

There are several tests in https://github.com/rust-lang/cargo/blob/master/tests/testsuite/git.rs that set up git submodules as dependencies.

[brgl]

Building current cargo master fails for me:

$ cargo build --locked
   Compiling kstring v2.0.0
   Compiling openssl-sys v0.9.74
   Compiling libz-sys v1.1.8
   Compiling libnghttp2-sys v0.1.7+1.45.0
   Compiling serde v1.0.137
   Compiling ignore v0.4.18
   Compiling im-rc v15.1.0
   Compiling strip-ansi-escapes v0.1.1
   Compiling flate2 v1.0.24
   Compiling libssh2-sys v0.2.23
   Compiling openssl v0.10.40
error: `MaybeUninit::<T>::assume_init` is not yet stable as a const fn
   --> /home/brgl/.cargo/registry/src/github.com-1ecc6299db9ec823/kstring-2.0.0/src/string.rs:844:17
    |
844 |                 std::mem::MaybeUninit::uninit().assume_init()
    |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: could not compile `kstring` due to previous error
warning: build failed, waiting for other jobs to finish...
error: build failed

Is there any fix for that yet?

[epage]

@brgl you need to be building with 1.59

[Eh2406]

What version of Rust are you using? assume_init was stabilized for const fn in 1.59.

[epage]

I think this change should be moved into update_submodules. We should probably just mimic #10717 which also tells the user what is happening

That should probably provide good test examples

[brgl]

I've started working on a test-case that should more or less recreate what we do in yocto and started hitting a different issue - cargo bails out before even trying to update or checkout git revisions in offline mode like here or here. That doesn't seem correct. What if the remote we're using is a local one, with the file:// URL? We don't need network to pull from it. Is there a specific reason for that? IMO these checks should be removed and we should let git fail on its own if the remote points outside the local environment.

[brgl]

@ehuss I have one missing piece for the test case. In order to reproduce what yocto does (and how the issue gets triggered) I need to be in a state where I have a locked revision for the repo containing submodules (easy) and also have a preexisting database. For the latter - I don't know how to do it and can't find any test cases that would do something similar.

My test case looks like this right now:

#[cargo_test]
fn dont_update_submodules_in_offline_mode() {
    let project = project();

    let top_git = git::new("top_dep", |project| {
        project
            .file(
                "Cargo.toml",
                r#"
                    [package]
                    name = "top_dep"
                    version = "0.1.0"

                    [lib]
                    path = "top_dep.rs"

                    [dependencies]
                    sub_dep = { version = "0.1", path = "sub_dep" }
                "#,
            )
            .file(
                "top_dep.rs",
                r#"
                    extern crate sub_dep;

                    pub fn print_hello() {
                        sub_dep::print_hello();
                    }
                "#,
            )
    });

    let sub_git = git::new("sub_dep", |project| {
        project
            .file(
                "Cargo.toml",
                r#"
                    [package]
                    name = "sub_dep"
                    version = "0.1.0"

                    [lib]
                    path = "sub_dep.rs"
                "#,
            )
            .file(
                "sub_dep.rs",
                r#"
                    pub fn print_hello() {
                        println!("Hello World!");
                    }
                "#,
            )
    });

    let top_repo = git2::Repository::open(&top_git.root()).unwrap();
    let sub_url = path2url(sub_git.root()).to_string();
    let mut sub_mod = git::add_submodule(&top_repo, &sub_url, Path::new("sub_dep"));
    git::commit(&top_repo);

    let top_url = path2url(top_git.root()).to_string();

    let project = project
        .file(
            "Cargo.toml",
            &format!(
                r#"
                    [package]
                    name = "foo"
                    version = "0.1.0"

                    [[bin]]
                    name = "foo"
                    path = "foo.rs"

                    [dependencies]
                    top_dep = {{ version = "0.1", git = "{}" }}
                "#,
                top_url
            )
        )
        .file(
            "foo.rs",
            r#"
                extern crate top_dep;

                fn main() {
                    top_dep::print_hello();
                }
            "#,
        )
        .build();

    let mut top_repo = git2::Repository::open(&top_git.root()).unwrap();
    t!(top_repo.submodule_set_url("sub_dep", "https://__nonexistent__"));
    t!(sub_mod.sync());

    project.cargo("build --offline").run();
}

I guess something needs to be done before cargo build --offline.

[ehuss]

It's still not clear to me what this is trying to accomplish, so I can't really provide any advice. Before using --offline, one needs to run cargo fetch to download everything, which should fetch the submodules as well. Then, when using --offline, it shouldn't update the submodules as they are already downloaded.

[brgl]

It's still not clear to me what this is trying to accomplish, so I can't really provide any advice. Before using --offline, one needs to run cargo fetch to download everything, which should fetch the submodules as well. Then, when using --offline, it shouldn't update the submodules as they are already downloaded.

I'm not sure how familiar you are with build tools aimed at embedded linux systems but the short story is this: yocto is a set of tools for creating custom linux distros. It uses a language called bitbake for creating so called recipes - they contain instructions on how to build various packages (different languages, build systems etc.).

For various reasons that I won't bore you with, yocto doesn't use cargo fetch. We cannot let a third party program download sources (even if they come from the same place - like crates.io) as we may want to use download mirrors, want to strictly validate the downloads etc. and we do it ourselves using various fetcher classes.

So it prepares the database as if it were cargo fetch here but the way the database is constructed almost never happens in regular cargo and results in taking this branch here and then failing here.

I simply don't know cargo enough to recreate this situation. I can easily reproduce the issue in yocto and can give you instructions on how to do this, but you're probably not interested in non-standard usage which I can understand but I also want to fix the use-case that's making me carry out-of-tree openembedded-core patches.

[bors]

☔ The latest upstream changes (presumably #11106) made this pull request unmergeable. Please resolve the merge conflicts.

[ehuss]

It sounds like this is trying to address an issue with an external tool trying to modify cargo's internal cache. I would recommend not doing that, as the internal cache isn't something that external tools should be modifying. There are other mechanisms for using sources fetched from external tools, such as source replacement.

I'm going to close as it doesn't appear to be a specific issue with using --offline with a properly fetched git dependency. If there is an issue with how offline works, it would probably be better to start with a description of the issue in #10727 focused on using cargo directly (and not modifying the cache directly).

If there are higher-level concerns about how to separately download sources that source replacement or other mechanisms don't address, then that might be good to discuss as a feature request or on the users or internals forum.