-
-
Notifications
You must be signed in to change notification settings - Fork 520
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
fix: Prevent conflict between parallel makers #3519
base: main
Are you sure you want to change the base?
Conversation
At the moment maker-dmg will simultaneously try to write to `out/make/AppName.dmg`. This commit changes that to `out/make-darwin-arm64/AppName.dmg` preventing any conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pretty rough breaking change and IMO not how we want to fix this.
Potentially it would be possible to get maker-dmg to generate a DMG with the required name from the start instead of always generating it with name.dmg and then renaming to name-version-arch.dmg.
This sounds more reasonable
Someone asked me about this on Discord last week and I agree with the above. As you stated in the downsides section in the description, I think this too likely to catch people off-guard. |
@MarshallOfSound the fallback I mentioned won't work if the user supplies a name via config (though the may have been broken for a long time when building for multiple arches, not sure). For now I think it's out of scope of this PR to fix that, but I'll leave that up to you. current: {
name: "@electron-forge/maker-dmg",
config: {
name: "AppName",
overwrite: true,
},
platforms: ["darwin"],
}, (this would break because we can't write two AppName.dmg to the same out/make folders. I believe this would have broken before this PR and before 7.0 as well though) Proposed ability to pass function {
name: "@electron-forge/maker-dmg",
config: {
name: (version: string, platform: string, arch: string) => `AppName-${arch}-${platform}`,
overwrite: true,
},
platforms: ["darwin"],
} (idk if we'd want version or not, the user can get that themselves) |
Updated the PR, also removed the two tests that tested renaming as we don't need to rename anymore. |
@MarshallOfSound is there any chance you could take another look? |
Updated to resolve merge conflicts |
How about MakerSquirrel or MakerAppImage to solve this problem?
|
At the moment maker-dmg will simultaneously try to write to
out/make/AppName.dmg
. This commit changes that toout/make-darwin-arm64/AppName.dmg
preventing any conflictsSummarize your changes:
at the moment maker-dmg fails when run in parallel, for an explanation on why / how / the details see here: #3517
This solution:
We pass a different path into the maker from core for each platform & arch combination, which prevents it from trying to use the same file in an intermediary step.
Downsides:
People might rely on the location of the dmg & zip to be in
out/make
directly, this would obviously ruin that.Alternatives considered:
Potentially it would be possible to get maker-dmg to generate a DMG with the required name from the start instead of always generating it with
name.dmg
and then renaming toname-version-arch.dmg
.code for context
maker-dmg/src/MakerDMG.ts