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

Wrong cache invalidation (using PVP versions) with cabal install vs cabal build #7811

Closed
jneira opened this issue Nov 11, 2021 · 8 comments
Closed

Comments

@jneira
Copy link
Member

jneira commented Nov 11, 2021

The reason is, because cabal install is semi-broken when you install from git, since it caches based on PVP versions and will not always pick up changes from dependencies via source-repository-package. That's why we switched to cabal build.

and

I think the problem also exists for anything listed in packages, which is the case for e.g. ghcide.

We should investigate the bug and try to reproduce it consistently to avoid to have to use cabal build + cabal list-bin, workaround that does not play well with dynamic linking

More context:

//cc @hasufell

@phadej
Copy link
Collaborator

phadej commented Nov 11, 2021

Duplicate of #6211 and #5782 which are marked fixed. Without a reproducer, I'm simply going to close this.

@phadej phadej closed this as completed Nov 11, 2021
@jneira
Copy link
Member Author

jneira commented Nov 11, 2021

Hmm hls nor its companions (ghcide, hls-plugin-api etc) dont use internal libraries 🤔

@phadej
Copy link
Collaborator

phadej commented Nov 11, 2021

Then I don't think there's a bug.

The reason is, because cabal install is semi-broken when you install from git, since it caches based on PVP versions

Is definitely not true. Contents of the sdisted tarball are part of unit-id hash.

This issue needs a reproducer, no point to keep the issue open indefinitely until.

@jneira
Copy link
Member Author

jneira commented Nov 11, 2021

This issue needs a reproducer, no point to keep the issue open indefinitely until.

hmm but does a bug report needs a reproducer to keep it opened? that is the rationale behind the blocked: needs repro label.
should we close all issues labeled with it?

my intent was just work in getting such reproducer and close it if it is not possible...

Is definitely not true. Contents of the sdisted tarball are part of unit-id hash.

that is my understanding but i wanted to confirm the bad behaviour detected in ghcup-hs

@phadej
Copy link
Collaborator

phadej commented Nov 11, 2021

My opinon on blocked: needs repro that if they have been open for say few months, they can be closed. (EDIT: and it's not the cabal maintaienrs job to try to figure out the reproducer after initial triage). This is slightly different, as this looks like a duplicate.

@jneira
Copy link
Member Author

jneira commented Nov 11, 2021

My opinon on blocked: needs repro that if they have been open for say few months, they can be closed

agree

This is slightly different, as this looks like a duplicate.

But, a duplicate of? I've spent some time searching the issue tracker to find it (but maybe i failed of course).
Imo all reports have the right of be presumed innocent until proven guilty 🙂

But whatever, will reopen if i find the reproducer

@hasufell
Copy link
Member

I tried to reproduce it and I can't.

  • -- Hash tarball files for packages where we have to do that. This includes
    -- tarballs that were local in the first place, plus tarballs from repos,
    -- either previously cached or freshly downloaded.
    --
    let allTarballFilePkgs :: [(PackageId, FilePath)]
    allTarballFilePkgs = localTarballPkgs
    ++ remoteTarballPkgs
    ++ sourceRepoTarballPkgs
    ++ repoTarballPkgsDownloaded
    ++ repoTarballPkgsNewlyDownloaded
    hashesFromTarballFiles <- liftIO $
    fmap Map.fromList $
    sequence
    [ do srchash <- readFileHashValue tarball
    return (pkgid, srchash)
    | (pkgid, tarball) <- allTarballFilePkgs
    ]
    monitorFiles [ monitorFile tarball
    | (_pkgid, tarball) <- allTarballFilePkgs ]
    -- Return the combination
    return $! hashesFromRepoMetadata
    <> hashesFromTarballFiles
  • , entry "src" showHashValue pkgHashSourceHash

As you can see here... cabal hashes the entire contents of the source tarball. If you have the same PVP version, but different contents, it will rebuild the dependency. So the effect we observed was either due to something else or our assumptions were wrong.

@jneira
Copy link
Member Author

jneira commented Nov 11, 2021

@hasufell thanks for check it, maybe returning to use cabal install in ghcup it will eventually trigger it again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants