Skip to content
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

pulumi: 3.122.0 -> 3.152.0 #382594

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

pulumi: 3.122.0 -> 3.152.0 #382594

wants to merge 11 commits into from

Conversation

tie
Copy link
Member

@tie tie commented Feb 16, 2025

This PR updates Pulumi to the latest version (pulumi/pulumi@v3.122.0...v3.152.0) and fixes cross-compilation.

Refactor and version bumps for pulumiPackages will be submitted in a follow up PRs.

Old description

https://github.com/NixOS/nixpkgs/compare/master...tie:nixpkgs:pulumi-packages-bump?expand=1

This PR updates Pulumi to the latest version (pulumi/pulumi@v3.122.0...v3.152.0), along with significant refactor and updates for pulumiPackages. This PR fixes cross-compilation for Pulumi packages and uses correct Python package set for SDKs. All packages now have passthru.updateScript to ease updates in the future.

We also remove mkPulumiPackage helper function and use buildGoModule directly. While Pulumi packages have similar architecture, there are minor differences in build logic that would otherwise require a lot of special cases. Note that mkPulumiPackage was never exposed so it is a backwards compatible change.

There is an existing PR #352221, but it’s already outdated and seems to be unfinished (e.g. some packages fail to build).

Note that Pulumi requires protobuf4 for Python packages. See also #351751 (comment)

Build all Pulumi packages, SDKs and tests with:

nix build --impure --expr 'with import ./. {
  config.allowAliases = false;
  overlays = [
    (_: prev: {
      pythonPackagesExtensions = prev.pythonPackagesExtensions ++ [
        (final: _: {
          protobuf = final.protobuf4;
        })
      ];
    })
  ];
};
linkFarmFromDrvs "pulumi-packages" (
  lib.concatLists (
    lib.mapAttrsToList (
      _: p:
      lib.optionals (lib.isDerivation p) (
        [ p ] ++ lib.attrValues (p.sdks or { }) ++ lib.attrValues (p.tests or { })
      )
    ) (pulumiPackages // { inherit pulumi; })
  )
)'

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first commit is really hard to review, because so many different changes are in one commit. I see:

  • addition of maintainer
  • refactors
  • version update

It would be good to split things up into multiple commits, to create smaller diffs.

@tie tie force-pushed the pulumi-bump branch 5 times, most recently from 54818ee to 0e44ab3 Compare February 20, 2025 03:17
@tie tie marked this pull request as ready for review February 20, 2025 03:26
@tie
Copy link
Member Author

tie commented Feb 20, 2025

@wolfgangwalther, I’ve recreated commit history from the final revision, so this is hopefully easier to review commit-by-commit.

@tie tie requested a review from wolfgangwalther February 20, 2025 03:28
@nix-owners nix-owners bot requested a review from natsukium February 20, 2025 03:28
Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked through the first 10 or so commits and then suddenly there were new packages introduced etc...

This PR seriously needs to be split up in general. 3k lines of code is too much to review - and the PR title also doesn't match anymore. This is not an update... this is rebuilding the whole pulumi package set from scratch.

I suggest you start with a first PR which just adds you as maintainer and does all kinds of modernization, ideally pure refactors - those will already get rid of quite some lines and are easy to review, because you won't have any rebuilds. This could also contain the moves to pkgs/by-name etc.

Then the next step would be everything that is required for the version bump (s).

And then something to improve the infrastructure around the package set, all the helpers etc.

And then one or more PRs to introduce new packages.

Or some other split entirely - but 3k lines is a lot at once.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most changes to the pulumi-langauge-xxx files look unrelated to the actual update and more like refactors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are small cleanups, so I think we can bundle them with the update. I’ve also split them into separate commits in the latest revision.

@tie tie changed the title pulumi: 3.122.0 -> 3.150.0 pulumi: 3.122.0 -> 3.152.0 Feb 23, 2025
@tie tie force-pushed the pulumi-bump branch 2 times, most recently from ba244e3 to eee48c7 Compare February 23, 2025 17:14
@tie tie force-pushed the pulumi-bump branch 2 times, most recently from 376ad22 to 7e3961c Compare February 23, 2025 17:26
@tie
Copy link
Member Author

tie commented Feb 23, 2025

@wolfgangwalther, updated the PR to only include version bump for pulumi package. I’ll submit all other changes in follow up PRs.

@tie tie requested a review from wolfgangwalther February 23, 2025 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants