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

Simple.SrcDist cleanup (fixes #3656) #3702

Closed
wants to merge 104 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
104 commits
Select commit Hold shift + click to select a range
83de3dd
Inline function
fmaste Aug 11, 2016
26f1416
Extract function
fmaste Aug 11, 2016
6e2d216
extract function
fmaste Aug 11, 2016
2831895
Indent
fmaste Aug 11, 2016
2e543d3
Not used
fmaste Aug 11, 2016
6e13c2c
Not used
fmaste Aug 11, 2016
94fc48c
Comment
fmaste Aug 11, 2016
7f708aa
Extract function
fmaste Aug 11, 2016
7d4a2d4
Rename function
fmaste Aug 11, 2016
46a80b3
Rename variable
fmaste Aug 11, 2016
27a2c27
Rename variable
fmaste Aug 11, 2016
74bca62
Rename variable
fmaste Aug 11, 2016
d2c89ed
Reorder arguments
fmaste Aug 11, 2016
2434443
Comment
fmaste Aug 11, 2016
93b2ed9
Move code
fmaste Aug 11, 2016
55b0c0a
Clean/indent code
fmaste Aug 11, 2016
65651e3
Inline, inline
fmaste Aug 12, 2016
8c8e645
Inline
fmaste Aug 12, 2016
9f9840a
Rename function
fmaste Aug 12, 2016
68e6af3
Rename function
fmaste Aug 12, 2016
d30c098
Inline code
fmaste Aug 12, 2016
bdacfeb
Move code
fmaste Aug 12, 2016
cf0d3fe
Comment
fmaste Aug 12, 2016
42b82a9
Add one more indirection
fmaste Aug 12, 2016
b3ebada
Add one more indirection
fmaste Aug 12, 2016
5854642
Inline code
fmaste Aug 12, 2016
9607910
Use value directly
fmaste Aug 12, 2016
587ffd9
Move out code
fmaste Aug 12, 2016
e691c81
Clean
fmaste Aug 12, 2016
3649010
Comment doc
fmaste Aug 12, 2016
ad1e679
Rename function
fmaste Aug 12, 2016
93c23d7
Move code
fmaste Aug 12, 2016
294a616
Extract function
fmaste Aug 12, 2016
24ce841
Extract function
fmaste Aug 12, 2016
c8747f1
Rename variable
fmaste Aug 12, 2016
2310c02
Indent
fmaste Aug 12, 2016
c9076e6
Identical functions
fmaste Aug 12, 2016
795102b
Not used
fmaste Aug 12, 2016
1de657f
Rename function
fmaste Aug 12, 2016
311f426
Extract code
fmaste Aug 12, 2016
36ca5c5
Move out code
fmaste Aug 12, 2016
43aa9b0
Commend docs
fmaste Aug 12, 2016
f5f86c5
Comment docs
fmaste Aug 12, 2016
8ebe890
Rename variable
fmaste Aug 12, 2016
0106dd6
Rename variable
fmaste Aug 12, 2016
b015e65
Rename variable
fmaste Aug 12, 2016
3ebc180
Rename variable
fmaste Aug 12, 2016
153aa2f
Rename variable
fmaste Aug 12, 2016
a7197ee
Rename variable
fmaste Aug 12, 2016
97aab44
Rename variable
fmaste Aug 12, 2016
baebd95
Indent
fmaste Aug 12, 2016
9cb0124
Rename variable
fmaste Aug 12, 2016
524f9c1
Rename variable
fmaste Aug 12, 2016
0e11f49
Comment
fmaste Aug 12, 2016
60bed62
Rename variable
fmaste Aug 12, 2016
418b464
Extract code
fmaste Aug 12, 2016
83f351c
Extract code
fmaste Aug 12, 2016
3924216
Extract code
fmaste Aug 12, 2016
4c99d0d
Extract code
fmaste Aug 12, 2016
1536052
Extract code
fmaste Aug 12, 2016
c6f2f0c
Simplify
fmaste Aug 12, 2016
0d1347a
Indent
fmaste Aug 12, 2016
05cd0a9
Move code
fmaste Aug 13, 2016
802fa24
Move code
fmaste Aug 13, 2016
af3e285
Rename function
fmaste Aug 13, 2016
0d5df1c
Indent
fmaste Aug 13, 2016
3489839
Docs
fmaste Aug 13, 2016
783cdf3
Indent
fmaste Aug 13, 2016
f7e924a
Rename function
fmaste Aug 13, 2016
aba5b6b
Filter modules directly from the list
fmaste Aug 13, 2016
8fa0448
Comment
fmaste Aug 13, 2016
b9675b2
Inline
fmaste Aug 13, 2016
9fcdafa
Not needed
fmaste Aug 13, 2016
b44d59d
Rename variable
fmaste Aug 13, 2016
dfb3919
preprocessComponent only needs the pkg for its ID
fmaste Aug 13, 2016
1e32ae9
Inline
fmaste Aug 13, 2016
b91020e
Indent
fmaste Aug 13, 2016
533faec
Rename variable
fmaste Aug 13, 2016
9db9a66
Don't edit PackageDescription
fmaste Aug 13, 2016
7ba0fd8
Not used
fmaste Aug 13, 2016
76b68bc
Extract common function
fmaste Aug 17, 2016
f09ede4
Clean
fmaste Aug 17, 2016
190b427
TODO
fmaste Aug 17, 2016
47b4a54
Move UI code to exported function
fmaste Aug 17, 2016
8ad6b65
Move UI code to exported function
fmaste Aug 17, 2016
5eefe36
Docs
fmaste Aug 17, 2016
1b6114f
All the checks together
fmaste Aug 17, 2016
db22629
Comment
fmaste Aug 17, 2016
c931ae1
Comment error
fmaste Aug 25, 2016
634789c
Unify comments
fmaste Aug 25, 2016
3bcecd6
Add autogen fields for c-sources and js-sources
fmaste Aug 25, 2016
c4d1e50
Extract
fmaste Aug 25, 2016
e535cdb
Extract
fmaste Aug 25, 2016
446ef89
Inline
fmaste Aug 25, 2016
9ed9020
Get cSources and jsSources from filtered bi
fmaste Aug 25, 2016
789a600
Filter cSources and jsSources
fmaste Aug 25, 2016
57bbfb1
Indent
fmaste Aug 25, 2016
9b641a1
Clean
fmaste Aug 25, 2016
0332e98
Typo
fmaste Aug 25, 2016
3e23f1e
Tests
fmaste Aug 31, 2016
a4cec48
Merge branch 'master' into issues-3656
fmaste Aug 31, 2016
07b9041
Forgot
fmaste Sep 1, 2016
df45203
Merge branch 'master' into issues-3656
fmaste Sep 12, 2016
e09257a
Error
fmaste Sep 12, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Cabal/Cabal.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,21 @@ extra-source-files:
tests/PackageTests/AutogenModules/SrcDist/MyLibrary.hs
tests/PackageTests/AutogenModules/SrcDist/MyTestModule.hs
tests/PackageTests/AutogenModules/SrcDist/my.cabal
tests/PackageTests/AutogenSources/Package/Dummy.hs
tests/PackageTests/AutogenSources/Package/MyBenchModule.hs
tests/PackageTests/AutogenSources/Package/MyExeModule.hs
tests/PackageTests/AutogenSources/Package/MyLibModule.hs
tests/PackageTests/AutogenSources/Package/MyLibrary.hs
tests/PackageTests/AutogenSources/Package/MyTestModule.hs
tests/PackageTests/AutogenSources/Package/my.cabal
tests/PackageTests/AutogenSources/SrcDist/Dummy.hs
tests/PackageTests/AutogenSources/SrcDist/MyBenchModule.hs
tests/PackageTests/AutogenSources/SrcDist/MyExeModule.hs
tests/PackageTests/AutogenSources/SrcDist/MyLibModule.hs
tests/PackageTests/AutogenSources/SrcDist/MyLibrary.hs
tests/PackageTests/AutogenSources/SrcDist/MyTestModule.hs
tests/PackageTests/AutogenSources/SrcDist/my.cabal
tests/PackageTests/AutogenSources/SrcDist/my_sources.c
tests/PackageTests/BenchmarkExeV10/Foo.hs
tests/PackageTests/BenchmarkExeV10/benchmarks/bench-Foo.hs
tests/PackageTests/BenchmarkExeV10/my.cabal
Expand Down
70 changes: 68 additions & 2 deletions Cabal/Distribution/PackageDescription/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,18 @@ checkLibrary pkg lib =
"An 'autogen-module' is neither on 'exposed-modules' or "
++ "'other-modules'."

-- check that all autogen-c-sources appear on c-sources
, check
(not $ and $ map (flip elem
(cSources $ libBuildInfo lib)) (autogenCSources $ libBuildInfo lib)) $
PackageBuildImpossible $ "An 'autogen-c-sources' is not on 'c-sources'"
Copy link
Contributor

@ezyang ezyang Sep 12, 2016

Choose a reason for hiding this comment

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

This error would be better if (1) it explained WHY you had to duplicate them (it's for backwards compatibility with old Cabal) and (2) said that it was sufficient to just copy paste. (You should probably make a helper function for this message, since you're going to use it a lot...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. The real reason why we repeat filenames is not for backward compatibility is to be consistent with how autogen-modules work, it would be awkward to explain that on an error message.

  2. Do you mean a helper function inside this module that does not $ and $ map (flip elem e) or something like libModules and libModulesAutogen on Distribution.Types.Library ?


-- check that all autogen-js-sources appear on js-sources
, check
(not $ and $ map (flip elem
(jsSources $ libBuildInfo lib)) (autogenJsSources $ libBuildInfo lib)) $
PackageBuildImpossible $ "An 'autogen-js-sources' is not on 'js-sources'"

]

where
Expand Down Expand Up @@ -298,6 +310,22 @@ checkExecutable pkg exe =
"On executable '" ++ exeName exe ++ "' an 'autogen-module' is not "
++ "on 'other-modules'"

-- check that all autogen-c-sources appear on c-sources
, check
(not $ and $ map (flip elem
(cSources $ buildInfo exe)) (autogenCSources $ buildInfo exe)) $
PackageBuildImpossible $
"On executable '" ++ exeName exe ++ "' an 'autogen-c-sources' is not "
++ "on 'c-sources'"

-- check that all autogen-js-sources appear on js-sources
, check
(not $ and $ map (flip elem
(jsSources $ buildInfo exe)) (autogenJsSources $ buildInfo exe)) $
PackageBuildImpossible $
"On executable '" ++ exeName exe ++ "' an 'autogen-js-sources' is not "
++ "on 'js-sources'"

]
where
moduleDuplicates = dups (exeModules exe)
Expand Down Expand Up @@ -343,8 +371,27 @@ checkTestSuite pkg test =
(testModulesAutogen test)
) $
PackageBuildImpossible $
"On test suite '" ++ testName test ++ "' an 'autogen-module' is not "
++ "on 'other-modules'"
"On test suite '" ++ testName test ++ "' an 'autogen-module' is "
++ "not on 'other-modules'"

-- check that all autogen-c-sources appear on c-sources
, check
(not $ and $ map (flip elem
(cSources $ testBuildInfo test))
(autogenCSources $ testBuildInfo test)) $
PackageBuildImpossible $
"On test suite '" ++ testName test ++ "' an 'autogen-c-sources' is "
++ "not on 'c-sources'"

-- check that all autogen-js-sources appear on js-sources
, check
(not $ and $ map (flip elem
(jsSources $ testBuildInfo test))
(autogenJsSources $ testBuildInfo test)) $
PackageBuildImpossible $
"On test suite '" ++ testName test ++ "' an 'autogen-js-sources' is "
++ "not on 'js-sources'"

]
where
moduleDuplicates = dups $ testModules test
Expand Down Expand Up @@ -394,6 +441,25 @@ checkBenchmark _pkg bm =
PackageBuildImpossible $
"On benchmark '" ++ benchmarkName bm ++ "' an 'autogen-module' is "
++ "not on 'other-modules'"

-- check that all autogen-c-sources appear on c-sources
, check
(not $ and $ map (flip elem
(cSources $ benchmarkBuildInfo bm))
(autogenCSources $ benchmarkBuildInfo bm)) $
PackageBuildImpossible $
"On benchmark '" ++ benchmarkName bm ++ "' an 'autogen-c-sources' "
++ "is not on 'c-sources'"

-- check that all autogen-js-sources appear on js-sources
, check
(not $ and $ map (flip elem
(jsSources $ benchmarkBuildInfo bm))
(autogenJsSources $ benchmarkBuildInfo bm)) $
PackageBuildImpossible $
"On benchmark '" ++ benchmarkName bm ++ "' an 'autogen-js-sources' "
++ "is not on 'js-sources'"

]
where
moduleDuplicates = dups $ benchmarkModules bm
Expand Down
6 changes: 6 additions & 0 deletions Cabal/Distribution/PackageDescription/Parse.hs
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,12 @@ binfoFieldDescrs =
, listFieldWithSep vcat "js-sources"
showFilePath parseFilePathQ
jsSources (\paths binfo -> binfo{jsSources=paths})
, listFieldWithSep vcat "autogen-c-sources"
showFilePath parseFilePathQ
autogenCSources (\paths binfo -> binfo{autogenCSources=paths})
, listFieldWithSep vcat "autogen-js-sources"
showFilePath parseFilePathQ
autogenJsSources (\paths binfo -> binfo{autogenJsSources=paths})
, simpleField "default-language"
(maybe mempty disp) (option Nothing (fmap Just parseLanguageQ))
defaultLanguage (\lang binfo -> binfo{defaultLanguage=lang})
Expand Down
Loading