Fix ls_modules.py for overlong ids.

[robinp]

For some reason, long ids are broken to the next line, so the current
line parsing logic fails.

For a few minutes, I was detoured to think this is #1067, but then noticed this is an actual python script error.

TESTED=manually

[aherrmann]

Thank for fixing this!

Could you also add a regression test?

[robinp]

As I see this piece is not tested yet, and I wouldn't want to spend time adding scaffolding. If you added the mundane pieces, I would love to.

[aherrmann]

I don't think this requires a lot of scaffolding. Under what use-case did you encounter this issue? I.e. what package had a sufficiently long id to trigger the line wrapping?
A simple regression test could be to add an example under tests/ that defines a target with a similarly lengthy package id and a dependency on it to trigger the issue.

[robinp]

I see - it was data-default-instances-containers, which got pulled in somehow for GHC 8.8.3. I'm not sure what generates that global package database - I have both a ghcWithPackages with quite some packages, and haskell_toolchain_library entries generated for them to access.

How would I bring in a haskell_toolchain_library with a long enough name?

[aherrmann]

Oh, sorry I misread the change and didn't realize this was about the global package db. Indeed, the global package db dump is generated in the toolchain rule using ghc-pkg dump --global and the way that data-default-instances-containers ends up in there is through ghcWithPackages.

In that case, one way to test this would be a script that calls ls_modules directly on an example db-dump that exhibits the line wrap and compares the output against an expected output.

This leads me to a follow-up question. Couldn't the name: field be wrapped as well? I recall this issue has been discussed in the past, see #1169. As mentioned in that thread there is already an implementation that handles wrapping more generally in pkgdb_to_bzl.py. pkgdb_to_bzl.py is called in a repository rule context, so sharing the code in a module is a bit tricky to setup. But, using the same code in both with comments pointing to the other would already be an improvement and sounds more robust than ad-hoc handling of the id: field only.

[infinisil]

Just encountered this today. And indeed, I believe all package conf files (and probably the database subsequently) have this line wrapping behavior. In nixpkgs I fixed this for the Haskell builder using an AWK expression that turns it into a more easily processable form, see NixOS/nixpkgs#78738

[aherrmann]

The issue came up again in #1453 so better to merge this to at least have the issue fix. We can do the factoring out and testing in a follow-up.

Closes #1453

[aherrmann]

@Mergifyio rebase

[mergify]

Command rebase: failure

Pull request can't be updated with latest base branch changes
Mergify needs the permission to update the base branch of the pull request.
tweag needs to authorize modification on its base branch.

[aherrmann]

@robinp Looks like there is a permissions issue. Neither mergify nor I can push the rebase of this branch. Could you please rebase on master and push?

[aherrmann]

I took a stab at implementing #1396 (comment) in #1457. The module import in pkgdb_to_bzl.py in repository rule context turned out to work easier than I feared.

So, we can use #1457 instead of this PR.