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

WinPath.Updater returns %PROGRAMFILES(X86)% #71

Closed
ANF-Studios opened this issue May 20, 2021 · 6 comments
Closed

WinPath.Updater returns %PROGRAMFILES(X86)% #71

ANF-Studios opened this issue May 20, 2021 · 6 comments
Assignees
Labels
bug Something isn't working WinPath.Updater WinPath.Updater's project wontfix This will not be worked on

Comments

@ANF-Studios
Copy link
Owner

ANF-Studios commented May 20, 2021

On a x86_64 computer, WinPath.Updater returns %PROGRAMFILES(x86)%, this is a bug and a concern as Windows team is planning to allow the user to change the default hard disk -- as of writing this issue, this value is hard coded.

I looked into this issue and apparently it happens because of a 32-bit application being run on a 64-bit host environment. Please see #64 for more information.

@ANF-Studios ANF-Studios added bug Something isn't working WinPath.Updater WinPath.Updater's project labels May 20, 2021
@ANF-Studios ANF-Studios self-assigned this May 20, 2021
@ANF-Studios ANF-Studios mentioned this issue May 20, 2021
7 tasks
@ANF-Studios ANF-Studios changed the title WinPath.Updater returns %PROGRAMFILES(X86) WinPath.Updater returns %PROGRAMFILES(X86)% May 22, 2021
@levicki
Copy link

levicki commented May 22, 2021

There are several issues with your WinPath.Updater code:

  1. You are hard-coding Program Files and Program Files (x86) folder names in your code. Are you aware that those names are localized in non-English versions of Windows?
  2. Are you aware that writing to Program Files or Program Files (x86) requires elevation (administrator rights)? Are you even checking for that?
  3. You seem to be wrong in your assessment that getting wrong path is a Win32 problem, I believe it is not and you are simply not using the right API. You should use Environment.GetFolderPath and pass ProgramFiles or ProgramFilesX86 as argument.
  4. If your program is 32-bit you should never install it into Program Files on a 64-bit OS, because that is where 64-bit programs are supposed to be installed. It should go into Program Files on a 32-bit OS and into Program Files (x86) on a 64-bit OS. In your current code the if condition seems to be reversed.

Finally, why bother with writing a command line tool (with an updater no less!) when half the functionality already exists in the setx command, not to mention that you can download a simple and nice GUI tool for that from my website? :-)

@ANF-Studios
Copy link
Owner Author

Hello @levicki, thanks for pointing out issues.

There are several issues with your WinPath.Updater code:

  1. You are hard-coding Program Files and Program Files (x86) folder names in your code. Are you aware that those names are localized in non-English versions of Windows?

Yes, that's why it was a concern; that commented code was returning Program Files (x86) regardless of everything.

  1. Are you aware that writing to Program Files or Program Files (x86) requires elevation (administrator rights)? Are you even checking for that?

Yep, originally, the code to start WinPath.Updater is from WinPath, and yes, it does start it as an administrator and handles everything. The code behind very very messy, but that's only because I plan to replace WinPath.Updater entirely with v0.4.0 (see #67 for more details).

  1. You seem to be wrong in your assessment that getting wrong path is a Win32 problem, I believe it is not and you are simply not using the right API. You should use Environment.GetFolderPath and pass ProgramFiles or ProgramFilesX86 as argument.

That's actually the entire problem in the first place. Refer to this image (x64 host running both x86 and x64 binaries respectively):
image

(in reply to that comment on the other issue; Your problem has nothing to do with this particular issue which is about .Net implementation of environment variables) and when I said And there's really nothing we can do about it as far as I know, only Win32 API team can change this, I meant that there's nothing .NET can do about it, only Win32 API's team, since .NET calls Windows' API, it doesn't magically know it. And there's really no need for a new issue for this, just sharing some information 😄.

  1. If your program is 32-bit you should never install it into Program Files on a 64-bit OS, because that is where 64-bit programs are supposed to be installed. It should go into Program Files on a 32-bit OS and into Program Files (x86) on a 64-bit OS. In your current code the if condition seems to be reversed.

Yea, I don't do that, I know that. The current structure is very confusing I myself should know that, the way it works is that once WinPath (stored in program files x86 on 32-bit hosts and program files on x64 hosts) starts updating, it downloads the new version and the updater. The updater is only 32-bit and arm, once downloaded, it starts WinPath.Updater.exe which then simply copies it to the right location, it's hard-coded, I know, but, that issue. But hey, that issue should be solved thanks to you now.

Finally, why bother with writing a command line tool (with an updater no less!) when half the functionality already exists in the setx command, not to mention that you can download a simple and nice GUI tool for that from my website? :-)

That's pretty much like saying "why make something when it already exists" my good man. But the main reason I wanted to do that is for

  1. A good learning experience, it's a project that I truly find motivation for. I didn't even spend a complete whole year of programming yet.
  2. This project originally started as a side-project for me -- personal use -- which I started because of that delete button from that environment variables window which is really scary and I don't feel great when messing with it.


On a foot note; can you share your gui application of yours, I would love to take a look at it.

@levicki
Copy link

levicki commented May 23, 2021

Yes, that's why it was a concern; that commented code was returning Program Files (x86) regardless of everything.

Ok, but this issue of yours has to do with Environment.GetFolderPath function, and not with Environment.SetEnvironmentVariable function which was the .Net Framework issue you posted your problem to.

Environment.GetFolderPath is calling SHGetFolderPathW from Win32 API which is deprecated, and is just a wrapper for SHGetKnownFolderPath.

In the documentation for KNOWNFOLDERID parameter, right next to FOLDERID_ProgramFiles it says "See Remarks for more information", and near the bottom of that quite lengthy page, there is a Remarks section which explains the behavior you are observing.

image

So, it turns out that 32-bit program cannot get path to 64-bit Program Files on 64-bit OS (and by "cannot" I only mean "cannot in a legitimate way" because I am sure it is possible in some other way). Basically all that boils down to "works as designed", "won't fix", etc, so I think it would be best if you removed your post from that other issue and I can then remove mine where I told you I will respond here.

You have two options:

  1. Drop 32-bit support, and make everything 64-bit only.

OR

  1. Have 32-bit updater for 32-bit app, and 64-bit updater for 64-bit app as it should be if you want to support both platforms.

Any other option will just cause you more trouble.

The above was a comment from a developer perspective, what follows is from a user perspective.

As a computer user I very much dislike the idea of every program having its own update checking (be it on startup, as a scheduled task, or worse as an always running background service), not to mention its own hand-rolled installers and updaters which often do not respect system conventions and/or system preferences, and have no rollback functionality to restore the system to a known working state if something goes wrong.

If you want to distribute your programs, you either provide them as a simple .zip archive, or you use one of the myriad known good installers that provide full uninstall, rollback in case of failure, update of existing versions, silent installation and uninstallation, 32-bit and 64-bit OS differences, installation of pre-requisites, etc, such as InstallShield, WIX, MSI, or my favorite InnoSetup which is free to use.

That's pretty much like saying "why make something when it already exists" my good man.

It is great as a learning experience, but I was confused by the need for another command line version of SETX :-)

A final friendly advice:

C# and .Net are great productivity tools. They enable you to churn out enormous amount of functional code in a rather short time. That's great when you are working on client/server architecture projects (ASP.NET, MVC, etc), but when you are working on a Windows platform and writing console, desktop, or Windows service applications you cannot skip the basics.

Depending on the application type you might need to know about any of the following:

  • Win32 API (to know how things work under the hood)
  • COM (for DirectX, DirectWrite, Direct2D, DirectShow, shell interfaces, Office programmability, etc)
  • Windows ABI, calling convetions, marshalling, and interop (when you need to implement stuff that .Net doesn't cover at all or implements badly)
  • Windows controls, window message pump, and event driven programming (for overriding WinForms controls)
  • Windows services, session 0 separation, user privileges, tokens, and ACL manipulation
  • WinHTTP and/or SCHANNEL specifics (when WebClient isn't good enough)
  • Windows quirks and compatibility shims for 32-bit code (to avoid problems like the one you just bumped into)
  • Historical stuff such as environment variable limits, file path limits, etc

And the list goes on.

For those who started with C/C++, this was the only way to go, but people starting with .Net usually don't have that platform knowledge and aren't aware they will need it as soon as they aren't happy with how some .Net WinForms control or library API is working.

As for my GUI version, you can download it from my website downloads section. It's called SmartPathEditor.

@ANF-Studios
Copy link
Owner Author

Yes, that's why it was a concern; that commented code was returning Program Files (x86) regardless of everything.

...
So, it turns out that 32-bit program cannot get path to 64-bit Program Files on 64-bit OS (and by "cannot" I only mean "cannot in a legitimate way" because I am sure it is possible in some other way). Basically all that boils down to "works as designed", "won't fix", etc, so I think it would be best if you removed your post from that other issue and I can then remove mine where I told you I will respond here.

That's exactly what I said over there, sure it was about Environment.SetEnvironmentVariable but was not limited to that. I think it should be pretty obvious that my side note also refers to other such variables and actions.

You have two options:

  1. Drop 32-bit support, and make everything 64-bit only.

OR

  1. Have 32-bit updater for 32-bit app, and 64-bit updater for 64-bit app as it should be if you want to support both platforms.

Any other option will just cause you more trouble.

Of course, of course, recall the issue I referred to you in my previous message. I'm going to deprecate support (possibly keep backwards compatibility for a bit) for WinPath.Updater. I'm planning to roll this out slowly and slowly by 0.5.0, possibly 0.4.0 (which will be the release after this bug fix release). The project will either be named WinPath.Manager or WinPath.Elevated -- the former makes more sense -- anyhow, the point; this project will be shipped in 32-bit, 64-bit, arm, and arm64.

The above was a comment from a developer perspective, what follows is from a user perspective.

As a computer user I very much dislike the idea of every program having its own update checking (be it on startup, as a scheduled task, or worse as an always running background service), not to mention its own hand-rolled installers and updaters which often do not respect system conventions and/or system preferences, and have no rollback functionality to restore the system to a known working state if something goes wrong.

I agree with this to be honest, and I completely understand that. If you didn't realize that yet, WinPath never checks for update. The user doesn't even need to go beyond v0.1.0, I think it's a "fully shipped" release, it has the core features meant for it. The only way the user can update is by running winpath update and that's only when it does that, I do not do that elsewhere. I think it's highly unnecessary. It's all up to the user, besides, I don't really get anything from forcing an update, it's all up to the user, all responsibility.

If you want to distribute your programs, you either provide them as a simple .zip archive, or you use one of the myriad known good installers that provide full uninstall, rollback in case of failure, update of existing versions, silent installation and uninstallation, 32-bit and 64-bit OS differences, installation of pre-requisites, etc, such as InstallShield, WIX, MSI, or my favorite InnoSetup which is free to use.

I agree, but that makes sense when the application is actually big enough. WinPath is literally just a standalone not even a megabyte big:
image

I also eventually decide to roll out an installer script that basically sets this up for the user, though not compulsory, it just eases the process of installation.

A final friendly advice:

C# and .Net are great productivity tools. They enable you to churn out enormous amount of functional code in a rather short time. That's great when you are working on client/server architecture projects (ASP.NET, MVC, etc), but when you are working on a Windows platform and writing console, desktop, or Windows service applications you cannot skip the basics.
...
And the list goes on.

For those who started with C/C++, this was the only way to go, but people starting with .Net usually don't have that platform knowledge and aren't aware they will need it as soon as they aren't happy with how some .Net WinForms control or library API is working.

Sorry, what's your point? I don't quite understand what you're trying to say here.



Oh and, thanks for your time, it really helps getting strong feedback.

@levicki
Copy link

levicki commented May 24, 2021

That's exactly what I said over there, sure it was about Environment.SetEnvironmentVariable but was not limited to that.

It seems we are talking past each other. The issue "over there" was limited to Environment.SetEnvironmentVariable until you posted unrelated information.

The issue with Environment.SetEnvironmentVariable is a bug (or an API design failure resulting in loss of orthogonality) in .Net Framework code which causes Environment.SetEnvironmentVariable method to behave differently compared to underlying Win32 API. It is all well documented in my back and forth with Microsoft engineers.

Your issue was that you did not understand how underlying Win32 API which you are calling through C# wrapper actually works -- you complained about expected and documented behavior.

The fact that both Environment.SetEnvironmentVariable and Environment.GetFolderPath are sitting in the same namespace in .Net Framework does not make them related. In fact, each underlying Win32 API belongs to a different dev team -- SetEnvironmentVariable is a Windows kernel API, while SHGetFolderPath and SHGetKnownFolderPath are Windows shell APIs.

Sorry, what's your point? I don't quite understand what you're trying to say here.

What I was trying to say is that you need much better understanding of the Windows platform before reporting issues with Windows platform API, and I gave you a list of things which I consider necessary knowledge if one is to write Windows applications.

Furthermore, you should always open a new issue for each problem you discover -- developers don't like when people report multiple issues in a single ticket and there are many good reasons to avoid that.

Finally, hijacking discussion threads that you perceive as related to your issue is generally frowned upon in every discussion forum on the internet, and most people consider it rude.

Oh and, thanks for your time, it really helps getting strong feedback.

You are welcome.

@ANF-Studios
Copy link
Owner Author

Everything is settled, relating to this issue. We will not (and can not by semver) change this current system until v0.4.0 in favor of an elevated manager.

There is nothing to discuss regarding this topic.

@ANF-Studios ANF-Studios added the wontfix This will not be worked on label Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working WinPath.Updater WinPath.Updater's project wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants