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

runners: jlink: win32: search for valid JLink.exe #83562

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JordanYates
Copy link
Collaborator

Since "JLink.exe" is also an executable distributed with JDK's, do an explicit search on the standard SEGGER install directories for a JTAG "JLink.exe" before falling back to whatever is first on PATH.

Fixes #51825.

@JordanYates JordanYates force-pushed the 250105_win_jlink_path branch from eccefa3 to 2eab62b Compare January 5, 2025 11:06
pdgendt
pdgendt previously approved these changes Jan 5, 2025
@theadib
Copy link
Contributor

theadib commented Jan 5, 2025

Hello Jordan,
you only test for "versioned" JLink folders: ... SEGGER" / "JLink_V*"))
Please could you also add to the tests the "unversioned" variant: .../SEGGER/JLink <- without "_V*"

That is created when selecting "update existing installation" instead "create new instance" during installation proccess.

Thanks, Adib.

@theadib
Copy link
Contributor

theadib commented Jan 5, 2025

... and maybe add the USERs Program folder in the case the user has selected "for this user only"
/SEGGER/JLink
where is Pythons: pathlib.Path.home()

@JordanYates JordanYates dismissed stale reviews from henrikbrixandersen and pdgendt via 27d7a33 January 5, 2025 23:22
@JordanYates JordanYates force-pushed the 250105_win_jlink_path branch from 2eab62b to 27d7a33 Compare January 5, 2025 23:22
@JordanYates
Copy link
Collaborator Author

Added Path.home() to roots checked, and JLink as a preferred folder to use:
image
image

Since "JLink.exe" is also an executable distributed with JDK's, do an
explicit search on the standard SEGGER install directories for a JTAG
"JLink.exe" before falling back to whatever is first on PATH.

Fixes zephyrproject-rtos#51825.

Signed-off-by: Jordan Yates <[email protected]>
@JordanYates JordanYates force-pushed the 250105_win_jlink_path branch from 27d7a33 to f703bf5 Compare January 5, 2025 23:25
@rruuaanng
Copy link
Collaborator

If the JLINK_HOME environment variable is set, we should start the search directly from there. This is how my local branch is currently configured. WDYT? @JordanYates

@JordanYates
Copy link
Collaborator Author

If the JLINK_HOME environment variable is set, we should start the search directly from there. This is how my local branch is currently configured. WDYT? @JordanYates

Is that an environment variable set by the JLink install tools?

@rruuaanng
Copy link
Collaborator

If the environment variable is set, we should start the search directly from there. This is how my local branch is currently configured. WDYT? @JordanYatesJLINK_HOME

Is that an environment variable set by the JLink install tools?

No, perhaps this is a habit shared by some users, like myself, or others who have the same habit. Maybe I should ask the author for their thoughts on this.

@JordanYates
Copy link
Collaborator Author

If the environment variable is set, we should start the search directly from there. This is how my local branch is currently configured. WDYT? @JordanYatesJLINK_HOME

Is that an environment variable set by the JLink install tools?

No, perhaps this is a habit shared by some users, like myself, or others who have the same habit. Maybe I should ask the author for their thoughts on this.

IMO if people are not installing it in one of the 3 standard locations, it is not unreasonable to require them to either setup $PATH manually or explicitly specify --commander on the command line.

@JordanYates JordanYates requested a review from pdgendt January 6, 2025 01:58
@rruuaanng
Copy link
Collaborator

That's what I want to say, I don't like to install software on C: disk.

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

Successfully merging this pull request may close these issues.

west: runners: jlink: JLink.exe name collision
6 participants