Add get-bounds subcommand

[pbrisbin]

I found this useful so that I could (in bulk) address package versions that were released without bounds (due to a bug in stack upload --pvp-bounds) by getting the bound used in previous versions (via this new command) before adding and publishing (via existing commands).

Example,

% hackage-cli get-bounds *.cabal | head
freckle-app.cabal: Blammo >=0
freckle-app.cabal: Glob >=0
freckle-app.cabal: MonadRandom >=0
freckle-app.cabal: aeson >=0
freckle-app.cabal: aws-xray-client-persistent >=0
freckle-app.cabal: aws-xray-client-wai >=0
freckle-app.cabal: base >=0
freckle-app.cabal: bcp47 >=0
freckle-app.cabal: bugsnag >=0
freckle-app.cabal: bytestring >=0

[andreasabel]

Thanks for the contribution!

So when I do cabal run all -- get-bounds hackage-cli.cabal it prints nothing. Am I doing something wrong or have a misunderstood the description of the command?

get-bounds               Print bounds from the library section of a package.

[pbrisbin]

Hmm, I'm not familiar with cabal run all. In my testing, I did stack build --copy-bins, then ran it as shown in the PR description, which you can see worked:

% hackage-cli get-bounds *.cabal | head
freckle-app.cabal: Blammo >=0
freckle-app.cabal: Glob >=0
freckle-app.cabal: MonadRandom >=0
freckle-app.cabal: aeson >=0
freckle-app.cabal: aws-xray-client-persistent >=0
freckle-app.cabal: aws-xray-client-wai >=0
freckle-app.cabal: base >=0
freckle-app.cabal: bcp47 >=0
freckle-app.cabal: bugsnag >=0
freckle-app.cabal: bytestring >=0

I'm also not familiar with much of the tool's internals, or why hackage-cli.cabal may not work even though freckle-app.cabal did. I mostly just cribbed the get-bounds functionality from half of add-bounds without really understanding.

Does cabal run all -- add-bound something something hackage-cli.cabal work?

[andreasabel]

E.g. cabal run all -- add-bound aeson '>=2.0' hackage-cli.cabal just reports an error:

Cannot find library section in hackage-cli.cabal

Likely it does not work with named (internal) libraries.

If get-bounds can also not handle this, it should print an error.

[pbrisbin]

Sure, will do. add-bounds must generate that error at a higher-level from the extractRange(s) function I cribbed. I will find it.

[pbrisbin]

Done!

[andreasabel]

I added a commit that removes some code duplication you introduced in the previous commits. It wasn't hard to do the necessary factoring for this. Maybe next time you can try harder not to do cut-and-paste. Teacher speaking here.

I am not sure yet about the code in getBounds that produces the error by trying to find the library section in the same way that addBound does.
Is there a reason why get-bounds should not be able to handle any .cabal file? I suspect the extractRanges could be written in a way that is correct for the GenericPackageDescription.

[pbrisbin]

It wasn't hard to do the necessary factoring for this. Maybe next time you can try harder not to do cut-and-paste. Teacher speaking here.

I appreciate the bluntness here, so I feel like I can return in kind: I didn't do those refactors because the code base doesn't appear well-gardened. I see many old patterns (return, forM), it's a single 1,000-line Main.hs with a (seemingly) vendored dependency, there are numerous HLint issues, and the lack of an auto-formatter configuration meant I'd have to be careful not to disrupt local style throughout. It just wasn't appealing, for me, to go further than a working feature in the most minimal diff possible.

If you're open to a contributor making bigger changes like that (in terms of diff-size, not functionality), I would be willing to address lints and establish an auto-formatter, though it would of course come with my own opinions.

I actually found it impressive that the code was so amenable to copy/paste the way it was. It was extremely easy to follow the compiler from piece to piece, and I kind of marveled at the result. It almost seemed intentionally organized to invite that. I was very happy to add the functionality I needed in the moment, and thought the patch was clean enough to offer up, but I'm not attached to it getting merged. Totally up to you if you'd like to take it as is, take it and address those refactorings (which should be easier for you, familiar with the project's idioms), or simply don't take it.

I am not sure yet about the code in getBounds that produces the error by trying to find the library section in the same way that addBound does.
Is there a reason why get-bounds hackage-cli should not be able to handle any .cabal file? I suspect the extractRanges could be written in a way that is correct for the GenericPackageDescription.

(edited, because this issue is broader than get-bounds, as you've shown at least add-bounds also suffers.)

I agree. This is clearly a kludge due to the fact the tool (overall) doesn't seem to support internal libraries. If it did, then get-bound producing no output when there is no library would be the simple and, IMO, correct thing to do. But it can only operate that way when it's not doing that behavior incorrectly on a whole class of valid cabal files.