Skip to content
This repository has been archived by the owner on Aug 2, 2020. It is now read-only.

For executables, we should read the main-is field from the cabal file. #627

Merged
merged 2 commits into from
Jun 18, 2018
Merged

For executables, we should read the main-is field from the cabal file. #627

merged 2 commits into from
Jun 18, 2018

Conversation

sighingnow
Copy link
Contributor

Previously, we simply treat file name for Main module as Main.hs to build executables. That doesn't work for the timeout program. This patch fixes the problem.

Previously, we simply treat file name for `Main` module as `Main.hs` to
build executable. That doesn't work for the `timeout` program. This patch
fixes the problem.
@sighingnow
Copy link
Contributor Author

This pr blocks #499 .

@alpmestan
Copy link
Collaborator

Looks reasonable to me.

Just (mod, filepath) ->
concatForM dirs $ \dir -> do
found <- doesFileExist (dir -/- filepath)
return $ if found then [(mod, unifyPath $ dir -/- filepath)]
Copy link
Owner

Choose a reason for hiding this comment

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

[ (mod, unifyPath $ dir -/- filepath) | found ] might be less awkward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@@ -136,7 +141,16 @@ moduleFilesOracle = void $ do
let cmp f = compare (dropExtension f)
found = intersectOrd cmp files mFiles
return (map (fullDir -/-) found, mDir)
let pairs = sort [ (encodeModule d f, f) | (fs, d) <- result, f <- fs ]

mainpairs <- case mainIs of
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add a comment here, explaining why mains are treated differently?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@snowleopard
Copy link
Owner

@sighingnow Many thanks! Just a couple of minor comments above. Please add a comment to clarify what's going on.

@sighingnow
Copy link
Contributor Author

@snowleopard Could we land this now ?

@snowleopard
Copy link
Owner

@sighingnow Yes, thanks! I'm just waiting for at least one green light from Travis (I believe 8.2.2 was fine).

@snowleopard snowleopard merged commit f319243 into snowleopard:master Jun 18, 2018
@snowleopard
Copy link
Owner

Merged.

@sighingnow
Copy link
Contributor Author

Thanks !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants