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

Support for --index-state on cabal update #4341

Merged
merged 4 commits into from Feb 24, 2017

Conversation

ezyang
Copy link
Contributor

@ezyang ezyang commented Feb 18, 2017

CC @hvr

@mention-bot
Copy link

@ezyang, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edsko, @dcoutts and @23Skidoo to be potential reviewers.

@@ -262,7 +262,7 @@ getSourcePackagesAtIndexState verbosity repoCtxt mb_idxState = do
return ()
IndexStateTime ts0 -> do
when (isiMaxTime isi /= ts0) $
warn verbosity ("Requested index-state " ++ display ts0
info verbosity ("Requested index-state " ++ display ts0
Copy link
Member

Choose a reason for hiding this comment

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

Why? IMO warn is more appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, my mental model of --index-state is that you are saying, "Give me the state of the index at time X." In this case, the date I pass in might not correspond to any timestamp explicitly mentioned in the index. But this doesn't feel like a warning case to me: the time of last update might be different from the time I gave, but what's wrong with that?

Copy link
Member

Choose a reason for hiding this comment

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

OK. I was thinking of the case where the requested timestamp is invalid, e.g. in the future or deep past, so the command doesn't do what the user expects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I suppose that is something we could warn about, but that's a different warning!

The strategy is to save an 01-index.timestamp file that remembers
what --index-state the user requested during cabal update.
Subsequently, we use that index state if no more precise
index state was specified (via the flag or a project config.)

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
@ezyang ezyang merged commit 0e74ff0 into haskell:master Feb 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants