Closed milkypostman closed 3 years ago
The failure to barf on "realgut" is mysterious to me, since I specifically did work to preserve the failure exit code in that situation, having added the logic for creating per-package build log files. I'll take a look.
is it because in the makefile the line is prefixed with @-
?
https://github.com/melpa/melpa/blob/efd78c643a0ebfe006e87e9e950dd056d04a1b37/Makefile#L111
Yes, just found that, but it was actually also the case prior to my changes. So my worry is that if we change it, it will cause early-exit-on-failure here: https://github.com/melpa/melpa/blob/master/docker/builder/parallel_build_all#L18
do you mind adding a comment in the makefile about why it's ignored?
I will call emacs directly from the CI. Which is what is done in our run-ci script. And you can see it fails now.
Okay, so if just some of the recipes fail, then make waits for unfinished jobs, and exits with a failure. It's not clear to me if it would run all of the jobs, or just the ones it had already started in its 8-parallel-jobs budget. More testing would be required.
do you mind adding a comment in the makefile about why it's ignored?
Will do.
I will call emacs directly from the CI. Which is what is done in our run-ci script. And you can see it fails now.
We should arguably use "make" from the CI too, but that seems more of a rabbit hole.
After a bit more testing, I think I have a way to fix this properly, essentially by making the individual make targets fail as expected, but passing -k to the make invocation in parallel_build_all.
I guess the question is: how do I update the "worker" docker image after modifying that latter script? Does it rebuild automatically after refreshing melpa from git?
haha, ok, I was reading https://www.gnu.org/software/make/manual/make.html#Errors about this same thing.
yes, the MELPA directory is mounted inside the docker image. on the next build the script does a git fetch origin and will get the latest changes.
Sweet, I'll merge that PR then.
LOL. what timing. You must have submitted right before the latest build. Can confirm -k
is being passed to the build command.
probably would be good to have a file that has errors written to it. is it worth redirecting to a file? at least it would contain the errors from the latest run.
Just an FYI: I was testing in my local client and I don't think the -k
is necessary. Because each recipe is specified separately, they are treated as different runs. Keeping -k
is fine but I think it's only for dependency of a target.
I don't think the
-k
is necessary. Because each recipe is specified separately, they are treated as different runs. Keeping-k
is fine but I think it's only for dependency of a target.
Yeah, I wanted to be conservative because the shell is run with -e
, so it will exit before subsequent commands on failure. Maybe that's okay, maybe not.
probably would be good to have a file that has errors written to it. is it worth redirecting to a file? at least it would contain the errors from the latest run.
The build output text file for each recipe will also have errors in it. Or do you mean something else?
oh, sorry, it's been a while. LOL welcome back!
I didn't realize there was a log for each recipe.
what would be good is to have a way to detect any issues with recipes. I.e., a way to figure out if any packages are having issues, not updating, etc.
I used to have scripts that ran and emailed me with problems but since the multi-file build, and because in practice either people notice or don't care, I haven't really audited the state of things.
Right, yes, something like that would make a lot of sense.
@tarsius you good with this PR?
sorry @purcell I think I re-asked for review. I don't fully understand how Github reviews work.
Awesomeness, thanks for tackling this.
Added ci.yml that will clone both the current PR and melpa, and then attempt to build some packages.