tsoding / musializer

Music Visualizer
MIT License
877 stars 92 forks source link

fixed x86_64-w64-mingw32-ar not found for windows #99

Open Subhranil-Maity opened 6 months ago

Subhranil-Maity commented 6 months ago

While runing nob on my windows machine it fails to find "x86_64-w64-mingw32-ar" and when i tried to look into the problem i found many mingw installation especially potable ones have "ar" is place of _"x8664-w64-mingw32-ar".It fails to do its stuff for this program to be in different names. I added a function command_exists(const char *command) to cheack if x86_64-w64-mingw32-ar exists in as a executable to nob, if it does not then tuses ar as its archiver.

Markos-Th09 commented 6 months ago

Since ar exists on all windows host targets exist, you could do something similar to windres with the preprocessor. This was mostly added because when cross compiling the native ar of some platforms (ex. the one apple provides on mac os) might not be able to process windows pe objects. I wasn’t aware windows mingw is weird in this way.

Subhranil-Maity commented 6 months ago

I know nothing about windres working but the nob will build for platform while compiling for specific platform with available tools.so I don't think this will create any kind of platform limitation as platform specific ar will be created each time.

Markos-Th09 commented 6 months ago

I know nothing about windres working but the nob will build for platform while compiling for specific platform with available tools.so I don't think this will create any kind of platform limitation as platform specific ar will be created each time.

Sorry if it wasn’t clear enough, I am talking about this part of the build. This is only a suggestion as zozin should decide about this.

Subhranil-Maity commented 6 months ago

Ah, gotcha! So, I checked out my installation folder and found stuff like gcc and g++, both with their original names and some with a prefix. But, you know, not all compilers are created equal, and I don't have the resources to test them all. And honestly, I'm not an use who uses or have used all compilers out there. So, fixing it for mingw might not solve the issue for other compilers.

Based on my research, it seems like this might be a mingw, Windows-specific problem. So, I'm using a mingw flag to switch to just using ar when it detects a mingw on windows(asuming the script is only used when only using mingw). Otherwise, it'll go with the prefixed one.

Hope that makes sense from my end

Markos-Th09 commented 6 months ago

I understand. I was just talking about the possibility of this being decided at compile time when using nob on a windows host instead of runtime, similar to how prefixed or unprefixed windres is decided to be used. This was suggested simply for the sake of consistency since the two problems are similar.

Subhranil-Maity commented 6 months ago

Yeah, I also made sure that if it detects a Windows host, it'll use the unprefixed version; otherwise, for POSIX or others, it sticks to the normal version. This shouldn't cause any problems with other targets.

Now I understand your initial message, that the mingw script isn't just for Windows, so I need to be careful when writing the code to consider all platforms.

Markos-Th09 commented 6 months ago

Yeah, I also made sure that if it detects a Windows host, it'll use the unprefixed version; otherwise, for POSIX or others, it sticks to the normal version. This shouldn't cause any problems with other targets.

Now I understand your initial message, that the mingw script isn't just for Windows, so I need to be careful when writing the code to consider all platforms.

I apologize if my original message wasn’t as clear as I thought. Since mingw runs on pretty much everything this is why this script is also intended for cross compiling. In fact with the help of wine, this is how zozin builds and tests musializer on windows in streams.

Subhranil-Maity commented 6 months ago

Alright, sounds good! Hopefully, this last commit (ignoring the merge commit) has patched up all the loopholes in my script.👍

Markos-Th09 commented 1 month ago

Closes #119

Subhranil-Maity commented 1 month ago

Can I by any chance help to fix the merge conflict 🥲

Markos-Th09 commented 1 month ago

I think it should be as simple as accepting the deletion and backporting it to the file inside src_build

Subhranil-Maity commented 1 month ago

Please elaborate I did not get it

Markos-Th09 commented 1 month ago

Just rebase. You probably ignore the rest of the changes and modify src_build/nob_win64_mingw.c accordingly when it comes up

Markos-Th09 commented 1 month ago

If you still don’t get it forget it. I’ll just open a new PR.

Subhranil-Maity commented 1 month ago

Ok I will do it tomorrow.

Subhranil-Maity commented 1 month ago

I hope this will work

jgabaut commented 1 week ago

As I asked in #120, I think _WIN32 would still be defined by mingw on Linux but I'm not sure this matters at all.

In any case I'd advice to rebase this in order to fixup the number of commits.

Subhranil-Maity commented 1 week ago

No that flag is only defined if compiled from windows host. And the rebase was already done👍

jgabaut commented 1 week ago

So I guess my misconception stems from me wrongly using --host rather than --target when configuring autotools.

For the rebase I don't know, it's showing a big diff for what on your fork looks like 7 commits.

Markos-Th09 commented 1 week ago

@jgabaut I have made pr #120 which has a cleaner diff to solve this

Markos-Th09 commented 1 week ago

As I asked in #120, I think _WIN32 would still be defined by mingw on Linux but I'm not sure this matters at all.

In any case I'd advice to rebase this in order to fixup the number of commits.

_WIN32 defined by mingw on linux. However, this check is in the nob build process to determine if the host is windows. However this doesn’t matter in this case because nob is compiled using the host compiler where it wouldn’t be defined on linux.

Subhranil-Maity commented 1 week ago

Sorry if I was unclear but the _WIN32 is not defined in linux it's only define in windows. I donot know much but as per my knowledge and experience I can confirm its working. You can test it by writing two block with one with ifdef linux and if def unix, and the other ifdef _WIN32 . Open the program in any editor in linux and windows. You will notice that only the host specific cod eis highlighted and the others grey Means only windows portion is highlighted in windows and the other in linux or Unix.

jgabaut commented 1 week ago

Sorry if I was unclear but the _WIN32 is not defined in linux it's only define in windows. I donot know much but as per my knowledge and experience I can confirm its working. You can test it by writing two block with one with ifdef linux and if def unix, and the other ifdef _WIN32 . Open the program in any editor in linux and windows. You will notice that only the host specific cod eis highlighted and the others grey Means only windows portion is highlighted in windows and the other in linux or Unix.

My doubt was about the host not being right.

When I cross build from Linux a Windows binary using mingw, I rely on configure taking the --host parameter. Perhaps it would be better to use the --target parameter to avoid this confusion? I don't know.

In any case, when doing this, the check may not have the intended behaviour since _WIN32 will be defined but the tools to call should still be named x86_64-w64-mingw32-TOOL.

That's what I meant, and it may not be an issue at all.

Markos-Th09 commented 1 week ago

Sorry if I was unclear but the _WIN32 is not defined in linux it's only define in windows. I donot know much but as per my knowledge and experience I can confirm its working. You can test it by writing two block with one with ifdef linux and if def unix, and the other ifdef _WIN32 . Open the program in any editor in linux and windows. You will notice that only the host specific cod eis highlighted and the others grey Means only windows portion is highlighted in windows and the other in linux or Unix.

My doubt was about the host not being right.

When I cross build from Linux a Windows binary using mingw, I rely on configure taking the --host parameter. Perhaps it would be better to use the --target parameter to avoid this confusion? I don't know.

In any case, when doing this, the check may not have the intended behaviour since _WIN32 will be defined but the tools to call should still be named x86_64-w64-mingw32-TOOL.

That's what I meant, and it may not be an issue at all.

But you can’t use the build tool if it was compiled for another operating system. You should use the compiler for the system you intend to use nob on.

jgabaut commented 1 week ago

But you can’t use the build tool if it was compiled for another operating system. You should use the compiler for the system you intend to use nob on.

Thanks, I get it now! :)