Skip to content

Finish apply_dist() #1730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

Finish apply_dist() #1730

wants to merge 26 commits into from

Conversation

duckinator
Copy link
Contributor

@duckinator duckinator commented Jan 28, 2025

EDIT (2025-03-13): I was reminded of this PR earlier today, and wanted to leave a quick note for anyone who may try picking up this work in the future.

You can definitely split this up into smaller PRs, I just didn't have the time to do so before my contract ended. You will likely need to mark things with #[allow(dead_code)] for it to be mergeable.

This PR is functional and has unit tests for each part that was split into its own file.

Since I tried to make each file self-contained, you can make PRs doing the following:

  1. Add pretty_assertions dep (Cargo.toml, Cargo.lock, cargo-dist/Cargo.toml)
  2. Add cargo-dist/src/init/apply_dist/helpers.rs
  3. Add cargo-dist/src/init/apply_dist/artifacts.rs
  4. Add cargo-dist/src/init/apply_dist/builds.rs
  5. Add cargo-dist/src/init/apply_dist/ci.rs
  6. Add cargo-dist/src/init/apply_dist/hosts.rs
  7. Add cargo-dist/src/init/apply_dist/installers.rs
  8. Add cargo-dist/src/init/apply_dist/publishers.rs
  9. Add cargo-dist/src/init/apply_dist/mod.rs + remove cargo-dist/src/init/apply_dist.rs
  10. Update cargo-dist/src/init/mod.rs.

If you do that, and they all are merged, you can rebase #1763 off main and the diff will be way smaller.


original description

NOTE: The errors are due to dist::init::apply_dist::apply() never being called outside of the test suite.


This PR is functional and has unit tests for each part that was split into its own file. I knew this PR was going to be distressingly large, so I tried to design it so each part could be treated entirely self-contained.

The exception is cargo-dist/src/init/apply_dist/mod.rs, which pulls everything else in that directory together.

There's also cargo-dist/src/init/mod.rs, but that's tiny.


There is definitely room for refactoring in some spots, because I learned a lot of tricks as I got farther into it. I don't think it's worth adding even more churn to this PR, though.

Detailed list of submodules, which I used to track my progress.

Each of the following submodules under dist::init::apply_dist handle all configuration values:

  • artifacts
    • archives (ArchiveLayer)
      • include
      • auto_includes
      • windows_archive
      • unix_archive
      • package_libraries
    • source_tarball
    • extra (deliberately not done)
    • checksum
    • tests
  • builds
    • common (CommonBuildLayer; empty struct)
    • ssldotcom_windows_sign
    • macos_sign
    • cargo (BoolOr CargoBuildLayer)
    • generic (BoolOr GenericBuildLayer)
    • system_dependencies (SystemDependencies)
    • min_glibc_version
    • omnibor
    • tests
  • ci
    • common (CommonCiLayer)
      • merge_tasks
      • fail_fast
      • cache_builds
      • build_local_artifacts
      • dispatch_releases
      • release_branch
      • pr_run_mode
      • tag_namespace
      • plan_jobs
      • build_local_jobs
      • build_global_jobs
      • host_jobs
      • publish_jobs
      • post_announce_jobs
    • github
      • common (CommonCiLayer)
      • runners (deliberately not done)
      • permissions (deliberately not done)
      • build_setup
    • tests
  • hosts
    • common (CommonHostLayer)
    • force_latest
    • display
    • display_name
    • github (BoolOr GithubHostLayer)
    • axodotdev (BoolOr AxodotdevHostLayer)
    • tests
  • installers
    • common (CommonInstallerLayer)
      • install_path
      • install_success_msg
      • install_libraries
      • bin_aliases (deliberately not done)
    • homebrew (BoolOr HomebrewInstallersLayer)
      • Bool
      • common
      • tap
      • formula
    • msi (BoolOr MsiInstallerLayer)
      • Bool
      • common
    • npm (BoolOr NpmInstallerLayer)
      • Bool
      • common
      • package
      • scope
    • powershell (BoolOr PowershellInstallerLayer)
      • Bool
      • common
    • shell (BoolOr ShellInstallerLayer)
      • Bool
      • common
    • pkg (BoolOr PkgInstallerLayer)
      • Bool
      • common
      • identifier
      • install_location
    • updater
    • always_use_latest_updater
    • tests
  • publishers
    • common (CommonPublisherLayer)
      • prereleases
    • homebrew (BoolOr HomebrewPublisherLayer)
      • common
    • npm (BoolOr NpmPublisherLayer)
      • common
    • tests

@duckinator duckinator changed the title [WIP] Finish apply_dist() (WIP) Finish apply_dist() Jan 29, 2025
@duckinator duckinator force-pushed the apply-dist branch 4 times, most recently from e3bdcf2 to 1b6e08b Compare February 5, 2025 19:51
@duckinator duckinator force-pushed the apply-dist branch 2 times, most recently from 53eb226 to d1b13c2 Compare February 10, 2025 18:53
@duckinator duckinator marked this pull request as ready for review February 13, 2025 23:05
@duckinator duckinator changed the title (WIP) Finish apply_dist() Finish apply_dist() Feb 14, 2025
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.

1 participant