search

awakesecurity / proto3-suite

Haskell Protobuf Implementation
https://hackage.haskell.org/package/proto3-suite
Other
81 stars 56 forks source link

Nix: if enableLargeRecords is false use -f-large-records #244

Closed j6carey closed 1 year ago

j6carey commented 1 year ago

The absence of "-flarge-records" is not enough because the "large-records" cabal flag is enabled by default.

This fix should improve the coverage of continuous integration tests for the proto3-suite repository.

j6carey commented 1 year ago

This is just a problem with how the flag is defined though, no? Currently theres no way to disable large records, the default value for the flag needs to be changed from True to False.

I'm able to disable the flag with -f-large-records; that's what this PR does.

riz0id commented 1 year ago

This is just a problem with how the flag is defined though, no? Currently theres no way to disable large records, the default value for the flag needs to be changed from True to False.

I'm able to disable the flag with -f-large-records; that's what this PR does.

I don't understand why passing -f-large-records should disable large records, seems like -f-large-records should enable large records. In any case, I don't understand how passing the -f-large-records flag is disabling the large-records flag in the cabal if both the default and manual values for large-records is True.

riz0id commented 1 year ago

If it does work then I'll approve it. It seems to me like value for Manual should be changed from False to True, the nix change you made here should be reverted. Or, the flag should be renamed to something like disable-large-records to clearly indicate that passing it to cabal disable compilation with large records.

j6carey commented 1 year ago

I don't understand why passing -f-large-records should disable large records, seems like -f-large-records should enable large records. In any case, I don't understand how passing the -f-large-records flag is disabling the large-records flag in the cabal if both the default and manual values for large-records is True.

What I am seeing is that when you pass -fx on the cabal command line, cabal tries to enable x; whereas if you pass -f-x then it tries to disable x; otherwise it takes the default value from the .cabal file.

j6carey commented 1 year ago

If it does work then I'll approve it. It seems to me like value for Manual should be changed from False to True, the nix change you made here should be reverted. Or, the flag should be renamed to something like disable-large-records to clearly indicate that passing it to cabal disable compilation with large records.

The "manual" flag actually means something other than the meaning of the command-line argument for the flag when present: https://cabal.readthedocs.io/en/stable/cabal-package.html#configuration-flags

By default, Cabal will first try to satisfy dependencies with the default flag value and then, if that is not possible, with the negated value. However, if the flag is manual, then the default value (which can be overridden by commandline flags) will be used.