Aeson should not be reexported

[ysangkok]

Using the following cabal.project in amazonka/lib/amazonka:

packages: .

source-repository-package
  type: git
  tag: 97f71a2
  location: [email protected]:brendanhay/amazonka.git
  subdir: lib/services/amazonka-sts
  subdir: lib/amazonka-core

Running cabal repl:

[1 of 9] Compiling Amazonka.Auth[boot] ( src/Amazonka/Auth.hs-boot, interpreted )
[2 of 9] Compiling Amazonka.EC2.Metadata ( src/Amazonka/EC2/Metadata.hs, interpreted )
[3 of 9] Compiling Amazonka.HTTP[boot] ( src/Amazonka/HTTP.hs-boot, interpreted )
[4 of 9] Compiling Amazonka.Auth    ( src/Amazonka/Auth.hs, interpreted )
[5 of 9] Compiling Amazonka.Env     ( src/Amazonka/Env.hs, interpreted )
[6 of 9] Compiling Amazonka.Logger  ( src/Amazonka/Logger.hs, interpreted )
[7 of 9] Compiling Amazonka.HTTP    ( src/Amazonka/HTTP.hs, interpreted )
[8 of 9] Compiling Amazonka.Presign ( src/Amazonka/Presign.hs, interpreted )
[9 of 9] Compiling Amazonka         ( src/Amazonka.hs, interpreted )
Ok, 9 modules loaded.
ghci> :t (.=)
(.=)
  :: (aeson-2.0.2.0-ab03924e3ed3e0c23aa6f63c6eb04c997f689f99e57f0368b67fd38c85e24324:Data.Aeson.Types.ToJSON.KeyValue
        kv,
      ToJSON v) =>
     aeson-2.0.2.0-ab03924e3ed3e0c23aa6f63c6eb04c997f689f99e57f0368b67fd38c85e24324:Data.Aeson.Key.Key
     -> v -> kv

I did not expect Amazonka to reexport combinator parts of Aeson. This is triggering redundant import warnings, but we are using Amazonka and Aeson in unrelated parts of our module, and it seems weird to have to split it up because of this reexport. It also seems unnecessary to need to qualify these imports.

This is using cabal-install v3.6.2.0 and GHC v9.2.1 and amazonka from main (commit 97f71a24b35ee8a8542f7e6479860eb9965a109e).

[bsaul]

I think the re-export starts here in amazonka-core's Amazonka.Data.JSON, which is then reexported by Amazonka.Data, which is reexported by Amazonka.Core, which is then reexported by the main Amazonka module in the amazonka package.

How should we handle this?

One idea is to remove the reexport of Amazonka.Data.JSON in Amazonka.Data then explicitly import Amazonka.Data.JSON only when it's needed.

[brendanhay]

The main Amazonka module should just be more selective about what it exports. The amazonka-core package is predominantly used by generated code and the code generator inserts qualified imports - it's a mistake to have it exported verbatim by the user-facing amazonka package.

[endgame]

Glad you've weighed in here @brendanhay - I was going to suggest that Amazonka.Prelude should be re-exporting all the bits needed by generated code, but I was a bit hesitant about making a call about the style of the public API. Another alternative would be to generate aeson imports in generated code.

[bsaul]

FWIW, if I comment out the reexport of Amazonka.Core in the mainAmazonka.hs module, cabal build amazonka runs successfully for me locally.

What should get reexported from core in the user-facing library?

[endgame]

Do service bindings build okay if you do that? Testing that amazonka-ec2 still built probably be a good indicative case. I'd test something big like EC2 and something JSON-using (like amazonka-dynamodb) as well.

I think that before trying to audit everything that's re-exported, I'd try moving the aeson re-exports to Amazonka.Prelude, which is not exported by Amazonka. This should keep things usable by service bindings.

[brendanhay]

None of the generated code should be depending on the amazonka package, so removing unnecessary core re-exports from Amazonka should be fine.

For users of the library, import/export annoyance arises where the generator fails to {un,}wrap types relating to serialisation, such as Amazonka.Data.Time - which would then force you to add a dependency on amazonka-core + import Amazonka.Data{,.Time}.

Prior to the Network.AWS -> Amazonka restructure you'd typically see applications depend on both amazonka and amazonka-core in order to access class, constructors, and lenses specifically relating to serialisation - the reexports from Amazonka were supposed to alleviate that, but are clearly too coarse.

[bsaul]

Do service bindings build okay if you do that? Testing that amazonka-ec2 still built probably be a good indicative case. I'd test something big like EC2 and something JSON-using (like amazonka-dynamodb) as well.

Unfortunately, I'm getting a seemingly unrelated error when I try (with or without removing the reexports from Amazonka.Core):

% cabal build amazonka-ec2 
Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - amazonka-ec2-2.0 (lib) (first run)
Preprocessing library for amazonka-ec2-2.0..
Building library for amazonka-ec2-2.0..
clang: error: no such file or directory: '/Users/bradley.saul/Documents/novisci/software/amazonka/amazonka/dist-newstyle/build/aarch64-osx/ghc-8.10.7/amazonka-ec2-2.0/build/Amazonka/EC2/CopyFpgaImage.dyn_o'
`gcc' failed in phase `Linker'. (Exit code: 1)

[endgame]

I would try removing (part or all of) your dist-newstyle directory and rebuilding, and/or using the nix-shell and bazel (if you can spare the disk space).

[bsaul]

Ran cabal clean and nearly succeeded:

[1229 of 1274] Compiling Amazonka.EC2.CancelConversionTask ( gen/Amazonka/EC2/CancelConversionTask.hs, /Users/bradley.saul/Documents/novisci/software/amazonka/amazonka/dist-newstyle/build/aarch64-osx/ghc-8.10.7/amazonka-ec2-2.0/build/Amazonka/EC2/CancelConversionTask.o, /Users/bradley.saul/Documents/novisci/software/amazonka/amazonka/dist-newstyle/build/aarch64-osx/ghc-8.10.7/amazonka-ec2-2.0/build/Amazonka/EC2/CancelConversionTask.dyn_o )
ghc: could not execute: opt

I'm not familiar enough with bazel to know how to get started there. Trying with nix-shell now, though I'm having to run with broken packages since:

% nix-shell
error: Package ‘compiler-rt-libc-9.0.1’ in /nix/store/ijv6rzijw570vdfp6lgdpdw1l8hny5p8-source/pkgs/development/compilers/llvm/9/compiler-rt/default.nix:95 is marked as broken, refusing to evaluate.

[endgame]

Strange that it only wanted opt after that many packages. What hardware are you running on? It's not an M1 mac by any chance? If so, try:

• (straight cabal) installing LLVM >=9 && <13, or
• (nix/bazel) nix-shell --system x86_64-darwin

There are some instructions for driving bazel in the readme, but you can also try bazel query //... | grep ec2 to find a promising target, and then bazel build //whatever_the_target_is_called.

[bsaul]

What hardware are you running on?

Indeed, it is an M1 mac. I have llvm 12.0.1 installed. Trying nix-shell (there's alot to install/download).

There are some instructions for driving bazel in the readme, but you can also try ...

Thanks. This will be a good intro to bazel for me.

[bsaul]

I wasn't able to get the bazel build running, but in a nix-shell cabal build amazonka-ec2 succeeded.

[bsaul]

but in a nix-shell cabal build amazonka-ec2 succeeded.

ditto for cabal build amazonka-dynamodb

[endgame]

That's a good sign. CI is currently busted but if you make a PR I can do a treewide build on my workstation to be sure. I think Brendan's concerns are valid - removing the reexport of Amazonka.Core might drop things needed by library clients rather than service bindings.

[bsaul]

PR created: #777