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

Increase detection probability of installed git, cygwin and potentially more #70

Closed
markusschuh opened this issue Apr 1, 2019 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed scripts related to shell scripts (bash and CMD) windows specific for Microsoft Windows OS
Milestone

Comments

@markusschuh
Copy link
Member

Category: Enhancement
Severity: Medium
Comments:

Changing files inside file system (outside app specific directories) during setup of an application is not less - or more - invasive than using the windows registry. It is only different. The registry has the advantage, that it is a common place with - at least on top level - well known structure. Of course it is up to devon-ide-tools, if uses registry or not for own configuration purposes. But besides that, the detection rate of external tools would be higher, if not only default paths would be checked, but instead also the related registry would be checked, that might contain the installation path.
With CMD batch methods the related coding is a bit more complex and hard to maintain, but it is doable:

FOR /F "tokens=2*" %%O IN ('REG.EXE QUERY "HKLM\Software\Cygwin\setup" /V "rootdir" 2^>NUL ^| FIND "REG_SZ"') DO set CYGWIN_ROOT_PATH=%%P
FOR /F "tokens=2*" %%O IN ('REG.EXE QUERY "HKLM\Software\GitForWindows" /V "InstallPath" 2^>NUL ^| FIND "REG_SZ"') DO set GIT_ROOT_PATH=%%P

If agreed, I'd propose a related change via pull request, which first checks those value and - if not exist or path empty - falls back to checks for some default paths as already realized.

N.B.: The read from registry would even more helpful, if it would also support the cases, when cygwin / git are installed without administrative access in some user-writable area on file system. But unfortunately this does not mean automatically that the above registry values could just be written from HKCU instead of HKLM. Newer cygwins seem to store some value there, but in a different syntax, that supports several cygwin in parallel. This makes using the registry more complex for that purpose
HKEY_CURRENT_USER\Software\Cygwin\installations
<some dyn 128 bit key in hex> REG_SZ ??\C:\tools\cygwin

Your environment - Windows 7/Windows 10, devon-ide-scripts-3.0.0-beta2

@markusschuh
Copy link
Member Author

markusschuh commented Apr 1, 2019

There is an even more "stable against system specific setups" version of the command, like: A "reg" or find in path before the windows system one. Or a "SystemRoot" containing spaces (if that works).

This command though needs another know workaround - windows CMD can't execute the sub-command, when it starts with a double quote (due to some historic compatibility - if I redetect the web reference, I'll add it here). The workaround: Start the command with an - otherwise useless - "call":

FOR /F "usebackq tokens=2*" %%O IN (`call "%SystemRoot%"\system32\reg.exe QUERY "HKLM\Software\Cygwin\setup" /V "rootdir" 2^>NUL ^| "%SystemRoot%\system32\find.exe" "REG_SZ"`) DO set CYGWIN_ROOT_PATH=%%P

@hohwille
Copy link
Member

hohwille commented Apr 1, 2019

Wow! I am impressed by your great contributions and activities. I also very much appreaciate your insights and knowledge both in linux/mac/bash as well as in windows/cmd/registry.
Yes, It would be great to find the real installation path of git-bash and cygwin rahter than guessign and trying some of the most primenent candidates.
Can we do the same thing for git-bash as well? That is actually the recommended and major shell for devon-ide. Cygwin is only supported as kind of fallback. However, any improvement to find a bash in windows more reliable is most welcome.

@hohwille hohwille added enhancement New feature or request help wanted Extra attention is needed windows specific for Microsoft Windows OS scripts related to shell scripts (bash and CMD) labels Apr 1, 2019
@markusschuh
Copy link
Member Author

Yes, already mentioned in my enhancement proposal. Here the "more stable against system specific setups" version:

for /F "usebackq tokens=2*" %%O in (`call "%SystemRoot%"\system32\reg.exe query "HKLM\Software\GitForWindows" /v "InstallPath" 2^>nul ^| "%SystemRoot%\system32\findstr.exe" REG_SZ`) do set GIT_ROOT_PATH=%%P
if exist "%GIT_ROOT_PATH%\git-bash.exe" ( echo git-bash.exe found )

For quick test on interactive CMD as usual the double percent (only needed in batch scripts) need to be replaced by single percent.

@hohwille
Copy link
Member

hohwille commented May 7, 2019

@markusschuh thanks for your valuable input. This seems to work fine for arbitrary installations of git bash and I also adopted the same for cygwin what also seems to work fine.

Of course some testing for various setups (including a vanilla windows without git and cygwin) will still be helpful.

@hohwille hohwille added this to the release:3.0.0 milestone May 7, 2019
hohwille added a commit that referenced this issue May 8, 2019
#99: automatically install devonfw extension pack into vscode
#103: restored integration documentation
#70: find git or cygwin from registry
#101: fixed
#84: prepared cobigen and TM Terminal
@hohwille
Copy link
Member

hohwille commented May 8, 2019

PR #105 merged. Hence done.

@hohwille hohwille closed this as completed May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed scripts related to shell scripts (bash and CMD) windows specific for Microsoft Windows OS
Projects
None yet
Development

No branches or pull requests

2 participants