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

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

WebDrake
Copy link
Contributor

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
Copy link
Member

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

@WebDrake
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
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.

@WebDrake
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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


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

Choose a reason for hiding this comment

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

I rolled MODEL_FLAG into this, but it might in principle still be preferable to keep it independent of DFLAGS so that it will still be used even in the event of the user calling the system dmd.

…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
Copy link
Contributor Author

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
Copy link

This pull is very desired to simplify tool packaging.

@WebDrake
Copy link
Contributor Author

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
Copy link

ghost commented Feb 26, 2014

@MartinNowak @yebblies Ok to merge?

@DmitryOlshansky
Copy link
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?

@mihails-strasuns
Copy link

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
Copy link
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.

ghost pushed a commit that referenced this pull request Feb 26, 2014
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
Copy link

ghost commented Feb 26, 2014

Done.

@mihails-strasuns
Copy link

Awesome, thanks!

@MartinNowak
Copy link
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.

# 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
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
Copy link

ghost commented Feb 27, 2014

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

@WebDrake
Copy link
Contributor Author

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
Copy link
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
Copy link
Contributor Author

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
Copy link

@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...

@MartinNowak
Copy link
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.

@WebDrake
Copy link
Contributor Author

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 makefile-dflags branch March 1, 2014 22:34
@WebDrake
Copy link
Contributor Author

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 mentioned this pull request Mar 1, 2014
@MartinNowak
Copy link
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.

@WebDrake
Copy link
Contributor Author

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 pull request was closed.
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.

9 participants