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

Add build commands for host dmd #4645

Merged
merged 1 commit into from
May 20, 2015
Merged

Conversation

andralex
Copy link
Member

This automatically downloads and installs the dmd compiler needed to boostrap the dmd build. cc @yebblies

@yebblies
Copy link
Member

I think this is too much complication for what it gives us.

  • None of the targets should depend on HOST_DC
  • Passing -conf is not always desired

I don't really understand what the rest does but it should be ok so long as it's only every manually invoked.

@braddr
Copy link
Member

braddr commented May 12, 2015

Dear god no. Do not have automatic downloading of anything please. I'm fine with detection of missing components and instructions of a manual step to take for installation, but this is horrid. It's also broken. The auto-tester is already properly setup and this change is causing downloads to occur are absolutely not needed.

@jacob-carlborg
Copy link
Contributor

This is only for Posix, not Windows.

@andralex
Copy link
Member Author

Whoa. Folks. Let's leave emotion out of this, "dear god no" etc hardly qualifies as a convincing argument.

@yebblies: The benefit here is that the setup is more user friendly to folks who don't have a previous version of dmd installed, or those who set up dmd to be the one under development. I belong to the latter category, so building dmd is quite difficult for me - e.g. make clean ruins all my prospects etc. I agree there should be a way for folks who already have dmd to just use it, so I'll update this. Specifying the configuration is necessary because there might be a ~/dmd.conf set up to use the development dmd (or may be corrupt or whatever). Self reliance and robustness is what we're looking for here.

@braddr: Could you please substantiate what the matter is with downloads. There is precedent in Phobos, a specific dmd is downloaded to build dub. After a few grudges I've learned to appreciate it. cc @MartinNowak

One more note: these few lines obviate the proposed use of dvm - another external tool with its own world.

@andralex
Copy link
Member Author

@braddr: I take it you'd be favorable with a solution that only downloads anything if $(HOST_DC) does not exist?


HOST_DMD_VER=2.067.1
HOST_DMD_ROOT=/tmp/.host_dmd-$(HOST_DMD_VER)
HOST_DMD_URL=http://downloads.dlang.org/releases/2015/dmd.$(HOST_DMD_VER).$(OS).zip
Copy link
Member

Choose a reason for hiding this comment

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

Please use http://downloads.dlang.org/releases/2.x/$(HOST_DMD_VER)/dmd.$(HOST_DMD_VER).$(OS).zip.

@MartinNowak
Copy link
Member

It makes a lot of sense to add bootstrapping to our build system.
Pinning the version of the host compiler also gives a more controlled build environment, e.g. avoids adding changes to ddmd that won't build with an older version of dmd.

One more note: these few lines obviate the proposed use of dvm - another external tool with its own world.

Yeah, let's please not go that path. Last attempt at deploying DVM for something useful was a disaster (travis-ci/travis-ci#730 (comment)). It's also a 5KLOC tool with a 15KLOC dependency on mambo.

@braddr: I take it you'd be favorable with a solution that only downloads anything if $(HOST_DC) does not exist?

Makes sense, e.g. the release build environment already comes with it's own host compiler.

@braddr
Copy link
Member

braddr commented May 14, 2015

The auto-tester is going to in the not terribly distant future disallow network access during a build. All prerequisites are required to be installed pre-build, not during. Validation of the build environment is a good practice. Construction of it is not. Providing functionality to assist with construction is fine, just not as a part of the build itself.

For any continuous integration system, they all aim to start with a clean, specifically constructed, environment, isolated from any past environments -- with varying levels of success. Having them downloading anything during the build step quickly multiplies. The auto-tester I run does about 1200 builds a day. Multiply that by 10 megs per build. I think it's safe to say that 12 gigs of redundant downloads is the wrong thing to do.

@andralex
Copy link
Member Author

All: thanks for the input. I'll change things to make auto-bootstrapping opt-in. Stay tuned.

@yebblies
Copy link
Member

The benefit here is that the setup is more user friendly to folks who don't have a previous version of dmd installed, or those who set up dmd to be the one under development. I belong to the latter category, so building dmd is quite difficult for me - e.g. make clean ruins all my prospects etc.

I would advise against using the just-built dmd for exactly this reason. That's what I did, up until 2.067.

I agree there should be a way for folks who already have dmd to just use it, so I'll update this. Specifying the configuration is necessary because there might be a ~/dmd.conf set up to use the development dmd (or may be corrupt or whatever).

If your installed dmd is 2.067, you might want to be using ~/dmd.conf.

I agree with Brad that any fully automatic download would be a bad idea. Manually invoking make download-dmd should be much better.

@MartinNowak
Copy link
Member

If your installed dmd is 2.067, you might want to be using ~/dmd.conf.

Right, but you don't want to use it with the downloaded DMD.

@yebblies
Copy link
Member

Right, but you don't want to use it with the downloaded DMD.

My understanding is that the line

+HOST_DC_RUN=$(HOST_DC) -conf=$(dir $(HOST_DC))dmd.conf

forces explicit conf on every setup.

@MartinNowak
Copy link
Member

You can override HOST_DC_RUN.

@andralex
Copy link
Member Author

OK, so now AUTO_BOOTSTRAP must be used explicitly for automatically taking care of old dmd.

# Host D compiler for bootstrapping
ifeq (,$(AUTO_BOOTSTRAP))
# No bootstrap, a $(HOST_DC) installation must be available
HOST_DC=dmd
Copy link
Member

Choose a reason for hiding this comment

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

Seems like you should still allow env variables, i.e. HOST_DC?=dmd.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK

@andralex
Copy link
Member Author

@MartinNowak: dunn. Destroy.

@jacob-carlborg
Copy link
Contributor

How about squashing the commits.

@andralex
Copy link
Member Author

@jacob-carlborg squashed

All: ping

@jacob-carlborg
Copy link
Contributor

It looks like you haven't addressed one of Martin's comments: #4645 (comment).

@jacob-carlborg
Copy link
Contributor

@MartinNowak
Copy link
Member

Auto-merge toggled on

MartinNowak added a commit that referenced this pull request May 20, 2015
Add build commands for host dmd
@MartinNowak MartinNowak merged commit 02e84cb into dlang:master May 20, 2015
@MartinNowak
Copy link
Member

The best way to download a file on Windows is Powershell.
https://gist.github.com/MartinNowak/8270666#file-setup-ps1-L19

@MartinNowak
Copy link
Member

It looks like you haven't addressed one of Martin's comments: #4645 (comment).

We can fix the url next time we update the compiler.

ifeq (,$(AUTO_BOOTSTRAP))
# No bootstrap, a $(HOST_DC) installation must be available
HOST_DC?=dmd
HOST_DC_FULL:=$(shell which $(HOST_DC))
Copy link
Member

Choose a reason for hiding this comment

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

This has broken specifying HOST_DC with an explicit -conf switch.

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.

5 participants