Add version metadata tool

[dlon]

---

This change is 

[hulthe]

Reviewed 7 of 19 files at r1, 1 of 4 files at r2, all commit messages.
Reviewable status: 8 of 19 files reviewed, 3 unresolved discussions

---

mullvad-update/meta/src/github.rs line 17 at r2 (raw file):

        .text()
        .await
        .context("Failed to retrieve text for changes.txt (tag missing?)")

⛏️ Ideally, all of the contexts should include the URL I think

---

mullvad-update/meta/src/config.rs line 29 at r2 (raw file):

            }
            Err(err) => Err(err).context(format!("Failed to read {CONFIG_FILENAME}")),
        }

⛏️ This is racey as we're opening the file twice, but probably not an issue irl tbh

---

mullvad-update/meta/src/config.rs line 36 at r2 (raw file):

        fs::write(CONFIG_FILENAME, toml_str.as_bytes())
            .await
            .context(format!("Failed to save {CONFIG_FILENAME}"))

Hmm. I would use anyhow! instead of format!, but I wonder if there's actually any difference in this case? Probably not.

---

mullvad-update/meta/src/artifacts.rs line 30 at r2 (raw file):

        .stream_position()
        .await
        .context("Failed to get file size")?;

have you considered using File::metadata instead?

[dlon]

Reviewable status: 6 of 19 files reviewed, 2 unresolved discussions (waiting on @hulthe)

---

mullvad-update/meta/src/artifacts.rs line 30 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

have you considered using File::metadata instead?

Done.

---

mullvad-update/meta/src/config.rs line 36 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Hmm. I would use anyhow! instead of format!, but I wonder if there's actually any difference in this case? Probably not.

Would anyhow! result in two errors? Anyhow, I switched to with_context with format!. That's what anyhow suggests in its docs. 🤷

---

mullvad-update/meta/src/github.rs line 17 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

⛏️ Ideally, all of the contexts should include the URL I think

It is included in the source error:

Error: Failed to retrieve changes.txt (tag missing?)

Caused by:
    0: error sending request for url (https://raw.githubuserconxtent.com/mullvad/mullvadvpn-app/refs/tags/2123.1/desktop/packages/mullvad-vpn/changes.txt)
    1: client error (Connect)
    2: dns error: failed to lookup address information: Name or service not known
    3: failed to lookup address information: Name or service not known

[hulthe]

Reviewed 7 of 19 files at r1, 1 of 4 files at r2.
Reviewable status: 14 of 19 files reviewed, 9 unresolved discussions (waiting on @dlon)

---

mullvad-update/meta/src/config.rs line 36 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

Would anyhow! result in two errors? Anyhow, I switched to with_context with format!. That's what anyhow suggests in its docs. 🤷

Using context always adds an additional error afaik. Using format vs anyhow doesn't matter in that respect

---

mullvad-update/meta/src/github.rs line 17 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

It is included in the source error:

Error: Failed to retrieve changes.txt (tag missing?)

Caused by:
    0: error sending request for url (https://raw.githubuserconxtent.com/mullvad/mullvadvpn-app/refs/tags/2123.1/desktop/packages/mullvad-vpn/changes.txt)
    1: client error (Connect)
    2: dns error: failed to lookup address information: Name or service not known
    3: failed to lookup address information: Name or service not known

Oh. reqwest includes it? Sweet, didn't know that.

---

mullvad-update/meta/src/main.rs line 107 at r2 (raw file):
⛏️ Imo, a help text directed towards the user shouldn't refer to them in third-person.

Suggestion:

Do not prompt for confirmation when overwriting signed files

---

mullvad-update/meta/src/platform.rs line 220 at r2 (raw file):

        create_dir_and_write(&signed_path, signed_bytes)
            .await
            .context("Failed to write signed data")?;

Include path in the context?

---

mullvad-update/meta/src/io_util.rs line 25 at r2 (raw file):

            } else {
                println!(" [y/N]");
            }

⛏️ Ideally, stdin should be drained before rendering the prompt

---

mullvad-update/meta/src/io_util.rs line 76 at r2 (raw file):

        .context("Failed to create directories")?;

    fs::write(path, contents).await?;

⛏️ .context?

Also, ideally, all of the contexts should include path. I admit it's annoying to add it everywhere though 😅

---

mullvad-update/meta/src/main.rs line 24 at r2 (raw file):

/// Metadata expiry to use when not specified (months from now)
#[allow(dead_code)]
const DEFAULT_EXPIRY_MONTHS: usize = 6;

dead code?

---

mullvad-update/meta/src/main.rs line 73 at r2 (raw file):

        platforms: Vec<Platform>,
        /// Rollout percentage (default is 1)
        #[arg(long)]

How come default_value isn't used here?

---

mullvad-update/meta/src/main.rs line 74 at r2 (raw file):

        /// Rollout percentage (default is 1)
        #[arg(long)]
        rollout: Option<f32>,

Does 1 mean 100% or 1%?

---

mullvad-update/src/format/mod.rs line 45 at r2 (raw file):

/// Signed JSON response, not including the signature
#[derive(Default, Debug, Deserialize, Serialize)]

Does it make sense for this to impl default? DateTime becomes unix epoch which may not be intuitive.

[hulthe]

Reviewed 2 of 4 files at r2.
Reviewable status: 16 of 19 files reviewed, 9 unresolved discussions (waiting on @dlon)

[dlon]

Reviewable status: 13 of 19 files reviewed, 5 unresolved discussions (waiting on @hulthe)

---

mullvad-update/meta/src/io_util.rs line 76 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

⛏️ .context?

Also, ideally, all of the contexts should include path. I admit it's annoying to add it everywhere though 😅

Done!

---

mullvad-update/meta/src/main.rs line 24 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

dead code?

Thanks, removed

---

mullvad-update/meta/src/main.rs line 73 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

How come default_value isn't used here?

It should be. Fixed!

---

mullvad-update/meta/src/main.rs line 74 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Does 1 mean 100% or 1%?

100%. Updated to clarify

---

mullvad-update/meta/src/main.rs line 107 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

⛏️ Imo, a help text directed towards the user shouldn't refer to them in third-person.

Suggestion:

Do not prompt for confirmation when overwriting signed files

Done.

---

mullvad-update/meta/src/platform.rs line 220 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Include path in the context?

It's already logged above, so it might not be helpful here?

---

mullvad-update/src/format/mod.rs line 45 at r2 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Does it make sense for this to impl default? DateTime becomes unix epoch which may not be intuitive.

That's exactly what I want! Would it be fine to simply document that?

[hulthe]

Reviewed 1 of 2 files at r3, 2 of 3 files at r4, 5 of 5 files at r5.
Reviewable status: 18 of 19 files reviewed, 1 unresolved discussion (waiting on @dlon)

---

mullvad-update/meta/src/platform.rs line 220 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

It's already logged above, so it might not be helpful here?

👍

---

mullvad-update/src/format/mod.rs line 45 at r2 (raw file):

Previously, dlon (David Lönnhager) wrote…

That's exactly what I want! Would it be fine to simply document that?

You don't have to do anything, I was just sanity-checking :)

[hulthe]

Reviewed 1 of 19 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dlon)

---

desktop/scripts/release/4-make-release line 66 at r5 (raw file):

    echo ">>> Backing up released data"
    cp -r signed/ published_signed/

Is "publish_signed" the backup? Are we publishing the backup?

[dlon]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @hulthe)

---

desktop/scripts/release/4-make-release line 66 at r5 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Is "publish_signed" the backup? Are we publishing the backup?

It's a backup of the last published data that was just pulled, which is used for showing a diff in a later step. Will think of a better name :)

[dlon]

I suggest we merge this after the main PR to avoid ballooning that further.

[hulthe]

Reviewed 5 of 5 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dlon)

---

desktop/scripts/release/4-make-release line 66 at r5 (raw file):

Previously, dlon (David Lönnhager) wrote…

It's a backup of the last published data that was just pulled, which is used for showing a diff in a later step. Will think of a better name :)

👍

The base branch was changed.

[MarkusPettersson98]

Reviewed 4 of 4 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dlon)