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

Add logic to detect ARM updates from the appcast file. #266

Merged
merged 3 commits into from
Dec 31, 2023

Conversation

tmiw
Copy link
Contributor

@tmiw tmiw commented Dec 27, 2023

This is a followup to #265 to enable WinSparkle to allow users to specifically provide ARM64 releases in the appcast file.

@vslavik
Copy link
Owner

vslavik commented Dec 28, 2023

Thanks a lot!

I'd call it a follow-up to #50 instead (noting to get cross-referenced from there). Will also need to update https://github.com/vslavik/winsparkle/wiki/Appcast-Feeds accordingly after merging...

src/appcast.cpp Outdated
@@ -212,10 +213,14 @@ bool is_suitable_windows_item(const Appcast &item)

// Check suffix for matching bitness
#ifdef _WIN64
#ifdef __AARCH64EL__ || _M_ARM64
Copy link
Owner

Choose a reason for hiding this comment

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

This is incorrect, it needs to be #if defined(__AARCH64EL__) || defined(_M_ARM64).

Also, where does __AARCH64EL__ come from? Can't find any reference to in relation to Windows...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

__AARCH64EL__ is defined by LLVM MinGW:

parallels@ubuntu-linux-22-04-desktop:~/winsparkle/build$ aarch64-w64-mingw32-clang -E -dM - </dev/null | grep AARCH64
#define __AARCH64EL__ 1
#define __AARCH64_CMODEL_SMALL__ 1
parallels@ubuntu-linux-22-04-desktop:~/winsparkle/build$

Unfortunately it doesn't look like that version of MinGW defines _M_ARM64, so we can't simply check for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, ifdef has been changed to if defined() per your above comment.

@vslavik vslavik merged commit 089ad75 into vslavik:master Dec 31, 2023
6 checks passed
@vslavik
Copy link
Owner

vslavik commented Dec 31, 2023

Thanks, merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants