Skip to content

Issue a nicer error message if critical envvars are missing #7256

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.

Sign up for GitHub

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

Closed
andralex wants to merge 1 commit into from
Closed

Issue a nicer error message if critical envvars are missing #7256

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/win32.mak
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ OS=windows

##### Directories

# DM_HOME should be defined in the user's environment
DM_HOME=Please_define_the__DM_HOME__environment_variable
Copy link
Copy Markdown
Member

@rainers rainers Oct 28, 2017

Choose a reason for hiding this comment

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

I never set DM_HOME (and the required naming does not fit my installation), the C compiler already knows its standard include folders.

Copy link
Copy Markdown
Member Author

@andralex andralex Oct 28, 2017

Choose a reason for hiding this comment

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

@rainers what if it's in a non-default location?

Copy link
Copy Markdown
Member

@rainers rainers Oct 29, 2017

Choose a reason for hiding this comment

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

dmc is not in the default location on my system, but in c:\l\dmc instead. It uses sc.ini, too, containing INCLUDE="%@P%\..\include";"%@P%\..\mfc\include";%INCLUDE% which points to the standard include folders.

Copy link
Copy Markdown
Member Author

@andralex andralex Oct 29, 2017

Choose a reason for hiding this comment

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

Cool, then can you please submit a small PR to eliminate it entirely? Also it would be great if you also updated the wiki instructions. Thx!

Copy link
Copy Markdown
Member

@rainers rainers Oct 30, 2017

Choose a reason for hiding this comment

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

See #7260
The wiki uses DM_HOME to refer to the DMD installation directory, that seems ok.


# DMC directory
DMCROOT=$(DM_HOME)\dm
# DMD source directories
Expand Down Expand Up @@ -93,6 +96,8 @@ TOOLS_DIR=..\..\tools
CC=dmc
# D compiler (set with env variable)
#HOST_DC=dmd
# For error detection:
HOST_DC=Please_define_the__HOST_DC__environment_variable
Copy link
Copy Markdown
Member

@rainers rainers Oct 28, 2017

Choose a reason for hiding this comment

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

This works to display a nicer error, but it no longer allows setting HOST_DC in the environment. It is only overwritten on the command line.

Copy link
Copy Markdown
Member Author

@andralex andralex Oct 28, 2017

Choose a reason for hiding this comment

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

On wine at least it works with HOST_DC set in the environment.

Copy link
Copy Markdown
Member Author

@andralex andralex Oct 28, 2017

Choose a reason for hiding this comment

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

Could you try it on native Windows? Thx!

Copy link
Copy Markdown
Member

@rainers rainers Oct 29, 2017

Choose a reason for hiding this comment

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

That's what I did. Are you sure to invoke DigitalMars make?

An explicit test of the variables works, too, but has to be added as a dependency to the defaulttarget and maybe others:

checkvars:
	cmd /c if "$(HOST_DC)" == "" (echo Please set HOST_DC && exit 1)

(DM make does not seem to interpret the result of executing an implicit shell command, so I had to make it explicit).

Copy link
Copy Markdown
Member Author

@andralex andralex Oct 29, 2017

Choose a reason for hiding this comment

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

Interesting, I'll look into this. For what it's worth I use wine make -f win32.mak that I assume uses dmd make, but I'll check when I get back to that machine.

Copy link
Copy Markdown
Contributor

@wilzbach wilzbach Dec 10, 2017

Choose a reason for hiding this comment

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

BTW we could also set the DC_HOST variable to dmd which is the default that's used for posix.mak and then simply add a nice error message as @rainers suggested.

(if DM make supports this)

Copy link
Copy Markdown
Contributor

@marler8997 marler8997 Dec 19, 2017

Choose a reason for hiding this comment

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

I tried to get this "default" functionality working with DM make and failed...

Copy link
Copy Markdown
Contributor

@marler8997 marler8997 Dec 19, 2017

Choose a reason for hiding this comment

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

Nevermind, got it it to work (#7468).

# Make program
MAKE=make
# Librarian
Expand Down Expand Up @@ -627,4 +632,3 @@ $G\ldfpu.obj : vcbuild\ldfpu.asm
############################## Generated Rules ###############################

# These rules were generated by makedep, but are not currently maintained