-
Notifications
You must be signed in to change notification settings - Fork 376
Account for target installer arch in mac pkg id generation #7831
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 the right change to make, but it means that ARM64 installations are not going to upgrade well WRT removing the old files. We might need to document that those should be manually cleaned up if folks wish to do so.
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.
Maybe also consider/test what'll happen if someone installs x64, then installs arm64, then upgrades x64 to the next release.
And perhaps a more interesting subscenario in the future: what if the first x64 installer installs to
/usr/local/share/dotnet, the arm64 installer replaces some files, and then the second x64 installer has the logic to install to/usr/local/share/dotnet/x64? (Would the x64 upgrade delete files from the arm64 install because it thinks the old x64 installer was the one to place them down? I'm not familiar with how these files are tracked.)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.
I believe we scoped out upgrade from pre x64-on-ARM64 installers, since the assumption is most of the ARM64 machines will be new installs. @richlander is that correct? It still may be interesting to understand what happens since that could be a state folks get into.