[WIP] Implement config v0 -> v1 migration. #1653

duckinator · 2024-12-18T20:56:16Z

Replaced by:

(config-migration) Add BoolOr<T> and Option<BoolOr<T>> helper functions. #1697
(config-migration) Implement Display for ChecksumStyle, derive Default for ArtifactLayer and InstallerLayer #1698
(config-migration) Add the ability to determine and track the config version. #1699
(config-migration) Add config::v0::load() and friends #1701
(config-migration) Add validate_install_paths() and merge_workspace_config() to TomlLayer #1703
(config-migration) Don't modify [profile.dist] until after migrations; consolidate migrations under one prompt. #1704
(config-migration) Make dist::init and dist::tasks into folder-based modules #1705
(config-migration) Move v0-specific functions into dist::config::v0. #1706
(config-migration) Move src/init/{mod -> v0}.rs, add new src/init/mod.rs to re-export functions #1717
(config-migration) Split dist::migrate out from dist::init #1718
(config-migration) Move root_workspace + workspace_toml to where they're used; remove unneeded desired_workspace_kind #1719
(config-migration) Split parts out of dist::init::v0 into other dist::init submodules #1721
(config-migration) Add dist::config::load() and friends #1723
(config-migration) Add config::version::want_v1() and empty do_migrate_from_v0() #1724
(config-migration) Add do_migrate_from_v0() implementation #1725
(config-migration) Split init_dist_profile() out into its own file #1726
(config-migration) Add dist::init::apply_dist #1727
Finish apply_dist() #1730
WIP v1 init (for real this time i promise) #1763

Required functionality for this PR:

make dist migrate move v0 to v1
- Have do_migrate_from_v0() actually copy workspace members to the new configuration.
make dist init use v1
make everything but dist init load v1
make sure dist plan (and other commands) gives an actionable error for v0 configs

Current state (2025-01-10):

duckinator · 2024-12-20T17:15:39Z

This is currently based off #1661. When that gets merged, I'll rebase off main.

duckinator · 2025-01-07T20:58:08Z

The various apply_*() functions should compile now, and some of it will actually be updated properly.

TODO:

get_new_dist_metadata()
finish up the apply_*() functions

mistydemeo · 2025-01-07T22:40:19Z

We were discussing get_new_dist_metadata during the call. I'll summarize what it does.

This is the interactive portion of init. It's used in two code paths: a newly-initted system, in which case we have no existing config, and updating existing configuration. If there's existing config, the interactive portions are designed to prefill the choice with the decisions that had already been made the last time; this ensures that mashing enter through the settings preserves the user's existing config without changing anything.

Unlike most of the rest of init, which operates on toml_edit documents, get_new_dist_metadata operates on a DistMetadata struct. It parses the existing config to DistMetadata, edits it in place, and returns the altered document. If there's no existing configuration, it instead initializes a completely empty DistMetadata.

Because this code is working from and generating a DistMetadata, we might not actually have to do any changes at all for this branch beyond swapping to the v1 version of DistMetadata.

For each bit of user-selectable config, we use the dialoguer crate to prompt the user for their input unless the user passed the --yes/-y flag, in which case we pick defaults/their existing config without prompting.

…onfigs.

…rship errors instead of type errors

…space

…gs." This reverts commit 46dc61d.

…n DistGraphBuilder

…ommonInstallerLayer.

mistydemeo

Left a comment on the as-yet incomplete items in init.rs, just to track them, along with a couple of things to keep an eye on testing.

Aside from the remaining init.rs changes, I think the only other remaining work is in the tests. All of our tests are written in the old config format, and are specifically using Cargo.toml, so they need to be updated to use dist-workspace.toml in the new config format in order to pass.

cargo-dist/src/init.rs

mistydemeo · 2025-01-10T23:53:00Z

cargo-dist/src/init.rs

+        publishers,
    } = &meta;

+    /*


This whole section needs to be uncommented and reimplemented for the new structure.

mistydemeo · 2025-01-11T00:20:46Z

cargo-dist/src/init.rs

+
+    apply_installers_common(npm_table, &npm.common);
+
+    // TODO: similar to shell


Impl missing here.

mistydemeo · 2025-01-11T00:21:22Z

cargo-dist/src/init.rs

+
+    apply_installers_common(powershell_table, &powershell.common);
+
+    // TODO: similar to shell


Impl missing here.

mistydemeo · 2025-01-11T00:21:44Z

cargo-dist/src/init.rs

+
+    apply_installers_common(shell_table, &shell.common);
+
+    // TODO


Impl missing here.

cargo-dist/src/init.rs

mistydemeo · 2025-01-11T00:23:56Z

cargo-dist/src/init.rs

+        panic!("Expected [dist.publishers] to be a table");
+    };
+
+    // ...


Impl missing here.

…host the assets. NOTE: pkg used to only be shown if it was already configured. However, since we have an outstanding PR for finishing up pkg support.

…ional_mac_pkg

cargo-dist/src/config/mod.rs

cargo-dist/src/config/v1/layer.rs

duckinator · 2025-01-13T20:41:26Z

cargo-dist/src/tasks.rs

                pkg_idx,
                workspace_layer.clone(),
-                package_metadata.to_toml_layer(false),
+                package_metadata.clone(), // fixme: this seems suspect since we call seemingly-mutating functions on it later


this is probably fine because package_metadata.to_toml_layer(false) returned a new object anyway?

duckinator · 2025-01-13T20:46:50Z

cargo-dist/src/init.rs

-    eprintln!("let's setup your dist config...");
-    eprintln!();
-
-    // For each [workspace] Cargo.toml in the workspaces, initialize [profile]
-    let mut did_add_profile = false;
-    for workspace_idx in workspaces.all_workspace_indices() {
-        let workspace = workspaces.workspace(workspace_idx);
-        if workspace.kind == WorkspaceKind::Rust {
-            let mut workspace_toml = config::load_toml(&workspace.manifest_path)?;
-            did_add_profile |= init_dist_profile(cfg, &mut workspace_toml)?;
-            config::write_toml(&workspace.manifest_path, workspace_toml)?;
-        }
-    }
-
-    if did_add_profile {
-        eprintln!("{check} added [profile.dist] to your workspace Cargo.toml");
-    }
-


This is moved later in the function, to avoid dist init being canceled but still overwriting Cargo.toml.

…ith}.

duckinator · 2025-02-14T21:53:32Z

This has been replaced by 19 other PRs.

duckinator force-pushed the dist-migrate-v1 branch 4 times, most recently from ce26547 to 9a7d939 Compare December 20, 2024 17:15

duckinator force-pushed the dist-migrate-v1 branch from 9a7d939 to 8670bcb Compare December 20, 2024 17:54

duckinator force-pushed the dist-migrate-v1 branch from bd40eca to 101bc69 Compare January 6, 2025 21:12

duckinator added 4 commits January 8, 2025 11:02

Have "dist init" only migrate from Cargo.toml configs to v0 generic c…

953b2db

…onfigs.

Make "dist migrate" convert from v0 to v1 config.

e1576c3

[WIP, BROKEN] start making "dist init" use v1 config

0b7f6d8

[WIP, BROKEN] apply_*() at least seem like they'll compile now

9f39ac6

duckinator force-pushed the dist-migrate-v1 branch from 4f99c57 to 9f39ac6 Compare January 8, 2025 16:02

duckinator and others added 16 commits January 8, 2025 14:48

Add BoolOr::truthy() to simplify logic elsewhere.

1d9f415

derive Default for InstallerLayer.

0999c3a

Migrate first chunk of get_new_dist_metadata() to v1 config

f9527bf

derive Default for PublisherLayer.

b0ff20a

made enough progress on get_new_dist_metadata() that i'm getting owne…

942ec4e

…rship errors instead of type errors

[formatting cleanup] clean up /* and */ indentation levels, and white…

05867a5

…space

formatting cleanup

4cf359e

initial stab at cleaning up ownership etc

441902c

Have "dist init" run do_migrate() if using a v0 config.

3434474

Have "dist generate" print an error if you have a v0 config.

c376686

[WIP] attempt to migrate "dist generate" to v1 config

25eab8c

have is_v0_config() return false when trying to load v1 configs.

46dc61d

Revert "have is_v0_config() return false when trying to load v1 confi…

9abf07c

…gs." This reverts commit 46dc61d.

Use try_load_config() instead of parse_metadata_table_or_manifest() i…

ddef86e

…n DistGraphBuilder

??? third ??? attempt at making is_v0_config work right

77872aa

Try being more precise about config versions

d833cd9

Have "dist init" add comments under [dist.installers] for things in C…

83a7619

…ommonInstallerLayer.

mistydemeo mentioned this pull request Jan 10, 2025

feat: pkg followups #1381

Open

mistydemeo reviewed Jan 11, 2025

View reviewed changes

mistydemeo mentioned this pull request Jan 11, 2025

dist rename rollout plan #1481

Open

11 tasks

duckinator added 11 commits January 13, 2025 12:26

add TODO(migration) comments for tracking what work needs to be done

99d1d02

Remove dead code.

52d1caf

Only have "dist init" show online installers if we have somewhere to …

3e6c407

…host the assets. NOTE: pkg used to only be shown if it was already configured. However, since we have an outstanding PR for finishing up pkg support.

removed now-unused argument to apply_dist_to_workspace_toml().

6ddf8a0

move pkg to offline/bundled installers; remove now-unneeded apply_opt…

19b73f6

…ional_mac_pkg

Remove unneeded code.

61765c2

Finish apply_installers_homebrew().

896a4f8

remove commented-out code

849b1b5

apply_artifacts()

86292bc

fix apply_installers_homebrew()

6bc0d1c

resolve clippy warnings

0dd9e54

duckinator commented Jan 13, 2025

View reviewed changes

cargo-dist/src/config/mod.rs Show resolved Hide resolved

duckinator commented Jan 13, 2025

View reviewed changes

cargo-dist/src/config/v1/layer.rs Outdated Show resolved Hide resolved

duckinator commented Jan 13, 2025

View reviewed changes

duckinator added 8 commits January 14, 2025 13:10

Rename BoolOr::truthy() to BoolOr::not_false()

8061922

Add BoolOrOptExt, implementing Option<BoolOr<T>>::none_or_false()

1684847

re-enable inlining the default install_path if not specified

8f76407

minor cleanup

7f746ba

finish implementing apply_installers_npm()

9ef9e4e

finish apply_installers_msi()

b045a9d

Add Option<BoolOr<T>>::is_some_and_not_false(), use get_or_insert{,_w…

b9489c8

…ith}.

use is_some_and_not_false() more

3901685

duckinator closed this Feb 14, 2025

duckinator deleted the dist-migrate-v1 branch February 14, 2025 21:53


		apply_installers_common(npm_table, &npm.common);

		// TODO: similar to shell


		apply_installers_common(powershell_table, &powershell.common);

		// TODO: similar to shell

[WIP] Implement config v0 -> v1 migration. #1653

[WIP] Implement config v0 -> v1 migration. #1653

Uh oh!

Conversation

duckinator commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

duckinator commented Dec 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

duckinator commented Jan 7, 2025

Uh oh!

mistydemeo commented Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mistydemeo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mistydemeo Jan 10, 2025

Choose a reason for hiding this comment

Uh oh!

mistydemeo Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

mistydemeo Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

mistydemeo Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mistydemeo Jan 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

duckinator Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duckinator Jan 13, 2025

Choose a reason for hiding this comment

Uh oh!

duckinator commented Feb 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

duckinator commented Dec 18, 2024 •

edited

Loading

duckinator commented Dec 20, 2024 •

edited

Loading

mistydemeo commented Jan 7, 2025 •

edited

Loading

duckinator Jan 13, 2025 •

edited

Loading