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

Bad C-c handling during 'stack init' #3073

Open
sjakobi opened this Issue Mar 19, 2017 · 6 comments

Comments

Projects
None yet
5 participants
@sjakobi
Contributor

sjakobi commented Mar 19, 2017

(This is a follow up from the discussion in #3055)

~/s/hackage-server (master|✔) $ stack init
Looking for .cabal or package.yaml files to use to init the project.
Using cabal packages:
- hackage-server.cabal
- tests/unpack-checks/missing-configure-0.1.0.0/missing-configure.cabal
- tests/unpack-checks/correct-package-0.1.0.0/correct-package.cabal

Selecting the best among 10 snapshots...

* Partially matches lts-8.5
    ....

Downloaded nightly-2017-03-19 build plan.    
Missing some cabal revision files, updating indices
Selected mirror https://s3.amazonaws.com/hackage.fpcomplete.com/                                 
Downloading timestamp                                                                            
Downloading snapshot                                                                             
Updating index                                                                                   
UpException user interrupt when using mirror https://s3.amazonaws.com/hackage.fpcomplete.com/      
Selected mirror http://hackage.fpcomplete.com/                                                   
Downloading timestamp                                                                            
Downloading snapshot                                                                             
Updating index                                                                                   
Updated package list downloaded                                                                  

Note the line UpException user interrupt when using mirror https://s3.amazonaws.com/hackage.fpcomplete.com/ – this is where I pressed C-c.

I would have expected stack to abort then but it appears as if stack simply retries downloading the index.

@mgsloan

This comment has been minimized.

Collaborator

mgsloan commented Mar 19, 2017

We don't have any such retry logic, looks like it is an upstream issue with hackage-security: haskell/hackage-security#187

snoyberg added a commit that referenced this issue Feb 13, 2018

Avoid async exception mishandling #3073
This commit SHOULD NOT BE MERGED TO master. It adds an extra-dep for
hackage-security from Hackage to work around #3073 and
haskell/hackage-security#187. Hopefully this
will be merged and released to Hackage.
@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Feb 13, 2018

I've created a patch for hackage-security which should hopefully address this:

haskell/hackage-security#202

I've also pushed a branch 3073-hackage-security-async-exceptions with commit 4bf68f0, which moves over to this patched hackage-security. It would be great to get some feedback on whether this patch fixes the issues described here.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Feb 13, 2018

I just tested the repro above: with previous builds of Stack against unpatched hackage-security, hitting Ctrl-C during the Updating index phase causes the output above. With this patched version, the program exits immediately.

snoyberg added a commit that referenced this issue Feb 14, 2018

Include patched hackage-security for #3073
NOTE: This is included via an extra-dep, which would constitute the
first time Stack would include a patched version of an upstream library.
This is due to the fact that
haskell/hackage-security#203 is likely not going
to be merged, despite fixing issues affecting Stack. This leaves us with
(AFAICT) 4 choices at the Stack level:

1. Continue using the officially released upstream version of
   hackage-security, bugs and all
2. Fork hackage-security on Hackage, and depend on the fork
3. Inline the code from hackage-security into Stack itself, and drop the
   explicit dependency on hackage-security
4. Include hackage-security via an `extra-dep` pointing at a Git commit.
   Our official builds will use the patched version of hackage-security,
   and anyone building from Hackage will end up with the unpatched version

This PR represents approach (4). If and when the PR is merged and
released to Hackage, this becomes a non-issue. But generally speaking,
we should have a policy in Stack for handling these kinds of upstream
issues cases.
@dcoutts

This comment has been minimized.

dcoutts commented Feb 14, 2018

Thanks for the contributions upstream. I'd suggest to persist with getting the contributions or equivalent fixes into the upstream package as that way everyone benefits from everyone else's improvements.

snoyberg added a commit that referenced this issue Feb 14, 2018

Include patched hackage-security for #3073
NOTE: This is included via an extra-dep, which would constitute the
first time Stack would include a patched version of an upstream library.
This is due to the fact that
haskell/hackage-security#203 is likely not going
to be merged, despite fixing issues affecting Stack. This leaves us with
(AFAICT) 4 choices at the Stack level:

1. Continue using the officially released upstream version of
   hackage-security, bugs and all
2. Fork hackage-security on Hackage, and depend on the fork
3. Inline the code from hackage-security into Stack itself, and drop the
   explicit dependency on hackage-security
4. Include hackage-security via an `extra-dep` pointing at a Git commit.
   Our official builds will use the patched version of hackage-security,
   and anyone building from Hackage will end up with the unpatched version

This PR represents approach (4). If and when the PR is merged and
released to Hackage, this becomes a non-issue. But generally speaking,
we should have a policy in Stack for handling these kinds of upstream
issues cases.

snoyberg added a commit that referenced this issue Feb 16, 2018

Include patched hackage-security for #3073
NOTE: This is included via an extra-dep, which would constitute the
first time Stack would include a patched version of an upstream library.
This is due to the fact that
haskell/hackage-security#203 is likely not going
to be merged, despite fixing issues affecting Stack. This leaves us with
(AFAICT) 4 choices at the Stack level:

1. Continue using the officially released upstream version of
   hackage-security, bugs and all
2. Fork hackage-security on Hackage, and depend on the fork
3. Inline the code from hackage-security into Stack itself, and drop the
   explicit dependency on hackage-security
4. Include hackage-security via an `extra-dep` pointing at a Git commit.
   Our official builds will use the patched version of hackage-security,
   and anyone building from Hackage will end up with the unpatched version

This PR represents approach (4). If and when the PR is merged and
released to Hackage, this becomes a non-issue. But generally speaking,
we should have a policy in Stack for handling these kinds of upstream
issues cases.

borsboom added a commit that referenced this issue Feb 16, 2018

Merge pull request #3865 from commercialhaskell/3073-fix-hackage-secu…
…rity

Include patched hackage-security for #3073
@borsboom

This comment has been minimized.

Contributor

borsboom commented Mar 30, 2018

It looks like hackage-security-0.5.3.0 includes a file locking fix (although it appears to be different than @snoyberg's contributed fix). Can anyone confirm whether that fixes the problem?

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Apr 1, 2018

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