Skip to content

Conversation

iangmaia
Copy link
Contributor

@iangmaia iangmaia commented Jul 23, 2025

To fix the error seen in Automattic/studio#1594 (build), this PR implemented:

  • Python installation during the Windows setup so that it can be used by npm install scripts
  • Addition of native compilation components to Visual Studio Build Tools

Being tested via Automattic/studio#1608


  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

@iangmaia iangmaia requested a review from a team July 23, 2025 20:17
@iangmaia iangmaia self-assigned this Jul 23, 2025
@iangmaia iangmaia added the enhancement New feature or request label Jul 23, 2025
@iangmaia iangmaia force-pushed the iangmaia/add-node-native-modules branch from 19d407b to f80bd61 Compare July 24, 2025 10:42
@iangmaia iangmaia force-pushed the iangmaia/add-node-native-modules branch from d8b34b1 to 5e96155 Compare July 24, 2025 15:01
@iangmaia iangmaia force-pushed the iangmaia/add-node-native-modules branch from b78c34d to fbdc993 Compare July 24, 2025 17:19
@iangmaia iangmaia force-pushed the iangmaia/add-node-native-modules branch from fbdc993 to fcebed7 Compare July 24, 2025 17:49
@iangmaia iangmaia force-pushed the iangmaia/add-node-native-modules branch from b9fcf03 to c4f50c9 Compare July 24, 2025 19:59
@iangmaia iangmaia requested a review from AliSoftware July 24, 2025 22:47
@iangmaia
Copy link
Contributor Author

@AliSoftware I think I addressed all the points. I struggled a bit with the unit tests later but everything seems to be finally working now 😌

$certificatePfxPath = "certificate.pfx"
certutil -decode $certificateBinPath $certificatePfxPath
Write-Output "Code signing certificate downloaded at: $((Get-Item $certificatePfxPath).FullName)"
if ($env:SKIP_CERTIFICATE_DOWNLOAD -eq "true") {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm guessing the reason why this is controlled by an env var and not a flag (aka param { [switch] … }) like the others is because the skipping is only used for the tests so far?

But that made me wonder if the studio jobs running the Unit Tests might also not need the cert and want to skip it 🤔 💭

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is only used for the tests so far?

Yes, that's the case so far. Also, this seemed more like a configuration rather than an API argument so the env var felt more appropriate.

if the studio jobs running the Unit Tests might also not need the cert

Good question. Imagining it still compiles the tested app in some way, perhaps they're still needed? But we can definitely try 👍

@iangmaia iangmaia merged commit 3c82b28 into trunk Jul 25, 2025
27 checks passed
@iangmaia iangmaia deleted the iangmaia/add-node-native-modules branch July 25, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants