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

Win make #61

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Win make #61

wants to merge 9 commits into from

Conversation

KarlKeiser
Copy link
Contributor

I cleaned up the script and removed some redundancies. near the top of the script, there are two macros to set the version numbers of two paths. Maybe there is a possibility to automatically find those version numbers (as those subdirectories are the only ones in their respective directory), but I'll have too look into it some more.

Copy link
Member

@c-bik c-bik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍.

Looks great. We are almost there!

This review has a few questions and improvement suggestions for your consideration.

/LIBPATH:"$(WIN_SDK_ROOT)\um\x64"\
/LIBPATH:"$(WIN_SDK_ROOT)\ucrt\x64"\
/LIBPATH:"%ERL_INTERFACE_DIR%\lib"\
/LIBPATH:"%ERL_INTERFACE_DIR%\include"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix indent.
four spaces, instead of a TAB everywhere



erloci.exe: erloci.lib priv\transcoder.obj priv\threads.obj priv\term.obj priv\port.obj priv\marshal.obj priv\logger.obj priv\erloci.obj priv\cmd_queue.obj priv\command.obj priv\erloci.obj
$(LINK) $(LINKFLAGS) /OUT:"priv\ocierl.exe" priv\transcoder.obj priv\threads.obj priv\term.obj priv\port.obj priv\marshal.obj priv\logger.obj priv\erloci.obj priv\cmd_queue.obj priv\command.obj priv\erloci.lib
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use some kind wildcard (like priv\*.obj) to select all .obj files?
Listing them isn't quite readable or maintainable!

# initialization done at the beginning of the installation
init: #
-@ if NOT EXIST "priv" mkdir "priv"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove trailing empty new lines!


# version of the Win SDK usually found in C:\Program Files (x86)\Windows Kits\10\Lib\
WIN_SDK_VERSION = 10.0.17134.0
# --------------------------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to autodetect these versions (or the path) instead of hardcoding them?

MSBuild c_src/erlocisln.sln /t:Clean /p:Configuration=$(conf) /p:Platform="x64" /m
del priv\*.lib priv\*.dll priv\*.exe /f /q
@echo Clean $(conf)
CPP = "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.15.26726\bin\Hostx64\x64\cl.exe"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be autodetectable instead of hardcoding?

@c-bik c-bik self-assigned this Aug 21, 2018
Karl Keiser added 2 commits August 24, 2018 08:26
fix some path/library dependencies, remove hardcoded version numbers
Copy link
Member

@c-bik c-bik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some cosmetics

  1. line break at 80 characters
  2. indent with 4 spaces

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants