Fix --offline flag #8676

cbclemmer · 2023-01-14T16:12:15Z

Please include the following checklist in your PR:

Patches conform to the coding conventions.
Any changes that could be relevant to users have been recorded in the changelog.
The documentation has been updated, if necessary.

The --offline flag previously created in #2578 but was only implemented for the install command even thought the flag didn't throw an error whenever the build command was run. This PR adds functionality for the --offline flag with the build command.
Additionally there is a new PackageTest for the flag using the build command.

ulysses4ever · 2023-01-14T18:57:35Z

Presumably, fixes #5346

cbclemmer · 2023-01-14T18:58:23Z

@ulysses4ever Yeah, I was waiting until I got the tests passing before I pinged anyone

ulysses4ever · 2023-01-14T18:59:19Z

No worries! Let's see...

cbclemmer · 2023-01-14T19:06:56Z

@ulysses4ever I got all the test steps you were talking about before working. The only issue is that when it errors it gives the file the error was called from and that could change later on. Not sure what the best fix for this is because I just sent up the error like rebuildTargets is supposed to do by defining an error BuildOutcome object.

Also as a note, because it's not downloading it from a remote repo I have to test the PackageLocation to be an instance of RepoTarballPackage, RemoteTarballPackage, or RemoteSourceRepoPackage instead of testing whether the BuildStatus is BuildStatusDownload like in the asyncDownloadPackages function (The test is of type RepoTarballPackage).

cbclemmer · 2023-01-18T16:54:30Z

@ulysses4ever I can probably create a tag like there is for the ghc version etc. to capture error details and that would fix it. I don't think there is anything like that at the moment.

ulysses4ever · 2023-01-18T19:16:25Z

I'm sorry I've been running a bit behind on cabal things. I'd have to defer this choice to someone more knowledgeable or just trust you.

Mikolaj · 2023-01-18T19:35:50Z

@cbclemmer: I'm thinking, could you have a look at git blame or PR authors of the related code and ping a few previous contributors (unless they are from > 5 years ago, I guess)? If nothing comes out of it, I will try to understand it, advise, merge and we'd depend on feedback from outraged users instead. ;)

cbclemmer · 2023-01-18T19:41:22Z

@ulysses4ever No worries. I just had an idea and wanted to share. I'll look around the output validator code and see what I can find.

cbclemmer · 2023-01-30T16:22:31Z

@ulysses4ever I added the tag and now the tests pass. It looks like it was several years ago since the file that normalizes the output was last edited.

Kleidukos

I guess we can do without the .vscode/tasks.json ;)

ulysses4ever

We are getting there!

Also, what's up with cabal-testsuite/PackageTests/MultipleLibraries/T7270/p/Main?

cabal-install/src/Distribution/Client/ProjectBuilding.hs

cabal-testsuite/src/Test/Cabal/OutputNormalizer.hs

cabal-testsuite/PackageTests/OfflineFlag/offlineFlag.out

cabal-install/src/Distribution/Client/ProjectBuilding.hs

ulysses4ever · 2023-01-31T03:51:18Z

cabal-install/src/Distribution/Client/ProjectBuilding.hs

+          elabPkgSourceId = PackageIdentifier { pkgName, pkgVersion }
+        } = (elabUnitId, Left (BuildFailure {
+          buildFailureLogFile = Nothing,
+          buildFailureReason = DownloadFailed . error $ makeError pkgName pkgVersion


I think it shouldn't involve error: it's not an exceptional situation like a network failure, or something else out of our control, it's we who are refusing to do something because of the flag -- it's no exception! So, I think, it should be DependentFailed rather than DownloadFailed. Perhaps someone has another opinion.

DependentFailed needs another package as a parameter this looks like it's used to say that the package that is failing has a dependency that failed. I need to say that this specific package failed while also not producing an exception. Nothing else in the BuildFailureReason data type looked like it suited the problem so I added a GracefulFailure option to BuildFailureReason and it takes a string that will be printed to the terminal without any extra text or callstack, I think anything more specific would clutter up the data type more than is needed.

Full disclosure: I'm no expert in Cabal error types. But, I thought that DependentFailed would work quite all right for this use case. Imagine you depend on 10 packages and 9 of them are in the cache. Then, you may want to know which one wasn't there. On the other hand, if there're two dependencies that are not in the cache then reporting just one is not so nice, perhaps. Although, I could live with reporting one that happened to be the first to be required during the build…

That said, I don't have a strong preference. I'm just not a big fan of changing data types that sound fundamental. Cabal is a library and this would be a breaking change...

ulysses4ever · 2023-02-26T03:43:56Z

@cbclemmer just a gentle ping. Great work here, just need to fix minor things...

ulysses4ever · 2023-03-01T10:52:20Z

I sense we're getting close. We need reviewers and possibly a backport. I'll add my final review ASAP but we'd need another one.

cbclemmer · 2023-03-01T16:38:33Z

@Kleidukos I didn't see your review before. It's deleted now. I must have randomly pressed F5 at some point...

Kleidukos · 2023-03-01T16:57:24Z

@Kleidukos I didn't see your review before. It's deleted now. I must have randomly pressed F5 at some point...

No problem :)

ulysses4ever · 2023-03-04T15:38:48Z

@Kleidukos you requested changes on this PR (#8676 (review)) and I believe they were applied, so maybe you could either unblock it or even approve it?

Kleidukos

LGTM! :)

ulysses4ever · 2023-03-05T03:26:58Z

cabal-install/src/Distribution/Client/ProjectBuilding.hs

-               } = do
-
+               }
+  | fromFlagOrDefault False (projectConfigOfflineMode config) && not (null packagesToDownload) = return offlineError


I think you could use notNull instead of not and null separately.

ulysses4ever · 2023-03-07T19:59:00Z

@cbclemmer you should feel free to put the squash+merge label now.

Thank you for your contribution, and keep the good stuff coming!!! 🎉

cbclemmer · 2023-03-07T20:24:25Z

@ulysses4ever Thanks, I looked up notNull on hoogle and I thought it was included in the prelude but apparently it wasn't. I've reverted my changes and now it needs to do the build process again. Thanks for reviewing it

ulysses4ever · 2023-03-07T20:32:54Z

@cbclemmer oh my, I'm so sorry! The reason for my mishmash is that I confused it with notElem (which indeed is available from the Prelude).

ulysses4ever · 2023-03-09T23:45:36Z

@mergify backport 3.10

mergify · 2023-03-09T23:45:39Z

backport 3.10

✅ Backports have been created

#8838 Fix --offline flag (backport #8676) has been created for branch 3.10

* WIP * WIP * WIP * WIP * WIP * add offline logic branch * Clean up * Formatting * Optimize * Rename test folder * Add changelog file * Fix whitespace * Add <CABAL_ERROR> normalizer tag * code review changes * Delet vs code file * Fix import * fix wrong output file --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 0ed1218)

* WIP * WIP * WIP * WIP * WIP * add offline logic branch * Clean up * Formatting * Optimize * Rename test folder * Add changelog file * Fix whitespace * Add <CABAL_ERROR> normalizer tag * code review changes * Delet vs code file * Fix import * fix wrong output file --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

cbclemmer added 12 commits January 12, 2023 17:46

WIP

46ff2f7

WIP

60daa3d

WIP

aedb4bc

WIP

a4db7e2

WIP

2bacd71

add offline logic branch

82f3629

Clean up

8617b5f

Formatting

e088365

Optimize

9ac83e9

Rename test folder

c2bbca9

Add changelog file

71c987b

Fix whitespace

96ce1e9

Mikolaj added type: enhancement cabal-install: other labels Jan 14, 2023

ulysses4ever linked an issue Jan 14, 2023 that may be closed by this pull request

--offline flag doesn't seem to affect new-build #5346

Closed

cbclemmer added 2 commits January 24, 2023 14:44

Update from master

79776b5

Add <CABAL_ERROR> normalizer tag

eb6043f

Kleidukos requested changes Jan 30, 2023

View reviewed changes

ulysses4ever reviewed Jan 31, 2023

View reviewed changes

Kleidukos mentioned this pull request Feb 10, 2023

cabal build --offline tries to access the network anyway #8752

Closed

cbclemmer and others added 3 commits February 28, 2023 15:23

Delet vs code file

9f5f902

Fix import

d20174a

fix wrong output file

644044a

ulysses4ever added attention: needs-review attention: needs-backport 3.12 and removed type: enhancement labels Mar 1, 2023

Kleidukos self-requested a review March 4, 2023 17:40

Kleidukos approved these changes Mar 4, 2023

View reviewed changes

ulysses4ever reviewed Mar 5, 2023

View reviewed changes

ulysses4ever approved these changes Mar 5, 2023

View reviewed changes

cbclemmer force-pushed the cc/5346 branch from 2c3627f to 644044a Compare March 7, 2023 19:47

ulysses4ever removed the attention: needs-review label Mar 7, 2023

cbclemmer added the squash+merge me Tell Mergify Bot to squash-merge label Mar 7, 2023

mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Mar 9, 2023

Merge branch 'master' into cc/5346

7ffaf46

mergify bot merged commit 0ed1218 into haskell:master Mar 9, 2023

mergify bot mentioned this pull request Mar 9, 2023

Fix --offline flag (backport #8676) #8838

Closed

Mikolaj removed the attention: needs-backport 3.12 label Jun 17, 2023

ulysses4ever mentioned this pull request Aug 27, 2024

--offline does not stop Cabal from attempting to access the network #10288

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix --offline flag #8676

Fix --offline flag #8676

cbclemmer commented Jan 14, 2023 •

edited

Loading

ulysses4ever commented Jan 14, 2023

cbclemmer commented Jan 14, 2023

ulysses4ever commented Jan 14, 2023

cbclemmer commented Jan 14, 2023

cbclemmer commented Jan 18, 2023

ulysses4ever commented Jan 18, 2023

Mikolaj commented Jan 18, 2023

cbclemmer commented Jan 18, 2023

cbclemmer commented Jan 30, 2023

Kleidukos left a comment

ulysses4ever left a comment

ulysses4ever Jan 31, 2023

cbclemmer Feb 28, 2023

ulysses4ever Mar 5, 2023

ulysses4ever commented Feb 26, 2023

ulysses4ever commented Mar 1, 2023

cbclemmer commented Mar 1, 2023

Kleidukos commented Mar 1, 2023

ulysses4ever commented Mar 4, 2023

Kleidukos left a comment

ulysses4ever Mar 5, 2023

ulysses4ever commented Mar 7, 2023

cbclemmer commented Mar 7, 2023

ulysses4ever commented Mar 7, 2023

ulysses4ever commented Mar 9, 2023

mergify bot commented Mar 9, 2023 •

edited

Loading

Fix --offline flag #8676

Fix --offline flag #8676

Conversation

cbclemmer commented Jan 14, 2023 • edited Loading

ulysses4ever commented Jan 14, 2023

cbclemmer commented Jan 14, 2023

ulysses4ever commented Jan 14, 2023

cbclemmer commented Jan 14, 2023

cbclemmer commented Jan 18, 2023

ulysses4ever commented Jan 18, 2023

Mikolaj commented Jan 18, 2023

cbclemmer commented Jan 18, 2023

cbclemmer commented Jan 30, 2023

Kleidukos left a comment

Choose a reason for hiding this comment

ulysses4ever left a comment

Choose a reason for hiding this comment

ulysses4ever Jan 31, 2023

Choose a reason for hiding this comment

cbclemmer Feb 28, 2023

Choose a reason for hiding this comment

ulysses4ever Mar 5, 2023

Choose a reason for hiding this comment

ulysses4ever commented Feb 26, 2023

ulysses4ever commented Mar 1, 2023

cbclemmer commented Mar 1, 2023

Kleidukos commented Mar 1, 2023

ulysses4ever commented Mar 4, 2023

Kleidukos left a comment

Choose a reason for hiding this comment

ulysses4ever Mar 5, 2023

Choose a reason for hiding this comment

ulysses4ever commented Mar 7, 2023

cbclemmer commented Mar 7, 2023

ulysses4ever commented Mar 7, 2023

ulysses4ever commented Mar 9, 2023

mergify bot commented Mar 9, 2023 • edited Loading

✅ Backports have been created

cbclemmer commented Jan 14, 2023 •

edited

Loading

mergify bot commented Mar 9, 2023 •

edited

Loading