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

Fix various downloading options; error messages #13

Merged
merged 8 commits into from
Apr 19, 2016

Conversation

bendlas
Copy link
Contributor

@bendlas bendlas commented Apr 4, 2016

Util.runInteractiveProcess doesn't show errors from subprocesses, because getOutput throws, when the subprocess fails.
This patch reorders getOutput into a match on ExitSuccess thus unmasking error messages.

@ttuegel
Copy link
Collaborator

ttuegel commented Apr 5, 2016

This won't quite work. We need to consume the the output concurrently with the process execution or it could block waiting for its buffer to drain. Instead, we should use mapExceptionIO on the getOutput branch to tag the exceptions from that side and rethrow them if and only if the subprocess exits normally.

@ttuegel
Copy link
Collaborator

ttuegel commented Apr 5, 2016

Actually, there's one more thing we have to do: if getOutput throws an exception, it stops consuming output, so it must be replaced with something that drains the subprocess output buffer.

@bendlas
Copy link
Contributor Author

bendlas commented Apr 5, 2016

I added a commit, that fixes the first error, I discovered after enabling error messages: nix-prefetch-url would not find its certificate root. This brought me to the next error: In some cases (wiki it seems), Distribution.Melpa.getDeps, would get passed "" as sourceDirOrEl, which causes a build failure.

Now, I managed to hose my cabal by trying to switch into profiled shell. It was missing ghc and after adding that to build tools, it lists a couple of dependencies missing. Any ideas?

@bendlas
Copy link
Contributor Author

bendlas commented Apr 5, 2016

Note: The ca fix causes an impurity, by referring to /etc/ssl/certs/..., so it probably wouldn't work in chroot. But this seems acceptable for something that generates package lists and /etc/ssl/ is probably pretty standard.

@ttuegel
Copy link
Collaborator

ttuegel commented Apr 5, 2016

I never updated the profiled shell, I just fixed it in 201f59f. You can get a profiled shell with nix-shell -A env --arg profiling true.

The CA error sounds like a problem I had been having with the closure-size branch of Nixpkgs. What commit are you on?

@bendlas
Copy link
Contributor Author

bendlas commented Apr 5, 2016

cool, thanks

I'm on channels/nixos-unstable-small plus my commit queue: https://github.com/bendlas/nixpkgs/tree/dev

@bendlas
Copy link
Contributor Author

bendlas commented Apr 5, 2016

But you're right, the ssl fail is fishy. Judging from this line https://github.com/NixOS/nix/blob/master/src/libstore/download.cc#L145
the file under /etc/ssl should be picked anyway ...

Can you reproduce the ssl error, e.g. with ac-dabbrev?

@bendlas
Copy link
Contributor Author

bendlas commented Apr 5, 2016

Cool, now when trying to reproduce it, I'm getting

PackageException "ac-dabbrev" (ProcessFailed "nix-prefetch-url" ["https://www.emacswiki.org/emacs/download/ac-dabbrev.el"] (Died 1 "error: Nix database directory \8216/nix/var/nix/db\8217 is not writable: Permission denied\n"))

Plain nix-prefetch-url works ... brb, rebooting

@bendlas
Copy link
Contributor Author

bendlas commented Apr 5, 2016

doh, of course I'm not seeing the ssl error, having the ssl patch applied locally. The db-not-writable error seems like a new error. Different though, from the build errors I've been seeing and it seems to affect all the melpa packages. I think I should go to bed now.

@bendlas
Copy link
Contributor Author

bendlas commented Apr 5, 2016

OK, the db-not-writable is because of the --pure shell

@ttuegel
Copy link
Collaborator

ttuegel commented Apr 5, 2016

Is there some way to have nix-prefetch-* just download and hash sources, but not copy them into the store?

@bendlas
Copy link
Contributor Author

bendlas commented Apr 6, 2016

Doesn't look like there was such an option: https://github.com/NixOS/nix/blob/master/src/nix-prefetch-url/nix-prefetch-url.cc#L198

@ttuegel
Copy link
Collaborator

ttuegel commented Apr 6, 2016

Well, I would prefer to use pure shells, but that doesn't seem to be an option. For now, let's use impure shells instead. Later, I'll see about getting such an option implemented in Nix.

@bendlas
Copy link
Contributor Author

bendlas commented Apr 13, 2016

With the latest two commits, the update process is looking good. Let the lambdas flow like milk (from a box)

EDIT the update process for melpa, that is

@bendlas
Copy link
Contributor Author

bendlas commented Apr 13, 2016

ooh, curl is run as a subprocess to nix-prefetch-*, so it didn't pick up that SSL_CERT_FILE env var. i fixed that in the scripts, since i don't know how to export an env var from haskell

@bendlas
Copy link
Contributor Author

bendlas commented Apr 13, 2016

Strange, I'm still getting errors for nix-prefetch-hg, as if it didn't honor PRINT_PATH. Running it manually works as expected though ...

@bendlas
Copy link
Contributor Author

bendlas commented Apr 13, 2016

I locally solved the -hg issue, now i see the error with nix-prefetch-git outputting JSON, that you mentioned

@bendlas
Copy link
Contributor Author

bendlas commented Apr 15, 2016

With the latest two patches, I managed to successfully regenerate melpa and melpa-stable.


# usage: ./melpa-packages.sh --melpa PATH_TO_MELPA_CLONE

## env var for curl
export SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bendlas Can we inherit SSL_CERT_FILE from the parent shell? Also, with this change, is it still necessary to set SSL_CERT_FILE in Distribution.Nix.Fetch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we could, but at least on my shell, SSL_CERT_FILE isn't defined. I don't think it needs to be set again in the fetcher, though.

@bendlas bendlas changed the title Show errors from executing processes Fix various downloading options; error messages Apr 17, 2016
@ttuegel ttuegel merged commit 8020532 into nix-community:master Apr 19, 2016
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

2 participants