Typo in 1Stop.cmd prevents building for x86

[wrljet]

A typo in 1Stop.cmd script prevents building for x86 target. If PLATFORM=X86 is set, the script uses "RETAILs" as the target.

Line 26 of 1Stop.cmd: set "build_cmd=makefile.bat RETAILs makefile.msvc 32 %*"

-Bill

[srorso]

Hi Bill:

I have committed a correction; would you please validate it and report your results? Thanks!

Best Regards, Steve Orso

[wrljet]

Steve, Yes, that does the trick.

I have another typo and some CMAKE comments in README.BuildWINSupp.txt Should I mention that here?

-Bill

[srorso]

Hi Bill:

Yes, please. Thanks for bringing this to our attention.

Best Regards,

Steve Orso

From: Bill Lewis [mailto:notifications@github.com] Sent: Friday, February 10, 2017 11:41 AM To: hercules-390/hyperion hyperion@noreply.github.com Cc: Stephen Orso stephen.orso@yahoo.com; Comment comment@noreply.github.com Subject: Re: [hercules-390/hyperion] Typo in 1Stop.cmd prevents building for x86 (#194)

Steve, Yes, that does the trick.

I have another typo and some CMAKE comments in README.BuildWINSupp.txt Should I mention that here?

-Bill

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/hercules-390/hyperion/issues/194#issuecomment-278994413 , or mute the thread https://github.com/notifications/unsubscribe-auth/AQWuNmWvcT8PV-XzlGKtz6gRop-yhKXiks5rbJMygaJpZM4L9ecp . https://github.com/notifications/beacon/AQWuNqsIbwaGBRcW35-WmRpxsq7wpzEMks5rbJMygaJpZM4L9ecp.gif

[wrljet]

Using CMAKE 3.7.2, to build PCRE 8.20, following the description in README.BuildWINSupp.txt.

I'm using VS2015 x86 Native Tools Command Prompt.

In the 32-bit steps, around line 250, step 4 mentions an ASM686 among the red options. I don't get that option.

But I do have one that must be checked to get the DLLs built: BUILD_SHARED_LIBS Without this I get EXEs.

Around line 260, there is a typo in the cmake arguments: cmake --build . -config release

-config should be --config

In makefile.bat, around line 745 in the :fullpath subroutine, the %~dpnx thing doesn't work if there is a trailing space on the end of the INCLUDE path where win32.mak actual exists. It took a bit of time and swearing to figure that out. Perhaps add a warning someplace about a trailing space?

-Bill

[wrljet]

EXTPKG_FLAGS.msvc :

Typo: the comment at the top of the file mentions SFLIB_FLAGS.msvc

The error msg displayed: "SFLIB_DIR "..\x86\s3fh\lib\SoftFloat.lib" does not exist. Check "..\x86\s3fh" contents."

That left me scratching my head until I looked in EXTPKG_FLAGS.msvc and saw that it really meant SoftFloat_D.lib. That could use $(SFLIB_LIB) to generate the entire path and filename and give a more exacting error.

-Bill

[srorso]

Hi Bill:

I have corrected the editorial issues you have been kind enough to identify (ASM686, BUILD_SHARED_LIBS), the typo of the CMake command, and the improvement of the EXTPKG_FLAGS.msvc script. The corrections will appear with my next commit for the build (shortly).

Regarding the trailing space, I put together a small test that I think captures the essence of the issue you describe and have confirmed that it does not correctly identify the path. Rather than warning the builder about this in the documentation, I would rather detect and issue a diagnostic in makefile.bat. That will take a bit of thought and maybe a few lines of code, perhaps looking for the string " ;" in the path variable "%PATH;" (Trailing semicolon needed to detect the issue when the last path in %PATH% has a trailing space.) The trick will be pointing out which path in %PATH% has the trailing space.

Thank you for your diligence in finding and diagnosing these issues; your efforts are appreciated.

Best Regards Steve Orso

[srorso]

Hi Bill:

A question on the trailing space matter: Did the directory in question have a trailing space in its name on disk or was the trailing space just present in the %PATH% or %INCLUDE% variable?

Best Regards, Steve Orso

[wrljet]

Steve,

Trailing space was in %INCLUDE% variable itself. What happened was, after I got things set up, echo'd %INCLUDE%, then highlighted in the DOS window to copy it. And pasted it into vi, in a setvars.bat I was making up. But I accidentally grabbed whitespace off the DOS window and didn't realize it.

I figured since I was going to use VS2008 or VS2015 command prompt to do the building, and they have their own include files, it would be best if I put the include for WIN32.MAK at the very end. So my extra space was at the very end of the entire %INCLUDE%. (I might have noticed it when I pasted it together if it had been at the front end)

[wrljet]

Something else that happened to me along the way with all this...

_dynamic_version.cmd uses the MS-DOS 'FIND' command. I happen to have a lot of unix/gnu related commands in my path, and FIND was one of them. I got a bunch of error msgs along the lines of: find: /i (some error I now forget). This was because my find.exe was ahead of C:\Windows\system32 in the PATH.

So after figuring out what was going on, a little Huh? and some helpful swearing, I fixed this by adding C:\Windows\system32 to the front of the PATH.

-Bill

[wrljet]

Typo in BZIP2_DIR.msvc: "winbuld" The same typo exists in PCRE_DIR.msvc and ZLIB_DIR.msvc

Example:

#  Modified by Stephen Orso to use either ..\winbuild\bzip2 or winbuld\bzip2
#  as default directories.

[srorso]

Hi Bill:

Many thanks for your detailed feedback and actionable suggestions. I have been pondering the trailing blank issue and shall propose the following:

If any of the paths in the %PATH% or %INCLUDE% variable do not exist when seeking win32.mak, issue a warning message identifying the missing path as a quoted string.

Trailing blanks in a Windows path name, on disk or in an environment variable, are not invalid per se. Very silly, very hard to work with, easy to miss when diagnosing, maybe only helpful to virus writers, but not invalid. So including warnings about them, for me, anyway, approach some of the product liability warning label silliness seen in the U.S.

But warning noting that a path in an environment variable does not exist might be useful and targeted to the occurrence of the issue. As a warning, if the missing path is not the one containing win32.mak, no harm and no inconvenience. But if it is, the warning will appear some few lines before makefile.bat's message about a missing win32.mak.

For what it's worth, I just dumped win32.mak in the VS2015 include directory.

Again, thanks for your generous efforts reviewing the Windows build and suggesting specific improvements.

Best Regards, Steve Orso

[srorso]

Hi Bill:

Changes committed for all issues reported; closing this issue. Please feel free to reopen this issue if the changes create difficulties for you, or to open a new issue should you identify other changes that should be made.

Thanks again for your care in finding these things. It is greatly appreciated.

Best Regards, Steve Orso