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

Update compilers in Windows #18

Closed
wants to merge 4 commits into from
Closed

Update compilers in Windows #18

wants to merge 4 commits into from

Conversation

mondul
Copy link
Contributor

@mondul mondul commented Oct 25, 2019

Removed Visual C++ for MinGW. When compiling before, the extractor was not working correctly for binary files, as all new lines (\n) were being changed for carriage return plus new line (\r\n), hence binary files were being corrupted. This is due how the POSIX file operations were implemented by the compilers, assuming all manipulated files as non-binaries.

Another advantage on changing the compiler is that the resulting .exe does not require the Visual C++ runtime.

NOTES:

  • argp.h was taken from the noahp's cflow-mingw repo. Link.
  • The utimes function code was taken from the kdewin repo. Link.

Removed Visual C++ for MinGW. When compiling before, the extractor was not working correctly for binary files, as all new lines (\n) were being changed for carriage return plus new line (\r\n), hence binary files were being corrupted. This is due how the POSIX file operations were implemented by the compilers, assuming all manipulated files as non-binaries.

Another advantage on changing the compiler is that the resulting .exe does not require the Visual C++ runtime.
@Gregwar
Copy link
Owner

Gregwar commented Oct 25, 2019

Hello,
This way is incompatible with #17 which seems to give emphasis to MSVC instead
@Redfoxmoon3 and @mondul : Can you two specify what compiling toolchain you are using on Windows ?

@hiirotsuki
Copy link

hiirotsuki commented Oct 25, 2019

With my PR MinGW-w64 and MSVC are supported, one problem remains however that binary files are not treated as binary on windows. I haven't gotten time to fix this yet for both compiler targets.

@Gregwar
Copy link
Owner

Gregwar commented Oct 25, 2019

@Redfoxmoon3 so _MSC_VER is actually set by MinGW as well?
Isn't the binary files issues just about passing the b flag to fopen and so?

@hiirotsuki
Copy link

Code unique to msvc is guarded by _MSC_VER as mingw-w64 does not define it, both define _WIN32. for fopen setting b should do it, I think?

@mondul
Copy link
Contributor Author

mondul commented Oct 25, 2019

I just used mingw32 as I'm using a 32-bit Windows 7 virtual machine for compilation. That way I can reach a larger user base who does not have a Visual C++ runtime installed. With this approach I was able to change xoptarg, which needs MFC support, for a more simple solution.

@mondul
Copy link
Contributor Author

mondul commented Oct 25, 2019

Applied some of the changes made in #17 and now Visual Studio can be used for compilation.

@mondul mondul changed the title Change compiler in Windows Update compilers in Windows Oct 25, 2019
@Gregwar
Copy link
Owner

Gregwar commented Oct 28, 2019

@Redfoxmoon3 are you ok with this PR? Does it build for you?

@hiirotsuki
Copy link

hiirotsuki commented Oct 28, 2019

It now includes both a non-licensed source code file, and GPL source code files, which both makes this GPL and non-free, it's not merge-able like this. Adding binary flags to the various I/O functions on top of my PR should be enough to satisfy both compilers and fix the actual problem.

@mondul
Copy link
Contributor Author

mondul commented Oct 28, 2019

@Redfoxmoon3 @Gregwar Hi, just read about the licensing issue and reviewed both MIT and GPL licenses with my lawyer. Making it GPL does not mean it is no longer going to be free, but all changes must be disclosed in source and always include the license. I understand if the author would not like to change the licensing due the last part of the GPL stating "The GNU General Public License does not permit incorporating your program into proprietary programs", so I'm closing this pull request and deleting my fork.

@mondul mondul closed this Oct 28, 2019
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

Successfully merging this pull request may close these issues.

3 participants