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

Trim transitive tracing dependencies #5289

Closed
wants to merge 6 commits into from

Conversation

mtth
Copy link
Contributor

@mtth mtth commented Apr 4, 2020

Most of the previous entries are not needed (anymore? - I'm not sure why they had been added). ip is the only dependency which isn't already referenced; it is now in @andrewthad's section (author of the library).

I noticed this while investigating a fix for this issue, which requires an update to small-bytearray-builder which I don't own and AFAICT isn't used by tracing. For reference, here is the list of tracing's transitive dependencies:

$ stack ls dependencies --no-include-base --test
HUnit 1.6.0.0
QuickCheck 2.13.1
aeson 1.4.3.0
ansi-terminal 0.9.1
async 2.2.2
attoparsec 0.13.2.2
base-compat 0.10.5
base16-bytestring 0.1.1.6
basement 0.0.10
binary 0.8.6.0
blaze-builder 0.4.1.0
bytestring 0.10.8.2
case-insensitive 1.2.0.11
containers 0.6.0.1
cookie 0.4.4
deepseq 1.4.4.0
directory 1.3.3.0
dlist 0.8.0.6
exceptions 0.10.2
ghc-prim 0.5.3
hashable 1.2.7.0
hspec 2.7.1
hspec-core 2.7.1
hspec-discover 2.7.1
hspec-expectations 0.8.2
http-client 0.6.4
http-types 0.12.3
integer-gmp 1.0.2.0
integer-logarithms 1.0.3
ip 1.5.0
memory 0.14.18
mime-types 0.1.0.9
mtl 2.2.2
network 2.8.0.1
network-uri 2.6.1.0
parsec 3.1.13.0
pretty 1.1.3.6
primitive 0.6.4.0
process 1.6.5.0
quickcheck-io 0.2.0
random 1.1
rts 1.0
scientific 0.3.6.2
setenv 0.1.1.3
splitmix 0.0.2
stm 2.5.0.0
streaming-commons 0.2.1.1
tagged 0.8.6
template-haskell 2.14.0.0
text 1.2.3.1
tf-random 0.5
th-abstraction 0.3.1.0
time 1.8.0.2
time-locale-compat 0.1.1.5
tracing 0.0.4.1
transformers-compat 0.6.5
unix 2.7.2.2
unliftio 0.2.11
unliftio-core 0.1.2.0
unordered-containers 0.2.10.0
uuid-types 1.0.3
vector 0.12.0.3
wide-word 0.1.0.8
zlib 0.6.2

mtth and others added 6 commits April 4, 2020 11:33
Most of the previous entries are not needed anymore. `ip` is the only
one which isn't already transitively depended on (I kept it and moved it
under its owner).

For reference, here is the full list of dependencies

$ stack ls dependencies --no-include-base --test
HUnit 1.6.0.0
QuickCheck 2.13.1
aeson 1.4.3.0
ansi-terminal 0.9.1
async 2.2.2
attoparsec 0.13.2.2
base-compat 0.10.5
base16-bytestring 0.1.1.6
basement 0.0.10
binary 0.8.6.0
blaze-builder 0.4.1.0
bytestring 0.10.8.2
case-insensitive 1.2.0.11
containers 0.6.0.1
cookie 0.4.4
deepseq 1.4.4.0
directory 1.3.3.0
dlist 0.8.0.6
exceptions 0.10.2
ghc-prim 0.5.3
hashable 1.2.7.0
hspec 2.7.1
hspec-core 2.7.1
hspec-discover 2.7.1
hspec-expectations 0.8.2
http-client 0.6.4
http-types 0.12.3
integer-gmp 1.0.2.0
integer-logarithms 1.0.3
ip 1.5.0
memory 0.14.18
mime-types 0.1.0.9
mtl 2.2.2
network 2.8.0.1
network-uri 2.6.1.0
parsec 3.1.13.0
pretty 1.1.3.6
primitive 0.6.4.0
process 1.6.5.0
quickcheck-io 0.2.0
random 1.1
rts 1.0
scientific 0.3.6.2
setenv 0.1.1.3
splitmix 0.0.2
stm 2.5.0.0
streaming-commons 0.2.1.1
tagged 0.8.6
template-haskell 2.14.0.0
text 1.2.3.1
tf-random 0.5
th-abstraction 0.3.1.0
time 1.8.0.2
time-locale-compat 0.1.1.5
tracing 0.0.4.1
transformers-compat 0.6.5
unix 2.7.2.2
unliftio 0.2.11
unliftio-core 0.1.2.0
unordered-containers 0.2.10.0
uuid-types 1.0.3
vector 0.12.0.3
wide-word 0.1.0.8
zlib 0.6.2

Note that it contains a few entries which are transitive dependencies of
packages which are already in the build constraints. For example:

* binary < scientific
* deepseq < bytestring
* integer-gmp < integer-logarithms
* process < unliftio
* stm < streaming-commons
@snoyberg
Copy link
Contributor

snoyberg commented Apr 5, 2020

The build failed, it looks like those transitive dependencies still apply. Also, we don't typically allow people to add packages under the maintenance headlines of someone else.

@mtth
Copy link
Contributor Author

mtth commented Apr 6, 2020

Thanks for taking a look. I think what happened is that ip had different dependencies when I originally added tracing to Stackage. When ip-1.6.0 was released, it introduced several new transitive dependencies which @DanBurton added under my headline to keep things working.

I've removed ip from tracing's dependencies but there might be other packages which depend on it, so we might still need a maintainer for it (and its dependencies). What would you recommend?

@snoyberg
Copy link
Contributor

snoyberg commented Apr 6, 2020

It looks like the only other two maintainers affected by this are @athanclark, since attoparsec-ip and attoparsec-uri both depend on ip, and @nikita-volkov, since stm-hamt depends on primitive-extras. Are the two of you interested in taking over maintainership of the transitive dependencies of the relevant packages? If not, the curator team will have to decide to either:

  • Move them to grandfathered packages
  • Remove the packages depending on them

@nikita-volkov
Copy link
Contributor

I'm not sure what is requested of me here. Neither stm-hamt or primitive-extras depend on the "ip" package.

@athanclark
Copy link
Contributor

Hello, sorry for being late to the conversation. I'm not exactly sure what is being asked, but I originally made attoparsec-ip to be compliant with the ip package; if it's causing issues transitively, I could pretty quickly pop-and-swap an in-package data type, because the parser is independent from the rest of the ip package.

Would that be ideal for my end? Or if a maintainer for ip is being requested, I could lend a hand, but I'm not sure if I understood the issue properly.

@snoyberg
Copy link
Contributor

snoyberg commented Apr 6, 2020

Sorry for the lack of clarity. Let me see if I get this right:

  1. @mtth opened a PR adding a package
  2. That package depended on ip and a number of other transitive dependencies not yet included in Stackage at the time
  3. @DanBurton added the transitive dependencies under @mtth's name
  4. @nikita-volkov and @athanclark added packages which used those transitive dependencies
  5. @mtth dropped his usage of ip, and opened this PR to drop those transitive dependencies from Stackage
  6. That PR is failing because of the packages from @nikita-volkov and @athanclark

So the options here are:

  1. Move those transitive dependencies to grandfathered packages, something we try to avoid doing
  2. Move the transitive dependencies to being managed by @nikita-volkov and @athanclark well relevant
  3. Drop the packages depending on the transitive dependencies

My preference is for (2). Does that clarify?

@nikita-volkov
Copy link
Contributor

I'm up for any option that lets me avoid maintenance of packages which I'm not the author of.

@athanclark
Copy link
Contributor

I decided to remove the packages attoparsec-ip and attoparsec-uri entirely; attoparsec-ip just relies on the parsers provided by ip. Referenced by #5300

snoyberg added a commit that referenced this pull request Apr 15, 2020
@snoyberg
Copy link
Contributor

Please see #5307

@snoyberg snoyberg closed this Apr 15, 2020
@mtth
Copy link
Contributor Author

mtth commented Apr 17, 2020

Fantastic - thanks @snoyberg!

bergmark added a commit that referenced this pull request Apr 17, 2020
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.

5 participants