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

Fix Issue 12205: update posix.mak to use local, recently-built druntime and phobos #117

Merged
1 commit merged into from Feb 26, 2014

Conversation

Projects
None yet
9 participants
@WebDrake
Contributor

WebDrake commented Feb 19, 2014

This fixes https://d.puremagic.com/issues/show_bug.cgi?id=12205 by adding library/include paths to the locations where druntime and phobos have been built, plus a flag to prevent/suppress alignment errors in one of the builds.

In the event that the user wishes to build using the system dmd instead of the locally-built ../dmd/src/dmd, this is trivial to achieve by overriding with DMD=dmd. DFLAGS will also then be overridden with those from the system dmd.conf.

@MartinNowak

This comment has been minimized.

Show comment
Hide comment
@MartinNowak

MartinNowak Feb 19, 2014

Member

Let's add a working default dmd.conf/sc.ini to dmd/src instead.

Member

MartinNowak commented Feb 19, 2014

Let's add a working default dmd.conf/sc.ini to dmd/src instead.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 19, 2014

Contributor

OK, I'll give it a go tomorrow. The current solution was based on what I found in the druntime and phobos posix.mak files, but of course they have specialized needs that don't apply to tools.

Contributor

WebDrake commented Feb 19, 2014

OK, I'll give it a go tomorrow. The current solution was based on what I found in the druntime and phobos posix.mak files, but of course they have specialized needs that don't apply to tools.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 19, 2014

Contributor

Actually ... isn't dmd.conf searched for first in current working directory, then in path of dmd, then in /etc/, ... ? Could it be better to have a dmd.conf in the tools project? The reason I ask is because I'm wondering if a dmd.conf in dmd/src might interfere with the druntime and phobos builds.

Contributor

WebDrake commented Feb 19, 2014

Actually ... isn't dmd.conf searched for first in current working directory, then in path of dmd, then in /etc/, ... ? Could it be better to have a dmd.conf in the tools project? The reason I ask is because I'm wondering if a dmd.conf in dmd/src might interfere with the druntime and phobos builds.

@yebblies

This comment has been minimized.

Show comment
Hide comment
@yebblies

yebblies Feb 20, 2014

Member

Let's add a working default dmd.conf/sc.ini to dmd/src instead.

Let's not. Because then I won't be able to override the default with my own if I have a different layout, as sc.ini will be in version control.

Member

yebblies commented Feb 20, 2014

Let's add a working default dmd.conf/sc.ini to dmd/src instead.

Let's not. Because then I won't be able to override the default with my own if I have a different layout, as sc.ini will be in version control.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 20, 2014

Contributor

Let's not. Because then I won't be able to override the default with my own if I have a different layout, as sc.ini will be in version control.

TBH I think the tools build system pretty much assumes a certain standard layout these days. The move from the druntime/phobos builds using DMD = dmd to using DMD = ../dmd/src/dmd reflects that, and other aspects (e.g. the docs/website builds) have relied on a certain standard layout for even longer.

Contributor

WebDrake commented Feb 20, 2014

Let's not. Because then I won't be able to override the default with my own if I have a different layout, as sc.ini will be in version control.

TBH I think the tools build system pretty much assumes a certain standard layout these days. The move from the druntime/phobos builds using DMD = dmd to using DMD = ../dmd/src/dmd reflects that, and other aspects (e.g. the docs/website builds) have relied on a certain standard layout for even longer.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 20, 2014

Contributor

Looking at dmd.conf was productive at least in finding the solution :-) The newly-updated patch fixes the alignment problems by including the -L--no-warn-search-mismatch flag. The whole set of tools now builds correctly.

The remaining problem of this approach is that it's now non-trivial for the user to build using an alternative to ../dmd/src/dmd simply by adding a DMD=... statement when calling make. So, it could be a good idea to make the in-Makefile definition of DFLAGS dependent on DMD being non-equal to dmd (which can be assumed to have its own dmd.conf in place specifying the DFLAGS).

Contributor

WebDrake commented Feb 20, 2014

Looking at dmd.conf was productive at least in finding the solution :-) The newly-updated patch fixes the alignment problems by including the -L--no-warn-search-mismatch flag. The whole set of tools now builds correctly.

The remaining problem of this approach is that it's now non-trivial for the user to build using an alternative to ../dmd/src/dmd simply by adding a DMD=... statement when calling make. So, it could be a good idea to make the in-Makefile definition of DFLAGS dependent on DMD being non-equal to dmd (which can be assumed to have its own dmd.conf in place specifying the DFLAGS).

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 20, 2014

Contributor

Latest commit allows user to easily make use of system dmd for the build if they so wish.

Contributor

WebDrake commented Feb 20, 2014

Latest commit allows user to easily make use of system dmd for the build if they so wish.

Show outdated Hide outdated posix.mak Outdated
Show outdated Hide outdated posix.mak Outdated
Fix Issue 12205: update posix.mak to use local, recently-built drunti…
…me and phobos.

This fixes https://d.puremagic.com/issues/show_bug.cgi?id=12205
by adding library/include paths to the locations where druntime
and phobos have been built, plus a flag to prevent/suppress
alignment errors in one of the builds.

In the event that the user wishes to build using the system dmd
instead of the locally-built ../dmd/src/dmd, this is trivial to
achieve by overriding with DMD=dmd.  DFLAGS will also then be
overridden with those from the system dmd.conf.
@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 21, 2014

Contributor

OK, I've fixed, rejigged, rebased and re-pushed. The updated patch:

  • fixes the extension for DRUNTIMESO (thanks Luca!);
  • brings the MODEL_FLAG outside the DFLAGS again so that it can be used even if DMD is overridden.
Contributor

WebDrake commented Feb 21, 2014

OK, I've fixed, rejigged, rebased and re-pushed. The updated patch:

  • fixes the extension for DRUNTIMESO (thanks Luca!);
  • brings the MODEL_FLAG outside the DFLAGS again so that it can be used even if DMD is overridden.
@mihails-strasuns

This comment has been minimized.

Show comment
Hide comment
@mihails-strasuns

mihails-strasuns Feb 24, 2014

This pull is very desired to simplify tool packaging.

mihails-strasuns commented Feb 24, 2014

This pull is very desired to simplify tool packaging.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 26, 2014

Contributor

Ping.

Come on, the tools build is currently broken -- this patch fixes it, without imposing in any way on any other project.

We can always implement a better or more comprehensive build design later, but this really ought to land.

Contributor

WebDrake commented Feb 26, 2014

Ping.

Come on, the tools build is currently broken -- this patch fixes it, without imposing in any way on any other project.

We can always implement a better or more comprehensive build design later, but this really ought to land.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost commented Feb 26, 2014

@MartinNowak @yebblies Ok to merge?

@DmitryOlshansky

This comment has been minimized.

Show comment
Hide comment
@DmitryOlshansky

DmitryOlshansky Feb 26, 2014

Member

Hm I thought that tools were meant to be build with working DMD installation/dmd.conf.
Is that the main problem being solved here - building with freshly built DMD w/o dmd.conf?

Member

DmitryOlshansky commented Feb 26, 2014

Hm I thought that tools were meant to be build with working DMD installation/dmd.conf.
Is that the main problem being solved here - building with freshly built DMD w/o dmd.conf?

@mihails-strasuns

This comment has been minimized.

Show comment
Hide comment
@mihails-strasuns

mihails-strasuns Feb 26, 2014

Yes, it is the problem. All other repositories work without dmd.conf or any sort of global setup, tools being exception only complicates packaging.

mihails-strasuns commented Feb 26, 2014

Yes, it is the problem. All other repositories work without dmd.conf or any sort of global setup, tools being exception only complicates packaging.

@DmitryOlshansky

This comment has been minimized.

Show comment
Hide comment
@DmitryOlshansky

DmitryOlshansky Feb 26, 2014

Member

Well, since I don't compile tools on daily basis and I see the easy workaround of DMD=dmd I have no problem with this pull, let's merge it.

Member

DmitryOlshansky commented Feb 26, 2014

Well, since I don't compile tools on daily basis and I see the easy workaround of DMD=dmd I have no problem with this pull, let's merge it.

ghost pushed a commit that referenced this pull request Feb 26, 2014

Merge pull request #117 from WebDrake/makefile-dflags
Fix Issue 12205: update posix.mak to use local, recently-built druntime and phobos

@ghost ghost merged commit d306f7f into dlang:master Feb 26, 2014

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost commented Feb 26, 2014

Done.

@mihails-strasuns

This comment has been minimized.

Show comment
Hide comment
@mihails-strasuns

mihails-strasuns Feb 26, 2014

Awesome, thanks!

mihails-strasuns commented Feb 26, 2014

Awesome, thanks!

@MartinNowak

This comment has been minimized.

Show comment
Hide comment
@MartinNowak

MartinNowak Feb 27, 2014

Member

This broke existing build scripts for tools, matching magic names as in DMD=dmd is problematic and -L-no-warn-search-mismatch doesn't work on FreeBSD.

Member

MartinNowak commented Feb 27, 2014

This broke existing build scripts for tools, matching magic names as in DMD=dmd is problematic and -L-no-warn-search-mismatch doesn't work on FreeBSD.

# Set DFLAGS
ifneq (dmd,$(DMD))
DFLAGS = -I$(DRUNTIME_PATH)/import -I$(PHOBOS_PATH) -L-L$(PHOBOS_PATH)/generated/$(OS)/release/$(MODEL) -L--no-warn-search-mismatch $(DMDEXTRAFLAGS) -w
endif

This comment has been minimized.

@MartinNowak

MartinNowak Feb 27, 2014

Member

Remove the ifneq here and append the paths to DMD not DFLAGS, so that someone overriding DMD doesn't get wrong include/link paths.
Why do you need no-warn-search-mismatch if you already know the MODEL?

@MartinNowak

MartinNowak Feb 27, 2014

Member

Remove the ifneq here and append the paths to DMD not DFLAGS, so that someone overriding DMD doesn't get wrong include/link paths.
Why do you need no-warn-search-mismatch if you already know the MODEL?

This comment has been minimized.

@WebDrake

WebDrake Feb 27, 2014

Contributor

On balance I think it would be better to check for strict equality to ../dmd/src/dmd. Re DFLAGS: this is how it's done in the other build scripts (for druntime and phobos), see e.g.: https://github.com/D-Programming-Language/phobos/blob/master/posix.mak#L132 -- so I tried to follow the same principles here.

Re the need for no-warn-search-mismatch: all I can say is that absent this setting, I was getting horrible errors; with it, I wasn't; and if I recall, that same issue applied to trying to build anything with any dmd on my system, ever. It's one of the recommended dmd.conf settings: http://wiki.dlang.org/Building_DMD#Installation

@WebDrake

WebDrake Feb 27, 2014

Contributor

On balance I think it would be better to check for strict equality to ../dmd/src/dmd. Re DFLAGS: this is how it's done in the other build scripts (for druntime and phobos), see e.g.: https://github.com/D-Programming-Language/phobos/blob/master/posix.mak#L132 -- so I tried to follow the same principles here.

Re the need for no-warn-search-mismatch: all I can say is that absent this setting, I was getting horrible errors; with it, I wasn't; and if I recall, that same issue applied to trying to build anything with any dmd on my system, ever. It's one of the recommended dmd.conf settings: http://wiki.dlang.org/Building_DMD#Installation

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Feb 27, 2014

Oops. I guess we could use an autotester for the tools repo.

ghost commented Feb 27, 2014

Oops. I guess we could use an autotester for the tools repo.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 27, 2014

Contributor

This broke existing build scripts for tools

Just to mention: the whole reason for this PR was that the build scripts were already broken, and they must have been broken for everyone. I'd have been happy to fix it simply by setting DMD=dmd by default, but the change to ../dmd/src/dmd was obviously deliberate and so I prepared a fix to support that.

-L-no-warn-search-mismatch doesn't work on FreeBSD

Ack :-(

The reason it's there is because I realized it's one of the recommended flags in the standard dmd.conf and enabling it prevented the linking errors I encountered previously:
http://forum.dlang.org/post/mailman.9.1392159987.6445.digitalmars-d-learn@puremagic.com

However, I don't understand why I was getting those matching errors in the first place, given that the MODEL settings were identical for everything being build (dmd, druntime, phobos, tools).

Oops. I guess we could use an autotester for the tools repo.

Yes, I think that's essential -- I'm amazed it's not there already. It's very frustrating that this patch broke things for FreeBSD.

Contributor

WebDrake commented Feb 27, 2014

This broke existing build scripts for tools

Just to mention: the whole reason for this PR was that the build scripts were already broken, and they must have been broken for everyone. I'd have been happy to fix it simply by setting DMD=dmd by default, but the change to ../dmd/src/dmd was obviously deliberate and so I prepared a fix to support that.

-L-no-warn-search-mismatch doesn't work on FreeBSD

Ack :-(

The reason it's there is because I realized it's one of the recommended flags in the standard dmd.conf and enabling it prevented the linking errors I encountered previously:
http://forum.dlang.org/post/mailman.9.1392159987.6445.digitalmars-d-learn@puremagic.com

However, I don't understand why I was getting those matching errors in the first place, given that the MODEL settings were identical for everything being build (dmd, druntime, phobos, tools).

Oops. I guess we could use an autotester for the tools repo.

Yes, I think that's essential -- I'm amazed it's not there already. It's very frustrating that this patch broke things for FreeBSD.

@braddr

This comment has been minimized.

Show comment
Hide comment
@braddr

braddr Feb 27, 2014

Member

It's not part of the auto-tester because for most of it's life it's not been anywhere remotely in a testable state. If it's ready (meaning that it's as simple as: checkout; make; make unittest -- on all platforms) then I'll add it to my todo list.

Member

braddr commented Feb 27, 2014

It's not part of the auto-tester because for most of it's life it's not been anywhere remotely in a testable state. If it's ready (meaning that it's as simple as: checkout; make; make unittest -- on all platforms) then I'll add it to my todo list.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Feb 27, 2014

Contributor

It's not part of the auto-tester because for most of it's life it's not been anywhere remotely in a testable state. If it's ready (meaning that it's as simple as: checkout; make; make unittest -- on all platforms) then I'll add it to my todo list.

Well, the current patch ought to make it so, at least to the same extent as with druntime and phobos -- that is: it assumes that you have already built dmd, druntime and phobos and they are in ../dmd/, ../druntime/ and ../phobos/ respectively. There's currently no unittest target, though.

Contributor

WebDrake commented Feb 27, 2014

It's not part of the auto-tester because for most of it's life it's not been anywhere remotely in a testable state. If it's ready (meaning that it's as simple as: checkout; make; make unittest -- on all platforms) then I'll add it to my todo list.

Well, the current patch ought to make it so, at least to the same extent as with druntime and phobos -- that is: it assumes that you have already built dmd, druntime and phobos and they are in ../dmd/, ../druntime/ and ../phobos/ respectively. There's currently no unittest target, though.

@pszturmaj

This comment has been minimized.

Show comment
Hide comment
@pszturmaj

pszturmaj Feb 27, 2014

@braddr Excuse me for posting it here... I can't reply to your email, I'm always receiving mail delivery error:
The mail system your@mail.com: host your.mailhost.com[xx.xx.xx.xx] said: 451
4.7.1
Please try again later (TEMPFAIL) (in reply to RCPT TO command)

Sorry for the noise...

pszturmaj commented Feb 27, 2014

@braddr Excuse me for posting it here... I can't reply to your email, I'm always receiving mail delivery error:
The mail system your@mail.com: host your.mailhost.com[xx.xx.xx.xx] said: 451
4.7.1
Please try again later (TEMPFAIL) (in reply to RCPT TO command)

Sorry for the noise...

@Orvid

This comment has been minimized.

Show comment
Hide comment
@Orvid

Orvid Feb 28, 2014

--no-warn-search-mismatch prevents the makefile from compiling successfully on OSX due to the fact that OSX's ld needs it passed as -L-no_arch_warnings

Orvid commented on 0db1890 Feb 28, 2014

--no-warn-search-mismatch prevents the makefile from compiling successfully on OSX due to the fact that OSX's ld needs it passed as -L-no_arch_warnings

@MartinNowak

This comment has been minimized.

Show comment
Hide comment
@MartinNowak

MartinNowak Feb 28, 2014

Member

So who is taking care of fixing the no-warn-search-mismatch issue?
I don't see why it was added in the begging.

The reason it's there is because I realized it's one of the recommended flags in the standard dmd.conf and enabling it prevented the linking errors I encountered previously:
http://forum.dlang.org/post/mailman.9.1392159987.6445.digitalmars-d-learn@puremagic.com

Recheck the link error, it doesn't look like the architectures were mixed up.

Member

MartinNowak commented Feb 28, 2014

So who is taking care of fixing the no-warn-search-mismatch issue?
I don't see why it was added in the begging.

The reason it's there is because I realized it's one of the recommended flags in the standard dmd.conf and enabling it prevented the linking errors I encountered previously:
http://forum.dlang.org/post/mailman.9.1392159987.6445.digitalmars-d-learn@puremagic.com

Recheck the link error, it doesn't look like the architectures were mixed up.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Mar 1, 2014

Contributor

So who is taking care of fixing the no-warn-search-mismatch issue?

I'm happy to provide an update, but I don't have Mac or FreeBSD systems to test on. Do I understand right that on FreeBSD the build works if the -no-warn-search-mismatch flag is entirely removed?

Recheck the link error, it doesn't look like the architectures were mixed up.

I don't understand the link error. I did ask for assistance with this.

I added the flag because not only did it prevent the link error but to my recollection it's a recommended flag to use with D on Linux (I have a recollection of regularly encountering these kinds of linker errors with self-built dmd installs, until someone pointed out to add that flag to dmd.conf).

Contributor

WebDrake commented Mar 1, 2014

So who is taking care of fixing the no-warn-search-mismatch issue?

I'm happy to provide an update, but I don't have Mac or FreeBSD systems to test on. Do I understand right that on FreeBSD the build works if the -no-warn-search-mismatch flag is entirely removed?

Recheck the link error, it doesn't look like the architectures were mixed up.

I don't understand the link error. I did ask for assistance with this.

I added the flag because not only did it prevent the link error but to my recollection it's a recommended flag to use with D on Linux (I have a recollection of regularly encountering these kinds of linker errors with self-built dmd installs, until someone pointed out to add that flag to dmd.conf).

@WebDrake WebDrake deleted the WebDrake:makefile-dflags branch Mar 1, 2014

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Mar 1, 2014

Contributor

Please see #120 for a patch that should hopefully fix reported issues. I'll need help testing it as I don't have access to an OS X or FreeBSD machine.

Contributor

WebDrake commented Mar 1, 2014

Please see #120 for a patch that should hopefully fix reported issues. I'll need help testing it as I don't have access to an OS X or FreeBSD machine.

@MartinNowak MartinNowak referenced this pull request Mar 1, 2014

Merged

fix tools build #121

@MartinNowak

This comment has been minimized.

Show comment
Hide comment
@MartinNowak

MartinNowak Mar 1, 2014

Member

Let's not. Because then I won't be able to override the default with my own if I have a different layout, as sc.ini will be in version control.

It would be more tedious to override the default, but it's possible.
What's preventing you from using the default layout? It would be a much cleaner and simpler solution to have a working dmd.conf/sc.ini by default.

Spreading all of those DFLAGS around causes a lot of issues and duplication.
For example I just tried to add a unittest target to the tools makefile and it isn't easily possible to pass include/linker flags to rdmd_test, so I need dmd.conf again.

Member

MartinNowak commented Mar 1, 2014

Let's not. Because then I won't be able to override the default with my own if I have a different layout, as sc.ini will be in version control.

It would be more tedious to override the default, but it's possible.
What's preventing you from using the default layout? It would be a much cleaner and simpler solution to have a working dmd.conf/sc.ini by default.

Spreading all of those DFLAGS around causes a lot of issues and duplication.
For example I just tried to add a unittest target to the tools makefile and it isn't easily possible to pass include/linker flags to rdmd_test, so I need dmd.conf again.

@WebDrake

This comment has been minimized.

Show comment
Hide comment
@WebDrake

WebDrake Mar 2, 2014

Contributor

For reference -- as discussed in #120 and #121 -- it looks like one reason some people may not have been seeing the linker errors I encountered is if they have an /etc/dmd.conf file in place which defines DFLAGS. This will have been picked up on by the tools build process and may therefore have hidden errors that I was encountering (because I store dmd.conf with the dmd binary, not in a general config location).

So, worth checking for everyone: (i) do you have an /etc/dmd.conf on your system, and (ii) does it make use of the -L--no-warn-search-mismatch flag?

I have a horrible suspicion that the whole reason I ever added it to my dmd.conf was to solve linker problems with the tools build (then using system-installed dmd), and IIRC it's my dmd.conf that was used as the basis of the canonical one on the D wiki. So it looks like this was a longstanding bug in the tools linker commands that in turn has led to longstanding misinformation on the D wiki page. :-(

Suffice to say that @MartinNowak's recent patch allows the tools build to succeed for me without the -L--no-warn-search-mismatch flag, so people may wish to check their dmd.conf settings and see if they can remove the flag (if present) without ill effects.

Contributor

WebDrake commented Mar 2, 2014

For reference -- as discussed in #120 and #121 -- it looks like one reason some people may not have been seeing the linker errors I encountered is if they have an /etc/dmd.conf file in place which defines DFLAGS. This will have been picked up on by the tools build process and may therefore have hidden errors that I was encountering (because I store dmd.conf with the dmd binary, not in a general config location).

So, worth checking for everyone: (i) do you have an /etc/dmd.conf on your system, and (ii) does it make use of the -L--no-warn-search-mismatch flag?

I have a horrible suspicion that the whole reason I ever added it to my dmd.conf was to solve linker problems with the tools build (then using system-installed dmd), and IIRC it's my dmd.conf that was used as the basis of the canonical one on the D wiki. So it looks like this was a longstanding bug in the tools linker commands that in turn has led to longstanding misinformation on the D wiki page. :-(

Suffice to say that @MartinNowak's recent patch allows the tools build to succeed for me without the -L--no-warn-search-mismatch flag, so people may wish to check their dmd.conf settings and see if they can remove the flag (if present) without ill effects.

This issue was closed.

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