haskell / haskell-platform

Distribution of Haskell with batteries included
http://www.haskell.org/platform/
Other
380 stars 91 forks source link

Tweak to GhcInstallAction; distinguish a phonyTargetDir #254

Closed randen closed 7 years ago

randen commented 8 years ago

These are two commits that have been needed for the Windows installer builds. For the Windows HP build, the installation looks different on the user's machine from the other HP build environments. Due to this, these changes are specific to the Windows HP but are in code common to all environments. I am unable to test these changes on those other build environments, so these changes should be applied with that caveat. I have tried to make these edits innocuous to the other environments, but from HP build/installation problems in the past, I observe that those other environments have their own delicateness that might be affected. The one involving GhcInstallAction is very unlikely to have any imact, but I am more concerned with the phonyTargetDir change's effects.

gbaz commented 8 years ago

Thanks randy. Do you mind explaining the motivation a bit more when you get a chance. I think the latter change especially might be a good thing elsewhere, if anything...

randen commented 7 years ago

@gbaz (I am able to get back to this now, sorry.) There are more details in each of the two commits in this PR. Do those provide sufficient info for you? (I probably got the git process wrong by putting details in each commit rather than in the PR?)

gbaz commented 7 years ago

Ah -- my mistake as well for not seeing them there... I think I changed a related thing for mac already!

For posterity here's the details:

Allow a GhcInstallAction to skip directory enumeration

The Windows build is different from the other builds in a few ways, one of which is very important for this commit:

In the osGhcTargetInstall function in the Windows builds, the directory is not completed at the point this action is used, so we cannot have the Shake system enumerating this directory for dependencies too soon. So we must make its osGhcTargetInstall no longer returns a directory, which prevents the vTarget code in Dirs.hs from making dependencies of the directory contents too soon.

This change is part of a larger change but it is separated here as this isolates any effects on the common code. The Windows-specific changes will be in a separate commit for bookkeeping reasons. The GhcInstallAction type is only used internally to GhcDist and for the Windows case where it is the only build (currently) to use the GhcInstallCustom variant of the GhcInstall type.

Un-overload the "targetDir" target

If a FilePath matches a so-called "phony" target, Shake will count this as satisfying the phony target and skip its steps. Currently, the target called targetDir is overloaded, being used in a "~>" rule (for defining a "phony" target) but it is also used for contructing file paths, and it appears as a dependency in some rules (e.g., "need [targetDir]"). When it is a dependency, it actually should be upon a phony target, not the actual directory. Due to how dependencies are constructed for the Windows HP build, this problem has plagued it.

This change needs to be tested for the other build environments.

gbaz commented 7 years ago

I'll merge and pull later today -- lgtm.