-
Notifications
You must be signed in to change notification settings - Fork 49
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
added MSYSTEM=ARM64 case in /etc/profile #6
Conversation
etc/profile
Outdated
@@ -61,6 +61,13 @@ MINGW64) | |||
ACLOCAL_PATH="${MINGW_MOUNT_POINT}/share/aclocal:/usr/share/aclocal" | |||
MANPATH="${MINGW_MOUNT_POINT}/local/man:${MINGW_MOUNT_POINT}/share/man:${MANPATH}" | |||
;; | |||
ARM64) | |||
MINGW_MOUNT_POINT="${MINGW_PREFIX}" |
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.
MINGW_PREFIX is empty on my arm64 test build 🤷🏼♂️
I saw some references to MINGW_PREFIX in various places and it mostly relies on uname -m
. Since Bash will remain a 32-bit executable for now, it's probably best if we set `MINGW_MOUNT_POINT="/arm64" here.
Before setting MINGW_MOUNT_POINT
to /arm64
:
After:
@@ -61,6 +61,13 @@ MINGW64) | |||
ACLOCAL_PATH="${MINGW_MOUNT_POINT}/share/aclocal:/usr/share/aclocal" | |||
MANPATH="${MINGW_MOUNT_POINT}/local/man:${MINGW_MOUNT_POINT}/share/man:${MANPATH}" | |||
;; | |||
ARM64) | |||
MINGW_MOUNT_POINT="${MINGW_PREFIX}" | |||
PATH="${MINGW_MOUNT_POINT}/bin:/mingw32/bin:${MSYS2_PATH}${ORIGINAL_PATH:+:${ORIGINAL_PATH}}" |
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.
We don't need mingw32/bin
in here anymore if we choose to go down the path from git-for-windows/MINGW-packages#44 and rename mingw32
to arm64
prior to copying the arm64 executables into it:
Curious to hear your thoughts on this
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.
That's @dscho 's idea.
#6 (comment)
I have no preference on this.
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 would strongly recommend not renaming mingw32
to arm64
, but instead adding the latter. Then, PATH
can be configured in such a way that the binaries from arm64/bin/
are preferred over those from mingw32/bin/
, and we're free to only provide a subset of the .exe
files for ARM64 (with the rest just being used in their i686 form).
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.
Makes sense. Just applied that in git-for-windows/MINGW-packages#44; arm64
comes first in the path, then mingw32
.
@tommyvct would you mind creating a PR for https://github.com/git-for-windows/git-sdk-64 as well with these changes? So that both SDKs are consistent 🚀 |
@dennisameling I'll try. |
Eventually, this change needs to be converted into a part of the |
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.
Something like this should work, maybe added directly after this line: test i686 != "$arch" ||
grep -q '^ARM64)$' /etc/profile ||
sed -i '/^MINGW64)/{
:1;N;/[^;]$/b1;
s|^MINGW\(.*\)\${MINGW_PREFIX}\(.* PATH=[^:]*\)\(.*\)$|&\nARM\1/arm64\2:/mingw32/bin\3|
}' /etc/profile Note: the change I propose will have to be offered as a PR in https://github.com/git-for-windows/build-extra, and the change can be tested by building the |
My school work just started rolling in, I'll try what I can do. |
@tommyvct just ping me if you need some help 👍🏼 good luck with school! |
@dennisameling tell you what: I'll do the PR and ask you to verify that it works? |
Also works @dscho 😄 👍🏼 |
This PR might now be closed since this change was added automatically by 977f2bc |
Thanks! |
No description provided.