Fix the timestamp shown during cabal update #7934

andreabedini · 2022-01-27T14:02:11Z

Passing an index-state timestamp to cabal update currently makes it
write a message saying the index has been updated to the previous
timestamp.

$ cabal update hackage.haskell.org,2016-09-24T17:47:48Z
Downloading the latest package list from hackage.haskell.org
Package list of hackage.haskell.org is up to date at index-state 2022-01-27T12:59:23Z

This is because of a confusion between downloading the index and setting
the index state. These are independent actions.

This patch makes separates the two messages (updating the index and
updating the index-state timestamp) and makes sure the right timestamp
is printed.

~~I am sure there was an issue for this but I cannot find it (@fgaz ?).~~

This addresses #7246

I have tested this manually, with something like

$ export CABAL_DIR=$HOME/cabal-tmp
$ $(cabal list-bin cabal) update -v hackage.haskell.org,2016-09-24T17:47:48Z
$ $(cabal list-bin cabal) update -v hackage.haskell.org,2022-01-27T12:59:23Z
$ $(cabal list-bin cabal) update -v

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.

Please also shortly describe how you tested your change. Bonus points for added tests!

Mikolaj · 2022-01-27T14:18:12Z

Hi! Thank you for the PR. I can't find the mini-changelog file. A mini-test would be great, too...

andreabedini · 2022-01-27T14:31:27Z

Hi @Mikolaj thanks for the quick reply. I added a the mini changelog file (it's minimal but the change is not big either).
Regarding tests, this change is about a message show to the user. I haven't found a test I could base upon. Could you point me in the right direction?

robx · 2022-01-27T21:52:51Z

@andreabedini The tests under cabal-testsuite test cabal output, e.g. https://github.com/haskell/cabal/blob/master/cabal-testsuite/PackageTests/Outdated/outdated.out, maybe that would work? The documentation is pretty good, too: https://github.com/haskell/cabal/blob/master/cabal-testsuite/README.md. (But not sure if test-local-repo that's provided to those tests is in a state to cover this change.)

andreabedini · 2022-01-28T03:49:36Z

Hi @robx,

this is very helpful thanks. I was looking for tests inside the cabal-install folder and I didn't notice the testsuite. Documentation is good indeed!

I have played along the lines of withRepo to set up hackage as a remote repository but this makes cabal actually download the whole index, which can be considered expensive (>200MB).

I just pushed the test implementation. Let me know if I am on the right track. Thanks!

andreabedini · 2022-01-28T04:43:16Z

Before the patch, the sequence

cabal update hackage.haskell.org,2022-01-28T02:36:41Z
cabal update hackage.haskell.org,2016-09-24T17:47:48Z
cabal update hackage.haskell.org,2022-01-28T02:36:41Z

would produce

# cabal update
Downloading the latest package list from hackage.haskell.org
Updated package list of hackage.haskell.org to the index-state 2022-01-28T02:36:41Z
# cabal update
Downloading the latest package list from hackage.haskell.org
Package list of hackage.haskell.org is up to date at index-state 2022-01-28T02:36:41Z
# cabal update
Downloading the latest package list from hackage.haskell.org
Package list of hackage.haskell.org is up to date at index-state 2016-09-24T17:47:48Z

The second and third timestamps are wrong; this is only wrong in the message, cat ~/.cabal/packages/hackage.haskell.org/01-index.timestamp can confirm the index-state was set correctly.

After the patch, it produces:

# cabal update
Downloading the latest package list from hackage.haskell.org
Package list of hackage.haskell.org has been updated.
The index-state is set to 2022-01-28T02:36:41Z.
To revert to previous state run:
    cabal v2-update 'hackage.haskell.org,'
# cabal update
Downloading the latest package list from hackage.haskell.org
Package list of hackage.haskell.org is up to date.
The index-state is set to 2016-09-24T17:47:48Z.
To revert to previous state run:
    cabal v2-update 'hackage.haskell.org,2022-01-28T02:36:41Z'
# cabal update
Downloading the latest package list from hackage.haskell.org
Package list of hackage.haskell.org is up to date.
The index-state is set to 2022-01-28T02:36:41Z.
To revert to previous state run:
    cabal v2-update 'hackage.haskell.org,2016-09-24T17:47:48Z'

Where the message about setting the index-state is separated from the message about updating the index. In addition the instructions on how to revert to the previous index-state are now displayed everytime the index-state changes.

robx · 2022-01-28T09:00:53Z

I have played along the lines of withRepo to set up hackage as a remote repository but this makes cabal actually download the whole index, which can be considered expensive (>200MB).

I just pushed the test implementation. Let me know if I am on the right track. Thanks!

Yeah unfortunately it seems a remote repo is needed to test this. The "right" way to test that within this test suite would be to spawn a fake hackage server on localhost in-test. But that seems like excessive effort for this change.

I don't know the code-base well though, maybe there's something around somewhere? I imagine there might be some integration test coverage of communication with a remote repo. I'll have to defer to the maintainers, sorry.

Mikolaj · 2022-01-28T09:04:29Z

I don't remember myself. Could you grep the repo (git grep) for likely keywords? Let's just emulate how it's tested to date and if it's not, that's a good excuse to skip the test, given that this feature does not change the actual semantics of the update (though we'd prefer the tests to confirm that, even more than to confirm the display is now better).

andreabedini · 2022-01-28T13:26:11Z

I just realised taking to the real hackage will make the test flaky. If the index actually changes between the two calls the message will evidently change.

I will have a check if we have any test infra for hackage.

andreabedini · 2022-01-31T07:23:39Z

all right, I had a look a I couldn't find anything regarding a remote repository. Here is my plan:

with hackage-security (or with the hacakge-repo-tool executable) we can create a basic repo with keys and everything [1]
then we create a withRemoteRepo that serves the repo on HTTP for cabal to query
the test then works as expected

Furhter notes:

Unfortunately because of a bug it has to have at least one package and cannot be empty
The 'index-state' functionality requires a secure hackage repository

Comments? Feedback? It's totally possible that I missed something we already have.

EDIT: if it's too complicated to set up a secure repo on the fly in Haskell code, we can prepopulate one and check it into git

Mikolaj · 2022-01-31T11:53:19Z

Would that help?

cabal-testsuite/README.md-If possible, define the package locally.  If the package needs
cabal-testsuite/README.md:to be from Hackage (e.g., you are testing the global store code
cabal-testsuite/README.md:in new-build), use `withRepo "repo"` to initialize a "fake" Hackage with
cabal-testsuite/README.md-the packages placed in the `repo` directory.

cabal-testsuite/src/Test/Cabal/Monad.hs:    hackageRepoToolProgram,

andreabedini · 2022-01-31T14:00:06Z

@Mikolaj yes it does! 🤦

andreabedini · 2022-02-01T05:50:20Z

@robx @Mikolaj I have implemented the plan above. At the moment I am relying on the presence of python3 for a simple web server, this can done in Haskell too but I was unsure about bringing in dependencies.

andreabedini · 2022-02-01T05:53:33Z

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

@@ -63,7 +63,8 @@ run _verbosity mb_cwd env_overrides path0 args input = do
          , std_out = UseHandle writeh
          , std_err = UseHandle writeh
          }
-    (stdin_h, _, _, procHandle) <- createProcess prc
+
+    withCreateProcess prc $ \stdin_h _ _ procHandle -> do


This is required to make sure the spawn process is termined with the Haskell thread is terminated. This is important when we want to run long-running processes in parallel with a part of our code.

Mikolaj · 2022-02-01T07:54:13Z

python3 is already required by CI to test bootstraping and to test generating users-guide. But I don't know if cabal test run of our packages required it so far. If not, this new dependency should be documented. In the worst case we can make the test optional and not run by default (but enable it in our CI) --- I don't know what the general savoir-vivre is about requiring OS dependencies for tests above what is required by cabal build of the package.

Opinions, anybody?

andreabedini · 2022-02-01T08:22:22Z

FWIW, there's a

skipUnless "no python3" =<< isAvailableProgram python3Program

guarding withRemoteRepo, so at least it shouldn't just fail.

Mikolaj · 2022-02-01T08:45:46Z

FWIW, there's a
skipUnless "no python3" =<< isAvailableProgram python3Program
guarding withRemoteRepo, so at least it shouldn't just fail.

Oh great, so perhaps that'd be enough. What does it really do if there's no python? Crash? Stop tests? Displays a warning and goes on with tests?

andreabedini · 2022-02-01T08:50:02Z

Oh great, so perhaps that'd be enough. What does it really do if there's no python? Crash? Stop tests? Displays a warning and goes on with tests?

My understanding is that any test using withRemoteRepo would get skipped.

Just as we discuss this, I notice that I am never checking for the presence of hackage-repo-tool and indeed the tests are failing. Apologies. 1) I'll add skipUnless "no hackage-repo-tool" =<< isAvailableProgram hackageRepoTool 2) I'll see if I can add hackage-repo-tool to the CI.

andreabedini · 2022-02-01T09:15:18Z

I am afraid our only option is to install hackage-repo-tool from source :-/

Mikolaj · 2022-02-01T09:21:50Z

I am afraid our only option is to install hackage-repo-tool from source :-/

That's fine. With some luck (and configuration) CI would even cache the compiled tool.

andreabedini · 2022-02-02T09:08:57Z

There's a failure on GHC 7.10.3 due to withCreateProcess. Maybe it's just the bounds needs to be tightened. Problem is I cannot install GHC 7.10.3 on my work machine because https://gitlab.haskell.org/ghc/ghc/-/issues/20782. I need more time :)

Mikolaj · 2022-02-02T09:29:20Z

Please keep in mind #7534. So if it only breaks cabal-install testing (not Cabal), just mark the test as failing and move on.

andreabedini · 2022-02-03T02:04:35Z

Please keep in mind #7534. So if it only breaks cabal-install testing (not Cabal), just mark the test as failing and move on.

Thanks. Ok, it only breaks on ghc-7.10.3 likely because it picks a old version of process which does not have withCreateProcess.

cabal-testsuite pass on Windows but the test get skipped because it relies on unix paths (same as with withRepo, that part is common).

~~I don't see the testsuite being run at all on linux and mac.~~

Sorry for the continous stream of conciousness, the test is rightly skipped on linux and macos because of the missing hackage-repo-tool. I'll just add that to the CI then.

andreabedini · 2022-02-21T03:31:29Z

Rebased on master

andreabedini · 2022-02-23T03:50:30Z

I have some failures on MacOS that I need to investigate.

andreabedini · 2022-05-05T05:47:33Z

Oh right, checks have passed now but I belive the test I wrote is flaky and I would not rely on it much. It uses threadDelay to wait until the python webserver is up but this is very unreliable. I am testing a solution that waits for the webserver port to be available.

andreabedini · 2022-05-06T04:35:55Z

@jneira @Mikolaj I am more confident I have the right solution now.

Timing might have not been the only problem after all. Leaving the binding address unspecified might have caused a mistmatch between the http server and cabal on the address (and the family of addresses (IPv4/IPv6)). This is inherently platform and host configuration dependent, which must have been why the test was failing only on mac originally.

You can see here that waiting an entire minute was not enough.

You can also try to reproduce this confusion on your machine. This is what I get on my (it defaults to IPv4 and localhost is on IPv6).

❯ python3 -m http.server -d /tmp 
Serving HTTP on 0.0.0.0 port 8000 (http://0.0.0.0:8000/) ...
^C
Keyboard interrupt received, exiting.

❯ python3 -m http.server --bind localhost -d /tmp 
Serving HTTP on ::1 port 8000 (http://[::1]:8000/) ...
^C
Keyboard interrupt received, exiting.

Hopefully, generous waiting for the server to come up (exponential backoff limited at one minute) and explicity binding host would do the trick.

<3

Mikolaj

Magnificent work.

Passing an index-state timestamp to cabal update currently makes it write a message saying the index has been updated to the *previous* timestamp. $ cabal update hackage.haskell.org,2016-09-24T17:47:48Z Downloading the latest package list from hackage.haskell.org Package list of hackage.haskell.org is up to date at index-state 2022-01-27T12:59:23Z This is because of a confusion between downloading the index and setting the index state. These are independent actions. This patch makes separates the two messages (updating the index and updating the index-state timestamp) and makes sure the right timestamp is printed.

- Introduce `withRemoteRepo`. The function `withRemoteRepo` takes a folder with packages (in the same way as `withRepo` and presents it to cabal as a secure remote repository. - Convert NewUpewte/UpdateIndexState test from using Hackage to withRemoteRepo.

Note: as it is, this won't work on GHC 8.2 because network-wait-0.1.1.0 doesn't support it. Hopefully we can resolve this soon.

@mbg

Thanks @mbg!

jneira · 2022-05-06T15:28:12Z

.github/workflows/validate.yml

+        run: |
+          cd $(mktemp -d)
+          cabal install hackage-repo-tool
+


great move, improving the test suite

andreabedini · 2022-05-09T03:43:18Z

@jneira @Mikolaj Thank you for encouraging me to get this through. Very appreciated <3.

As written in the comments current_ts can be a non valid timestamp (nullTimestamp) if the timestamp is missing. In this case it does not make sense to tell the user how to revert to current_ts.

Minor clean up after PR #7934

andreabedini force-pushed the andrea/fix-update-message-timestamp branch from 7934d90 to eef2219 Compare January 27, 2022 14:03

Mikolaj added the cabal-install: cmd/update label Jan 27, 2022

andreabedini force-pushed the andrea/fix-update-message-timestamp branch from eef2219 to e431a36 Compare January 27, 2022 14:29

andreabedini commented Feb 1, 2022

View reviewed changes

andreabedini force-pushed the andrea/fix-update-message-timestamp branch from 28db31d to 97ce5c4 Compare February 21, 2022 03:31

gbaz approved these changes Feb 24, 2022

View reviewed changes

andreabedini self-assigned this Feb 25, 2022

andreabedini force-pushed the andrea/fix-update-message-timestamp branch 2 times, most recently from 63e5026 to 2db988f Compare May 5, 2022 03:31

andreabedini mentioned this pull request May 5, 2022

Fix index-state falling back warning #8086

Draft

Mikolaj approved these changes May 6, 2022

View reviewed changes

Mikolaj added the merge me Tell Mergify Bot to merge label May 7, 2022

andreabedini added 11 commits May 7, 2022 18:44

Add test case for cabal update message

a5fdd34

Skip withRemoteRepo if hackageRepoToolProgram is not available

6581a59

Install hackage-repo-tool in CI

d007aac

Avoid threadDeleay in withRemoteRepo

ac9d6aa

Note: as it is, this won't work on GHC 8.2 because network-wait-0.1.1.0 doesn't support it. Hopefully we can resolve this soon.

Bump network-wait version.

6310e9f

Thanks @mbg!

Replace <> with mappend to please ghc 8.0

b3b2f3e

Change waiting strategy

8145f8c

Wait more

9a55a0f

Explicit is better than implicit

00c9776

andreabedini force-pushed the andrea/fix-update-message-timestamp branch from bb0d71e to 00c9776 Compare May 7, 2022 18:44

mergify bot merged commit d80e24f into haskell:master May 7, 2022

jneira reviewed May 7, 2022

View reviewed changes

.github/workflows/validate.yml

run: |

cd $(mktemp -d)

cabal install hackage-repo-tool

Copy link

Member

jneira May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great move, improving the test suite

andreabedini added a commit that referenced this pull request May 9, 2022

Add more comments after PR #7934.

b8edcfe

andreabedini added a commit that referenced this pull request May 14, 2022

Add more comments after PR #7934.

0d4abc4

mergify bot added a commit that referenced this pull request May 14, 2022

Merge pull request #8131 from haskell/andreabedini/patch-1

c901542

Minor clean up after PR #7934

andreabedini deleted the andrea/fix-update-message-timestamp branch September 5, 2022 14:18

andreabedini mentioned this pull request Jan 18, 2023

cabal v2-update behaves strangely even with index-state set #7246

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the timestamp shown during cabal update #7934

Fix the timestamp shown during cabal update #7934

andreabedini commented Jan 27, 2022 •

edited

Loading

Mikolaj commented Jan 27, 2022

andreabedini commented Jan 27, 2022

robx commented Jan 27, 2022

andreabedini commented Jan 28, 2022

andreabedini commented Jan 28, 2022

robx commented Jan 28, 2022

Mikolaj commented Jan 28, 2022 •

edited

Loading

andreabedini commented Jan 28, 2022

andreabedini commented Jan 31, 2022 •

edited

Loading

Mikolaj commented Jan 31, 2022

andreabedini commented Jan 31, 2022

andreabedini commented Feb 1, 2022

andreabedini Feb 1, 2022

Mikolaj commented Feb 1, 2022

andreabedini commented Feb 1, 2022

Mikolaj commented Feb 1, 2022

andreabedini commented Feb 1, 2022

andreabedini commented Feb 1, 2022

Mikolaj commented Feb 1, 2022

andreabedini commented Feb 2, 2022

Mikolaj commented Feb 2, 2022

andreabedini commented Feb 3, 2022 •

edited

Loading

andreabedini commented Feb 21, 2022

andreabedini commented Feb 23, 2022

andreabedini commented May 5, 2022

andreabedini commented May 6, 2022 •

edited

Loading

Mikolaj left a comment

jneira May 6, 2022

andreabedini commented May 9, 2022

Fix the timestamp shown during cabal update #7934

Fix the timestamp shown during cabal update #7934

Conversation

andreabedini commented Jan 27, 2022 • edited Loading

Mikolaj commented Jan 27, 2022

andreabedini commented Jan 27, 2022

robx commented Jan 27, 2022

andreabedini commented Jan 28, 2022

andreabedini commented Jan 28, 2022

robx commented Jan 28, 2022

Mikolaj commented Jan 28, 2022 • edited Loading

andreabedini commented Jan 28, 2022

andreabedini commented Jan 31, 2022 • edited Loading

Mikolaj commented Jan 31, 2022

andreabedini commented Jan 31, 2022

andreabedini commented Feb 1, 2022

andreabedini Feb 1, 2022

Choose a reason for hiding this comment

Mikolaj commented Feb 1, 2022

andreabedini commented Feb 1, 2022

Mikolaj commented Feb 1, 2022

andreabedini commented Feb 1, 2022

andreabedini commented Feb 1, 2022

Mikolaj commented Feb 1, 2022

andreabedini commented Feb 2, 2022

Mikolaj commented Feb 2, 2022

andreabedini commented Feb 3, 2022 • edited Loading

andreabedini commented Feb 21, 2022

andreabedini commented Feb 23, 2022

andreabedini commented May 5, 2022

andreabedini commented May 6, 2022 • edited Loading

Mikolaj left a comment

Choose a reason for hiding this comment

jneira May 6, 2022

Choose a reason for hiding this comment

andreabedini commented May 9, 2022

andreabedini commented Jan 27, 2022 •

edited

Loading

Mikolaj commented Jan 28, 2022 •

edited

Loading

andreabedini commented Jan 31, 2022 •

edited

Loading

andreabedini commented Feb 3, 2022 •

edited

Loading

andreabedini commented May 6, 2022 •

edited

Loading