Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

make cmake work on macos#2060

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fpistm merged 1 commit intostm32duino:mainfromnathantsoi:cmake_macos
Jul 11, 2023
Merged

Conversation

nathantsoi
Copy link
Contributor

Summary

cmake's downloads (ctags and toolchain) were not working on macos.

Validation

tested on mac os 13.4.1 and cmake 3.26.4

@nathantsoi
Copy link
ContributorAuthor

nathantsoi commentedJul 10, 2023
edited
Loading

One caveat here. I found that thectags for macOS on the Arduino github releases page is not 64-bit, however, the ctags binary distributed in the Arduino app is... so I also had to:

cp /Applications/Arduino.app/Contents/Java/tools-builder/ctags/5.8-arduino11/ctags ~/.Arduino_Core_STM32_dl/d726370579d207be3e43211ba220e5b1/dist/ctags

I'm not sure if a 64-bit ctags binary is available outside of the Arduino installer, but this would be a better solution.

@fpistmfpistm requested a review frommassonalJuly 10, 2023 05:32
@massonal
Copy link
Contributor

Fromcmake_host_system_information, redirecting toCMAKE_HOST_SYSTEM_NAME:

Name of the OS CMake is running on.

On systems that have the uname command, this variable is set to the output of uname -s. Linux,
Windows, and Darwin for macOS are the values found on the big three operating systems.

Did you really observe the value "macOS"?! (That's fine, but it would mean the docs are confusing...)

As for the caveat you outline:ctags-5.8-arduino11-x86_64-apple-darwin.zip is what you're looking for, no? Or do you mean they packaged a 32-bits app with a 64-bits name?

@fpistmfpistm added the waiting feedbackFurther information is required labelJul 10, 2023
@fpistm
Copy link
Member

As an extra information: which Mac-OS version you used ?

@nathantsoi
Copy link
ContributorAuthor

MacOS version is Ventura13.4.1

And yes,@massonal, thex86_64 zip contains an i386 executable:

$ wget https://github.com/arduino/ctags/releases/download/5.8-arduino11/ctags-5.8-arduino11-x86_64-apple-darwin.zip$ unzip ctags-5.8-arduino11-x86_64-apple-darwinArchive:  ctags-5.8-arduino11-x86_64-apple-darwin.zip  inflating: ctags$ file ctagsctags: Mach-O executable i386

vs. The correct, 64-bit version included with the arduino app:

$ file /Applications/Arduino.app/Contents/Java/tools-builder/ctags/5.8-arduino11/ctags/Applications/Arduino.app/Contents/Java/tools-builder/ctags/5.8-arduino11/ctags: Mach-O 64-bit executable x86_64

@fpistm
Copy link
Member

@nathantsoi you don't answer this question from@massonal

Did you really observe the value "macOS"?! (That's fine, but it would mean the docs are confusing...)

On our side a MacBook Pro with Monterey v12.6.1 is used to validate and result ofuname -s isDarwin.
It seems strange the result was changed and CMAKE documentation was not updated.
Looking on Ventura, it is always based on Darwin kernel. Ventura version 13.4.1 is based on 22.5.0 Darwin kernel.

@nathantsoi
Copy link
ContributorAuthor

Correct, I get the stringmacOS:

(Snippet fromFindArduinoCtags.cmake)

cmake_host_system_information(  RESULT HOSTINFO  QUERY OS_NAME OS_PLATFORM)list(GET HOSTINFO 0 HOST_OS)list(GET HOSTINFO 1 HOST_ARCH)message(HOST_OS=${HOST_OS})

Results in:

HOST_OS=macOS

@massonal
Copy link
Contributor

Thanks for the confirmation@nathantsoi.
Then, it seems like it's a bug on CMake side (they probably forgot to update the doc at some point? 🤷 )
I suggest you raise a ticket by them...https://github.com/Kitware/CMake#reporting-bugs

As for the issue you have with ctags, do you see any functional impact from the 32-bits version?
I think here we'll try to stick to official releases as much as possible...
You can nonetheless raise an issue there too to ask for clarification!https://github.com/arduino/ctags

My final take on this PR: it does not introduce anybug, so it may be merged safely.
I'll let@fpistm decide on whether to merge it or wait on upstream fixes in the tools.

Copy link
Contributor

@massonalmassonal left a comment

Choose a reason for hiding this comment

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

This code is bug-less and harm-less.
Merging it is a matter of policy.

@nathantsoi
Copy link
ContributorAuthor

Re. the macOS string, I'll open a ticket w/ cmake.

Re. ctags, I should clarify that the 32-bit version of ctags will not run on macOS 13.x.

Using thehttps://github.com/arduino/ctags binary results in this error:

/bin/sh: /Users/ntsoi/.Arduino_Core_STM32_dl/d726370579d207be3e43211ba220e5b1/dist/ctags/ctags: Bad CPU type in executable

Hence the need to copy in the 64-bit version from the Arduino.app

Perhaps this can be noted in the cmake setup documentation for now? I am happy to do this if you would like.

@nathantsoi
Copy link
ContributorAuthor

Actually,@massonal, I just checked

message(CMAKE_HOST_SYSTEM_NAME=${CMAKE_HOST_SYSTEM_NAME})

And this does yield the documented result:CMAKE_HOST_SYSTEM_NAME=Darwin

Note, this is different from the snippets inensure_core_deps.cmake andFindArduinoCtags.cmake (#2060 (comment))

@massonal
Copy link
Contributor

That last item withCMAKE_HOST_SYSTEM_NAME really is confusing...cmake_host_system_information does redirect to it.

OS_NAME
New in version 3.10.

SeeCMAKE_HOST_SYSTEM_NAME

Definitely mention that in the ticket you write to CMake!

fpistm and nathantsoi reacted with thumbs up emoji

@fpistmfpistm added fix 🩹Bug fix and removed waiting feedbackFurther information is required labelsJul 11, 2023
@fpistmfpistm merged commit8ff274a intostm32duino:mainJul 11, 2023
@fpistm
Copy link
Member

Perhaps this can be noted in the cmake setup documentation for now? I am happy to do this if you would like.

Feel free to submit a PR for that 😉

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@massonalmassonalmassonal left review comments

Assignees
No one assigned
Labels
fix 🩹Bug fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@nathantsoi@massonal@fpistm

[8]ページ先頭

©2009-2025 Movatter.jp