commercialhaskell / stack

The Haskell Tool Stack
http://haskellstack.org
BSD 3-Clause "New" or "Revised" License
3.95k stars 842 forks source link

Build flag/CPP seems wrong when coming from resolver #6596

Closed pbrisbin closed 1 month ago

pbrisbin commented 1 month ago

Hi there- I'm struggling with a very weird behavior in one of our projects around build flags and CPP, and I really would just like a second set of eyes because I'm going a bit crazy.

We have a package, aws-sns-verify for verifying SNS messages. Part of that is validating a domain name. We use CPP to switch which domain we validate against (AWS vs localhost):

validRegPattern :: String
validRegPattern =
#ifdef DEVELOPMENT
  devRegPattern
#else
  prodRegPattern
#endif

devRegPattern :: String
devRegPattern = "^localhost$"

prodRegPattern :: String
prodRegPattern = "^sns\\.[a-zA-Z0-9\\-]{3,}\\.amazonaws\\.com(\\.cn)?$"

This is enabled via a build flag:

flag development
    description: Configure of testing
    default:     False
    manual:      True

library
    -- ...

    if flag(development)
        cpp-options: -DDEVELOPMENT

I'm finding that if my project uses lts-22.23, which contains aws-sns-verify-0.0.0.3, I end up getting a package that appears to be compiled as if the development flag were True. But if I stack unpack aws-sns-verify and include the local sources as an extra-dep, it works as intended.

I'm a bit baffled at how this can be, so I've put together a reproduction that attempts to be as isolated as possible by building in a docker context instead of my local system (which could've perhaps had some pre-compiled thing stuck with the flag on)

In order to reproduce you'll need to create the following files:

An example.hs that shows how the thing's been built:

module Main where

import Amazon.SNS.Verify.ValidURI

main :: IO ()
main = do
  putStrLn validRegPattern

  if validRegPattern == "^localhost$"
    then error "Unexpected, as if --flag:development"
    else pure ()

A package.yaml for it:

name: cpp-repro
version: 0.0.0.0

executables:
  cpp-repro:
    source-dirs: .
    main: example.hs
    dependencies:
      - base
      - aws-sns-verify

A stack.yaml that brings in the dependency normally:

# https://www.stackage.org/lts-22.23/package/aws-sns-verify-0.0.0.3
resolver: lts-22.23

Then run stack unpack aws-sns-verify and make a stack-works.yaml that uses those instead:

resolver: lts-22.23
extra-deps:
  - ./aws-sns-verify-0.0.0.3

Then a Dockerfile that can build things either way:

FROM fpco/stack-build:lts-22.23
RUN mkdir -p /app
WORKDIR /app
COPY . /app
ARG STACK_YAML=stack.yaml
ENV STACK_YAML=$STACK_YAML
RUN stack build --fast

And, finally, a docker-compose.yml for building and running those images:

services:
  broken:
    build: .
    command: stack exec cpp-repro

  works:
    build:
      context: .
      args:
        - STACK_YAML=stack-works.yaml
    command: stack exec cpp-repro

Then you can run the broken build (Hackage dep):

% docker-compose run broken
[+] Building 19.0s (10/10) FINISHED                                                                  docker:default
 => [broken internal] load build definition from Dockerfile                                                    0.0s
 => => transferring dockerfile: 190B                                                                           0.0s
 => [broken internal] load metadata for docker.io/fpco/stack-build:lts-22.23                                   0.1s
 => [broken internal] load .dockerignore                                                                       0.0s
 => => transferring context: 126B                                                                              0.0s
 => [broken 1/5] FROM docker.io/fpco/stack-build:lts-22.23@sha256:09dcc6cf3739dbb5f73bbb84dc15ee815f463409653  0.0s
 => [broken internal] load build context                                                                       0.0s
 => => transferring context: 7.33kB                                                                            0.0s
 => CACHED [broken 2/5] RUN mkdir -p /app                                                                      0.0s
 => CACHED [broken 3/5] WORKDIR /app                                                                           0.0s
 => CACHED [broken 4/5] COPY . /app                                                                            0.0s
 => CACHED [broken 5/5] RUN stack build --fast                                                                 0.0s
 => [broken] exporting to image                                                                               18.7s
 => => exporting layers                                                                                       18.6s
 => => writing image sha256:243b80945d177ebf081d0c239438d987e55c0a2de1902b737f1ff37086f24205                   0.0s
 => => naming to docker.io/library/cpp-repro-broken                                                            0.0s
Getting the project-level configuration file from the STACK_YAML environment variable.
^localhost$
cpp-repro: Unexpected, as if --flag:development
CallStack (from HasCallStack):

Or the one that works (unpacked sources):

% docker-compose run works
[+] Building 0.4s (10/10) FINISHED                                                                   docker:default
 => [works internal] load build definition from Dockerfile                                                     0.0s
 => => transferring dockerfile: 190B                                                                           0.0s
 => [works internal] load metadata for docker.io/fpco/stack-build:lts-22.23                                    0.2s
 => [works internal] load .dockerignore                                                                        0.0s
 => => transferring context: 126B                                                                              0.0s
 => [works 1/5] FROM docker.io/fpco/stack-build:lts-22.23@sha256:09dcc6cf3739dbb5f73bbb84dc15ee815f463409653c  0.0s
 => [works internal] load build context                                                                        0.0s
 => => transferring context: 7.33kB                                                                            0.0s
 => CACHED [works 2/5] RUN mkdir -p /app                                                                       0.0s
 => CACHED [works 3/5] WORKDIR /app                                                                            0.0s
 => CACHED [works 4/5] COPY . /app                                                                             0.0s
 => CACHED [works 5/5] RUN stack build --fast                                                                  0.0s
 => [works] exporting to image                                                                                 0.1s
 => => exporting layers                                                                                        0.0s
 => => writing image sha256:3f739e196ee14979df7d729a0068620fb14d011d0c20187619070dbff9239ad0                   0.0s
 => => naming to docker.io/library/cpp-repro-works                                                             0.0s
Getting the project-level configuration file from the STACK_YAML environment variable.
^sns\.[a-zA-Z0-9\-]{3,}\.amazonaws\.com(\.cn)?$
mpilgrem commented 1 month ago

LTS 22.23 sets the development flag to be true. See lines 8 and 9 of: https://github.com/commercialhaskell/stackage-snapshots/blob/master/lts/22/23.yaml. Background to that choice appears to be at:

pbrisbin commented 1 month ago

Whoa! So it seems that if a package needs flags set to pass its tests, those flags then implicitly become set for every user that installs the package? That seems Very Bad, don't you agree?

pbrisbin commented 1 month ago

I mean, there's the innocuous build-tests or build-examples kind of flags that projects use to make components non-buildable for normal installs, that they might need built in testing only. Users of those packages are just having time stolen. But I'm now imagining some SSL library that uses a skip-verification flag for its test suite, which seems totally reasonable in isolation, but now suddenly everyone installing it from Stackage is not verifying SSL... :scream:

Now that I think about it, our library is exactly that. If we deployed this this way, someone could submit a webhook payload with a self-signed cert on a localhost domain (like we use in tests) and we'd accept it incorrectly. We don't do any processing worth exploiting, thankfully, but this is a real vector.

mpilgrem commented 1 month ago

@pbrisbin, the questions you raise are better asked at the Stackage project's GitHub repository: https://github.com/commercialhaskell/stackage/issues. The Stack project makes use of the curated package sets but does not influence their content.

pbrisbin commented 1 month ago

Oh of course, sorry.