Skip to content

Drop 32bit Windows release support#7804

Merged
The-Compiler merged 1 commit intomainfrom
drop-32bit-windows
Aug 10, 2023
Merged

Drop 32bit Windows release support#7804
The-Compiler merged 1 commit intomainfrom
drop-32bit-windows

Conversation

@The-Compiler
Copy link
Copy Markdown
Member

See #6050, still keeping that open to track NSIS changes

See #6050, still keeping open to track NSIS changes
@toofar
Copy link
Copy Markdown
Member

toofar commented Aug 2, 2023

@The-Compiler can you give some guidance on what installer changes are required, if any? Just to change this conditional? (Maybe to turn it into this?)
Out of those variables I think ARCH can be cleaned up but I think PLATFORM is needed to check for compatibility and SUFFIX seems to be used for the InternalName version key, not sure if that needs to be stable across versions or not.

@The-Compiler
Copy link
Copy Markdown
Member Author

@toofar FWIW I decided to keep the installer changes out of here to avoid more conflicts with #7315. I think if we're not building a 32bit installer, keeping the additional code in there doesn't really hurt. Are you okay with merging this as-is?

Copy link
Copy Markdown
Member

@toofar toofar left a comment

Choose a reason for hiding this comment

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

Yup, merge away!

Sorry if it looked like I was passively aggressively blocking it by commenting without approving or disproving. I had just glanced at it then some-elses-problemed it.
I can see that the nsis installer defaults to x64 and you are removing the 32bit CI jobs, removing the now redundant x64 identifiers and any related conditionals.

LGTM

@The-Compiler The-Compiler merged commit 937b242 into main Aug 10, 2023
@The-Compiler The-Compiler deleted the drop-32bit-windows branch August 10, 2023 10:10
toofar added a commit that referenced this pull request Aug 12, 2023
We dropped 32bit support in #7804 and as a result removed the arch
suffix from the binary that pyinstaller produces. This commit removes it
form the lookup path in the installer too.

Note that we are leaving the arch string in the installer itself for
now. Mostly because it'll be removed as part of a later change when the
installer itself is refreshed. But it might also be useful to clarify in
the installer names what the arch is? Maybe, that reasoning might not
fit with the previous change to remove the arch strings.
toofar added a commit that referenced this pull request Aug 12, 2023
We dropped 32bit support in #7804 and as a result removed the arch
suffix from the binary that pyinstaller produces. This commit removes it
form the lookup path in the installer too.

Note that we are leaving the arch string in the installer itself for
now. Mostly because it'll be removed as part of a later change when the
installer itself is refreshed. But it might also be useful to clarify in
the installer names what the arch is? Maybe, that reasoning might not
fit with the previous change to remove the arch strings.
toofar added a commit that referenced this pull request Aug 13, 2023
We dropped 32bit support in #7804 and as a result removed the arch
suffix from the binary that pyinstaller produces. This commit removes it
form the lookup path in the installer too.

Note that we are leaving the arch string in the installer itself for
now. Mostly because it'll be removed as part of a later change when the
installer itself is refreshed. But it might also be useful to clarify in
the installer names what the arch is? Maybe, that reasoning might not
fit with the previous change to remove the arch strings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

2 participants