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-tool flag to omit tool and FV deps+modules from build #882

Merged
merged 8 commits into from
Jun 23, 2021

Conversation

sirlensalot
Copy link
Contributor

No description provided.

pact.cabal Outdated Show resolved Hide resolved
executables/Bench.hs Outdated Show resolved Hide resolved
executables/GasModel.hs Outdated Show resolved Hide resolved
executables/Repl.hs Outdated Show resolved Hide resolved
pact.cabal Outdated Show resolved Hide resolved
@@ -292,7 +316,7 @@ executable gasmodel
default-language: Haskell2010

test-suite hspec
if !impl(ghcjs)
if !impl(ghcjs) && flag(build-tool)
main-is: hspec.hs
type: exitcode-stdio-1.0
else
Copy link
Contributor

@larskuhtz larskuhtz Jun 23, 2021

Choose a reason for hiding this comment

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

The else branch will be selected for !impl(ghcjs) && !flag(build-tool), which probably is not intended. I think, the following would be correct:

  elif impl(ghcjs) && flag(build-tool)
    main-is:          hspec-ghcjs.hs
    type:             exitcode-stdio-1.0
  else
    buildable: False

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's intended this way to at least run some tests on ghc && !build-tool. What is really needed is a subset of tests for this case but that can wait.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, cool. I though the build-tool would be incompatible with the test suite. Much better this way. We should also reenable tests in the ci build for -build-tool.

pact.cabal Outdated Show resolved Hide resolved
* reorganize lib section in pact.cabal

* build tools only if the library supports it

* set build-tool to manual in order for the default value to become effective

* include -build-tool case into ci matrix

* fix build-tool flag in ci build

* set BUILD_TOOL macro for tool builds

* don't run tests and benchmarks in ci when -fbuild-tool
Copy link
Contributor

@larskuhtz larskuhtz left a comment

Choose a reason for hiding this comment

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

Approved with a few suggestions regarding the CPP macros for generating the errors.

Not sure, if they are needed, because building those modules is already disabled in the cabal file. If we want to keep them as an additional safety check, we may consider using #error instead of Haskells error, because the former would fail already at compilation time.

If we remove them we could simplify the cabal file for the respective components a bit by not setting the BUILD_TOOL macro and removing the flag(build-tool) branch of the if statement that guards the buildable property.

if !flag(build-tool)
  buildable: False

would be sufficient, since buildable: True is the default.

sirlensalot and others added 2 commits June 23, 2021 11:56
* Restore tests and remove CPP from executables

* fix tests

Co-authored-by: Stuart Popejoy <slpopejoy@users.noreply.github.com>
@sirlensalot sirlensalot merged commit e2f3dd1 into master Jun 23, 2021
@sirlensalot sirlensalot deleted the feat/build-tool branch June 23, 2021 17:40
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.

None yet

3 participants