-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Update SABnzbd to 4.2.3 #6061
Update SABnzbd to 4.2.3 #6061
Conversation
@hgy59 @th0ma7 I'm a bit confused, the regular build is working fine, however when I trigger a build with |
It might be a general issue with github. There are some errors uploading artifacts (Failed to FinalizeArtifact: ...) |
Yeah I saw that, but I think it's different from the publish problem that the build step seems to be skipped. |
Indeed it may be due to my pr as publishing could not be tested through github. I'll have a look at it over the weekend. |
I've re-run the failed jobs on that build and it was successful the second time around. I did see some failed tests but it completed. @Safihre maybe you can re-run the failed jobs in your repo? |
@mreid-tt If you check the logs of my build you can see it completely skips the building of |
@th0ma7 the error came indeed with #6002 before, a With your changes a You splitted the publish part from spksrc.spk.mk to spksrc.publish.mk, but I can't find parts of the following target of spk.mk in publish.mk:
since PLEASE fix the *.mk files and do not adjust the github scripts (i.e. please restore the behaviour that |
This is now fixed with #6064
|
hey @hgy59, thanks for the fix. I can confirm that the package now builds with publish action in GitHub. There is however a new problem with the publish URL as it now seems to be invalid. From my test run, I get the following error now:
|
@mreid-tt that's strange, because locally it works:
for the output above, I temp. set revision of synocli-misc to 4444 and run |
hey @hgy59, based on this code: Line 54 in 3b21f64
It seems that the Now I can set it in my GitHub variables like the Publish a package from Github wiki but this was not required before. |
hey @hgy59, checking into my build run logs, I see the following which may explain it:
|
There are some more issues in
With this, the empty PUBLISH_URL would at least be reported. |
Looking into the build log I don't see any of these echo command outputs present: Lines 176 to 177 in 3b21f64
Lines 187 to 188 in 3b21f64
which suggests that this line isn't being called at all: Line 174 in 3b21f64
|
Yes, this is an issue. when locally I can successfully call |
This is because local.mk is not generated when |
Even I cannot reproduce the error `` locally, I guess that it is caused by:
so far none of the *.mk files was designed to be included from a Makefile in the /spksrc folder. |
Just woke-up (and to catch-up), ...and sorry for all the troubles my PR provided. At first glance I doubt the EDIT: I can't reproduce the error above on neither my LXC or docker environment... weird... |
As this is for building using GitHub, running a test would not change the result unless this is merged.
hey @th0ma7, no worries. Do take a look at the proposed fix from @hgy59 and work from there. I'll await your findings. |
I may have found the culprit:
Whereas using github-action I believe the spksrc repository sits under
|
oos, I did a local test with |
@th0ma7 I can confirm that the error occures when building in
but the same command succeeds on #6065 (when |
Since the tests are global, we should either add all test targets in the global Makefile and not include any file from mk/ folder. as I mentioned above, the mk/*.mk files are not desigend to be included from a file not placed two directory levels below the work folder. |
From my findings I created a PR #6066 with a simple fix. I believe it's ok for merging so another attempt for publishing can be done. It shouldn't cause any issue, with hope this actually solves it. |
@hgy59 the reason I took the liberty to
Indeed, whereas the |
I've done a new build action with the latest changes to the spksrc and the publish actions were successful. @Safihre, I've not activated the builds in the admin interface so you can review and/or do final tests before publishing. |
Thnx everyone for the fixing and testing! |
@Safihre, just as an FYI, I did a manual install on my DS916+ and the installation was successful. See log below: Log file
|
Feel free to publish :) I'm away from computer for a few days and the building part didn't change, so I'm sure it's good! |
Description
Update SABnzbd to 4.2.3.
Simple update. If no remarks I'll merge and publish end of the week.
Checklist
all-supported
completed successfullyType of change