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

Changed icons path for distros #110

Closed
caksoylar opened this issue Jun 8, 2018 · 31 comments
Closed

Changed icons path for distros #110

caksoylar opened this issue Jun 8, 2018 · 31 comments

Comments

@caksoylar
Copy link
Contributor

I notice that the distributions I have installed (Ubuntu, Ubuntu-18.04) don't have their icons in $ProgramW6432/WindowsApps/$instdir/images/icon.ico anymore. This seems to be the case both for the 1803 update and the insider preview I am running on build 17686. As a result, config-distros.sh cannot find the icons and falls back to the default WSL icon. However there are various icons present at $ProgramW6432/WindowsApps/$instdir/assets/ instead.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 8, 2018

Yes that's not depends on Windows version. It's in the new updated Windows Store apps (i.e. Ubuntu18.04, Ubuntu16.04 etc). The executable ubuntu.exe contains that icon. So there is no separate ubuntu.ico files.

And the assets folder is for Start Menu images aka. tiles.

@mintty
Copy link
Owner

mintty commented Jun 8, 2018

I guess it will be possible to take the icon from the distro-specific wrapper then.
Provided the files are accessible. On my system, these wrappers are some weird kind of zero-length pseudo files from which I cannot configure an icon...

@caksoylar
Copy link
Contributor Author

In my system the executables under %localappdata%\Microsoft\WindowsApps\Canonical[...] are also zero length, but the ones in C:\Program Files\WindowsApps\Canonical[...] are non-zero length with embedded icons. Pointing to that exe for the icon in shortcut properties seemed to work.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 9, 2018

I found three ways to do this:

  1. Maintain separate ico files in wsltty repository.
  2. Store all ico files in a DLL and take the matching icon at runtime.
  3. Take a snapshot of the current processes, find which one is Windows store app, go to its installation path, extract the icon resource from the executable file.

Which one do you choose?

@mintty
Copy link
Owner

mintty commented Jun 9, 2018

Why not collect the icons like it's done now, just from the wrapper exes instead of the .ico files?

  1. would not need a complete overview of existing distros and not include future distros; also I'd prefer not to include icons, to avoid any copyright discussions.
  2. same as 1., plus fiddling around with Windows resources :(
  3. even more dynamic than 1; this is about installing shortcuts, for all installed distros (and fetching the icon for the currently invoked distro, which is a special case)

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 9, 2018

I thought of extracting the icon from exe file. Can mintty takes icon from another exe file without extracting?

I wonder if this is possible in wslbridge. That's why I asked @rprichard if wslbridge can execute wsl.exe or ubuntu.exe instead of bash.exe. rprichard/wslbridge#27

@mintty
Copy link
Owner

mintty commented Jun 9, 2018

For its own icon, for option -i or if started from a shortcut, mintty uses the ExtractIconEx function as you see in winmain.c. For configuration of shortcuts during wsltty installation (not appx), it simply determines the proper -i option or shortcut icon field to set, which can also be an .exe.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 9, 2018

Problem! To get the executable path in Appx installation, I've to use GetPackageFullName(). But the Appx first need to be opened to get the process handle.

Another way I found is to get that path from registry. But there is also difference. For example Ubuntu 18.04:

  • Installation path:
%ProgramFiles%\WindowsApps\CanonicalGroupLimited.Ubuntu18.04onWindows_1804.2018.427.0_x64__79rhkp1fndgsc
  • PackageFamily Registry:
CanonicalGroupLimited.Ubuntu18.04onWindows_79rhkp1fndgsc

@caksoylar
Copy link
Contributor Author

In my testing pointing to $ProgramW6432/WindowsApps/$instdir/$distro.exe on https://github.com/mintty/wsltty/blob/master/config-distros.sh#L132 and L133 works for Ubuntu. However it doesn't work for Ubuntu-18.04, since it looks for Ubuntu-18.04.exe but the file is actually called ubuntu1804.exe. I tried modifying the variable but dash doesn't have executables like tr that we can use to parse the name. (Even basename that is currently used in the script is erroring for me, though it doesn't seem to break anything.) I think using something from the Windows side to remove . and - characters a la cmd.exe /c "something" might be a good way to go.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 9, 2018

Wow! That's a good idea 👍
If you modify some thing like this: Get the $instdir as mention in that script. Then find any file which have .exe extension and grab the icon. I'll post a C pseudo code.

@caksoylar
Copy link
Contributor Author

Good idea to check for any executable. I would also look into doing this in https://github.com/mintty/wsltty/blob/master/mkshortcut.vbs#L39. Doing it in VBS can be easier than working in dash.

@mintty
Copy link
Owner

mintty commented Jun 9, 2018

The chaos of undocumented, unsystematic and ever-changing Windows interfaces is really bothering me.
Stripping punctuation from the distro name for the file name is not really a working idea; the wrapper for openSUSE-42 is actually called openSUSE-42.exe.
Is the wrapper name perhaps listed in the registry for newer versions?

@caksoylar
Copy link
Contributor Author

Wow yeah, didn't notice that. Just now I checked a couple other distros and kali-linux also has executable name kali.exe.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 9, 2018

I found a registry path that has the registry value: For example, HKCU\SOFTWARE\Microsoft\Windows\CurrentVersion\App Paths has ubuntu1804.exe.

Another way came in my mind. But I've tested it. If you see the AppxManifest.xml file in wsltty.appx repository you can understand it better. Every Appx package has the XML file. So the procedure will be like: Get the $instdir as before > read the AppxManifest.xml file > get the Executable (before EntryPoint) tag and copy its value (with sed or grep).
e.g. In Ubuntu 18.04, the XML file has Executable="ubuntu1804.exe".

@caksoylar
Copy link
Contributor Author

I just opened the pull request above after testing it on my system, it worked for all distros on the store. It would be good if you could test it too @Biswa96.

@mintty
Copy link
Owner

mintty commented Jun 11, 2018

Why not? It's already in use anyway. About declspec, I have no idea.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 11, 2018

@caksoylar Does mintty requires administartor privilege to read AppxManifest.xml file? Beacuse Windows UWP platform may not allow to enter %ProgramFiles%\WindowsApps but it allows only %LocalAppData%\Packages.

I've asked this question in Stack Overflow and someone said that it requires admin privileges.

@caksoylar
Copy link
Contributor Author

I know I had to get admin access to reach that location through Windows Explorer, but the script doesn't need admin access after that and it used to work before when images/icon.ico was there, so 🤷‍♂️

The reason I was also asking for testing was because I already changed the owner of the folder from TrustedInstaller to my user in order to be able to access it through Explorer. There is a chance it might fail if you never did that step.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 13, 2018

Here I post a function which scans the WindowsApps Folder and find the matching folder name with Distro Name. Then it finds a EXE file in that folder. This doesn't require admin privilege. But one confusion Canonical launched Ubuntu, Ubuntu-1604, Ubuntu-1804 in Windows Store.

Here is the function (click to open)
void findExe(LPWSTR DistroName, LPWSTR PackageName, LPWSTR PackageExe) {

    WCHAR Buffer[MAX_PATH], FirstFile[MAX_PATH];
    HANDLE hFile;
    WIN32_FIND_DATAW fileInfo;

    ExpandEnvironmentStringsW(L"%ProgramFiles%\\WindowsApps", Buffer, MAX_PATH);

    swprintf(FirstFile, MAX_PATH, L"%ls\\*", Buffer);

    hFile = FindFirstFileW(FirstFile, &fileInfo);
    if (hFile != INVALID_HANDLE_VALUE) {
        do {
            if (wcsstr(fileInfo.cFileName, DistroName) != 0) {
                memcpy(PackageName, fileInfo.cFileName, MAX_PATH);
            }
        } while (FindNextFileW(hFile, &fileInfo) != 0);
    }
    
    swprintf(FirstFile, MAX_PATH, L"%ls\\%ls\\*", Buffer, PackageName);

    hFile = FindFirstFileW(FirstFile, &fileInfo);
    if (hFile != INVALID_HANDLE_VALUE) {
        do {
            if (wcsstr(fileInfo.cFileName, L"exe") != 0) {
                memcpy(PackageExe, fileInfo.cFileName, MAX_PATH);
            }
        } while (FindNextFileW(hFile, &fileInfo) != 0);
    }
}

Here is the steps of how I found this idea, see my answer in Stack Overflow. Forgive me if you find any bad code 🙇

@mintty
Copy link
Owner

mintty commented Jun 14, 2018

First loop: you look for the packagename by distroname. This is currently handled already, by looking up the registry. Has anythig changed about this mapping or can/should we stay with the implemented approach?
Second loop: in the package folder, you look for any .exe file. If there are multiple ones, how do you pick the right one?

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 17, 2018

There is an exception in my provided code. This is better understand by some examples of Windows Store installation:

  • For Ubuntu 18.04:

    • Package installation paths:
      • %ProgramFiles%\WindowsApps\CanonicalGroupLimited.Ubuntu18.04onWindows_1804.2018.427.0_x64__79rhkp1fndgsc\AppxManifest.xml
    • WSL installation path (BasePath):
      • %LocalAppData%\Packages\CanonicalGroupLimited.Ubuntu18.04onWindows_79rhkp1fndgsc\LocalState
  • For Kali Linux:

    • Package installation paths:
      • %ProgramFiles%\WindowsApps\KaliLinux.54290C8133FEE_1.1.3.0_neutral_~_ey8k8hqnwqnmg\AppxMetadata\AppxManifest.xml
      • %ProgramFiles%\WindowsApps\KaliLinux.54290C8133FEE_1.1.3.0_neutral_split.scale-100_ey8k8hqnwqnmg\AppxManifest.xml
      • %ProgramFiles%\WindowsApps\KaliLinux.54290C8133FEE_1.1.3.0_x64__ey8k8hqnwqnmg\AppxManifest.xml
    • WSL installation path (BasePath):
      • %LocalAppData%\Packages\KaliLinux.54290C8133FEE_ey8k8hqnwqnmg\LocalState

Now it is clear that if someone provides only L"KaliLinux" in that function, it will provide that three paths. The kali.exe only present in the 3rd path. This is because the Kali Linux package is an AppxBundle (i.e. many appx in one file). Also it may be hard to choose AppxManifest.xml file.

@mintty
Copy link
Owner

mintty commented Jun 17, 2018

To repeat for clarification, all this recent fuss is just to find the proper icon again, right? Where is it in Kali Linux? (I'll install it myself later.)

@mintty
Copy link
Owner

mintty commented Jun 26, 2018

I'm inclined to consider it a bug of the distros if they hide their icon so deeply in cryptic configuration.
Mintty shall continue to support the distinct icon location, and should fallback to the penguin icon otherwise.

@Biswa96
Copy link
Contributor

Biswa96 commented Jun 26, 2018

The paths are not cryptic. With Windows .NET Framework and C#, it can be easily done. There are some deep connection between C# and raw C code. I'm not expert of C# (and don't like it). If anyone expert with C# and UWP, he/she can do it.
But I like cute Tux icon.

@mintty
Copy link
Owner

mintty commented Jun 26, 2018

I think it's cryptic, and kind of malicious,

  • to have no clear mapping from package name to launcher name
  • to hide something as basic as an icon as a Windows resource in an exe
  • to hide something as basic as the path of the launcher exe in an XML structure

I plan to release mintty really soon now, without further WSL-specific enhancements in the mintty code.

@mintty
Copy link
Owner

mintty commented Jul 1, 2018

Released 1.9.0, not supporting the new icon location.

@0x5c
Copy link

0x5c commented Mar 10, 2019

If the setup script is a setup, then why couldn't it just plainly extract the icons to a wsltty dir?
Then it wouldn't be needed to modify mintty itself in any way.

@mintty
Copy link
Owner

mintty commented Mar 11, 2019

extract the icons to a wsltty dir?

Wsltty does not have icons of any WSL distribution. They come with the respective distribution.
Also, mintty is not modified about icons, just the shortcuts are configured to refer to the proper icon, if available. If no icon is (easily) available, wsltty can only use a default fallback icon.

@0x5c
Copy link

0x5c commented Mar 11, 2019

I think I was misunderstood.
I was not talking about:

  • shipping icons with wlstty
  • modify mintty in any way

What I am talking about is extracting the icons on-the-spot at shortcut configuration time, and then giving those extracted icons to shortcuts/mintty.
This way, wlstty would be free of copyright issues, mintty wouldn't have to be modified in any way,
And we would get our bread and circuses icons.

@mintty
Copy link
Owner

mintty commented Mar 11, 2019

That's what the config script is actually doing, with the old location of icons within a WSL distribution. Just some recent distributions have changed their deployment scheme so the icon cannot be found as easily anymore. There was a discussion about this and a proposal by Biswa already. But his icon extraction did not work for me, so I gave up at the time.
If I can integrate a function that determines the icon file location from a WSL distribution name or id, I'll happily include that. It should preferably be a plain cmd script (Powershell not appreciated because of its long startup time) or an executable, but not depending on additional software, and preferably not on .net either.

@mintty
Copy link
Owner

mintty commented May 28, 2019

OK, after having lost sight of this issue for a while, I've now merged the patch (#111), but changed it to do without powershell; using shell commands and matching instead.

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

No branches or pull requests

4 participants