Skip to content

Update mtime on repo index file even if up to date #4950

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

Closed
wants to merge 1 commit into from

Conversation

psiska
Copy link
Collaborator

@psiska psiska commented Dec 11, 2017

Resolves: #4444

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.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

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

@23Skidoo
Copy link
Member

23Skidoo commented Dec 12, 2017

Haven't reviewed in detail, but this looks similar to #4578. Have you looked at that PR and the accompanying discussion?

@psiska
Copy link
Collaborator Author

psiska commented Dec 12, 2017

Yes I did have a look on #4578. Approach in that issue was to use mtime of timestamp files instead of index files. But actually you have pointed out it works only for secure remote repos.

Approach taken in this issue is like follows.
Warnings is emitted only when mtime on index is more than 15 days. So let's just modify mtime even if the local index is same as remote.
Covers both secure and unsecure repos.

@23Skidoo
Copy link
Member

Awesome, I'll try to test this today or tomorrow.

@23Skidoo
Copy link
Member

I tested this and it seems to work.

@23Skidoo
Copy link
Member

Merged, thanks!

@23Skidoo 23Skidoo closed this Dec 19, 2017
@23Skidoo
Copy link
Member

Merged manually, see f05615c.

@phadej
Copy link
Collaborator

phadej commented Dec 19, 2017

GHC-7.10.3 (directory-1.2.2.0):

Distribution/Client/Update.hs:43:26:
    Module ‘System.Directory’ does not export ‘setModificationTime’

setModificationTime is only in directory-1.2.3.0. http://hackage.haskell.org/package/directory-1.3.1.5/changelog

Please, be very careful with manual merges. Also https://travis-ci.org/haskell/cabal/jobs/315016658 (build/job of this PR) shows this failure.

Ping @23Skidoo

@23Skidoo
Copy link
Member

23Skidoo commented Dec 19, 2017

Ugh, I suspected that this would be the case. Will fix.

edit: See #4962.

@23Skidoo
Copy link
Member

BTW, it looks like we should apply the same fix to new-update too (D.C.CmdUpdate). @psiska, would you be interested in looking into that?

23Skidoo added a commit to 23Skidoo/cabal that referenced this pull request Dec 19, 2017
@psiska
Copy link
Collaborator Author

psiska commented Dec 19, 2017

Yes, I can have a look on Thursday / Friday.
Regarding the breakage - it looks like there is difference in what version of 'directory' is installed. The older version (< 1.2.3.0) is installed only with 7.10.3 - all other builds are choosing newer directory.

Looking into travis-install.sh does not reveal anything strange to me as the cabal-2.0.0.0 is installed in all environments. The fact that same cabal chooses different version of directory, looks to me like some kind of problem. Is this normal behavior?

@23Skidoo
Copy link
Member

@psiska GHC 7.10 comes with directory-1.2.2.0, and since that version is not excluded by the version bounds in cabal-install.cabal, it gets picked up by the solver. #4962 fixes the build error.

23Skidoo added a commit to 23Skidoo/cabal that referenced this pull request Dec 19, 2017
23Skidoo added a commit to 23Skidoo/cabal that referenced this pull request Dec 19, 2017
23Skidoo added a commit to 23Skidoo/cabal that referenced this pull request Feb 1, 2018
@23Skidoo 23Skidoo mentioned this pull request Feb 1, 2018
4 tasks
23Skidoo added a commit to 23Skidoo/cabal that referenced this pull request Feb 1, 2018
23Skidoo added a commit that referenced this pull request Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants