l3build icon indicating copy to clipboard operation
l3build copied to clipboard

Allow ctan upload after validation failures

Open benzea opened this issue 5 years ago • 15 comments

My package only has HTML documentation (I have some "fun" code to run the sphinx make from build.lua).

Either way, the CTAN validator does not like uploads without a PDF. However, they are fine with this in general, and it should be OK as long as one explains what is going on.

My simple suggestion would be mostly ignore the validation result. If ctanupload == true abort, but otherwise, still prompt maybe with a different message.

benzea avatar May 02 '20 20:05 benzea

Hmm It may instead be better if we raise this with the ctan maintainers. It would be better if the validator didn't fail on submissions that they would accept. Running a validation but ignoring the result seems wrong, although we could perhaps have some kind of --force or --skip-validation option that skipped the validation step.

davidcarlisle avatar May 02 '20 20:05 davidcarlisle

I am already talking to them about this. That was the reason to open this bug (have not gotten around to proposing a patch yet). :)

So, the situation on the CTAN side seems to be that validation errors are generally acceptable, as long as you can explain them in the "Administrative notes" ("note") field. So in this case, I would just say "Use index.html for documentation" and everything should be good.

That said, there is another discussion to be had with the texdoc people. For me it is currently picking an HTML file at random rather than index.html. But that should be an easy fix, by e.g. prefering index.html slightly in general or handling it as if it had the same name as the directory.

I suspect in the long run the CTAN validator is likely to be updated to accept HTML documentations.

benzea avatar May 03 '20 07:05 benzea

Perhaps we could have a variable storing expected errors and accepting the validation as successful if the validator gives the expected list of errors? (Does the ctan validator list all errors found, or only the first one?) Then this does not diminish the use of the validator.

blefloch avatar May 03 '20 08:05 blefloch

I only have one error right now, but it gives

[["INFO","TDS archive found","sdaps.tds.zip"],["ERROR","Missing PDF documentation"]]

So, looks like it would be feasible that I added Missing PDF documentation to a list of errors that should be ignored.

I would already be happy if l3docs yelled at me a bit and I could still hit the "I am sure" button each time. :)

benzea avatar May 03 '20 08:05 benzea

On Sun, 3 May 2020 at 09:04, Bruno Le Floch [email protected] wrote:

Perhaps we could have a variable storing expected errors and accepting the validation as successful if the validator gives the expected list of errors? (Does the ctan validator list all errors found, or only the first one?) Then this does not diminish the use of the validator.

certainly that's possible although currently l3build checks the return status of the validation run, so a clear yes/no not requiring any parsing of the return body. (The xml geek within still believes validation errors should be fatal:-) I think I'd prefer a boolean settable in build.lua that says to skip validation

davidcarlisle avatar May 03 '20 09:05 davidcarlisle

Am 03.05.20 um 11:14 schrieb David Carlisle:

I think I'd prefer a boolean settable in build.lua that says to skip validation

if we consider the current failure a bug of the validator (that is likely to be fixed at some time) then I would agree.

But if it is likely to stay this way, then having to fully disable validation just to get past this point doesn't seem so good to me.

At least in that case you should get a full listing of the validator "errors" displayed so that you can still benefit from its checking other stuff.

FrankMittelbach avatar May 03 '20 10:05 FrankMittelbach

At least in that case you should get a full listing of the validator "errors" displayed so that you can still benefit from its checking other stuff.

Yeah, I think for now the validator is right for throwing an error. And I think that l3build rightfully shouts at me for the situation. It is just that I know that uploading is still acceptable, and I would have preferred to do so using l3build rather than the CTAN web form. :)

Maybe this should really stay as is. Though in that case, it might be nice to adjust the error a bit, maybe adding a further message:

Not uploading package to CTAN. You cannot use l3build for uploading if your package contains warnings or errors.

benzea avatar May 03 '20 11:05 benzea

looking at the code we don't always just take the status code

  if match(fp_return,"WARNING") or match(fp_return,"ERROR") then
    exit_status=1
  end

....

davidcarlisle avatar May 03 '20 13:05 davidcarlisle

oh well :-) Guess in case of warnings we should continue but clearly display the warnings (perhaps with a WARNING :-) ) but not stop the process. Or, if we want to be on the save side have a variable ctan-allow-warnings and if so not stop at warnings.

FrankMittelbach avatar May 03 '20 13:05 FrankMittelbach

The check on fp_return is in fact just after upload not after validation so that's not the issue here.

Also I get no error for lacking pdf (it is a warning, but maybe this is a change at ctan?)

I just tried with a zip file

$ unzip -l zzz-ctan.zip 
Archive:  zzz-ctan.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
        0  05-03-2020 14:22   zzz/
        5  05-03-2020 14:01   zzz/zzz.sty
        6  05-03-2020 14:21   zzz/zzz.html
---------                     -------
       11                     3 files

and build.lua


module="zzz"

uploadconfig={
announcement="test, ignore",
author="me",
uploader="me",
email="[email protected]",
ctanPath="macros/latex/zzz",
license="lppl",
version="0.0",
summary="testing validation, github 118",
}

and got no validation errors whether I did dry-run (validation only) or a full run when it updated

davidcarlisle avatar May 03 '20 14:05 davidcarlisle

Also I get no error for lacking pdf (it is a warning, but maybe this is a change at ctan?)

Probably they just downgraded it to a warning since I complained about it yesterday :)

Hmm, nope, still getting the following trying to upload the sdaps package (it already fails during validation obviously and is never uploaded):

...re/texlive/texmf-dist/scripts/l3build/l3build-upload.lua:194: Warnings from CTAN package validation:
[["INFO","TDS archive found","sdaps.tds.zip"],["ERROR","Missing PDF documentation"]]

The file is sdaps-ctan.zip if anyone is interested. Repository https://github.com/sdaps/sdaps-class (requires a few dependencies though, so probably not that helpful overall).

benzea avatar May 03 '20 14:05 benzea

@benzea Thanks I can reproduce with that zip (using the same build.lua that I post above, just changing the module name) I should be able to narrow that down now to see what is happening

davidcarlisle avatar May 03 '20 21:05 davidcarlisle

Hmm, maybe it only happens because I have historically started to use .ins/.dtx source files? Considering I don't want to create any documentation from the sources, it might be a good idea to just ship the .sty files directly. No idea if that might make a difference, but could be cleaner overall.

benzea avatar May 04 '20 07:05 benzea

@benzea I have had clarification from Gerd on the behaviour of the validation check and now understand the issue better.

If I submit your sdaps-ctan.zip with the uploadconfig above I get

[["INFO","TDS archive found","sdaps.tds.zip"],["ERROR","Missing PDF documentation"]]

l3build will not submit after the error but if there is a "long enough" note then the validation system downgrades errors to warnings and changes the response, so if I use a build.lua

module="sdaps"

uploadconfig={
announcement="test, ignore",
author="me",
uploader="me",
email="[email protected]",
ctanPath="macros/latex/sdaps",
license="lppl",
version="0.0",
summary="testing validation, github 118",
note=[[please will you
accept this
invalid
zip
because
the html
documentation is sufficient:
actually discard it, this is testing  submission by l3build
as discussed in email
David

]]
}

Then the response is

[["INFO","TDS archive found","sdaps.tds.zip"],["WARNING","Missing PDF documentation"]]

Unfortunately l3build still then aborts, but after changing it as

--- a/l3build-upload.lua
+++ b/l3build-upload.lua
@@ -163,14 +163,19 @@ end
       fp_return = shell(ctan_post .. "validate")
     end
   end
-  if match(fp_return,"WARNING") or match(fp_return,"ERROR") then
+  if (match(fp_return,"ERROR")) then
     exit_status=1
   end
 
   -- if upload requested and validation succeeded repost to the upload URL
     if (exit_status==0 or exit_status==nil) then
     if (ctanupload ~=nil and ctanupload ~=false and ctanupload ~= true) then
-      print("Validation successful, do you want to upload to CTAN? [y/n]" )
+      if (match(fp_return,"WARNING")) then
+       print("Warnings from CTAN package validation:\n" .. fp_return)
+      else
+       print("Validation successful." )
+      end
+      print("Do you want to upload to CTAN? [y/n]" )
       local answer=""
       io.stdout:write("> ")
       io.stdout:flush()

Then I get what is I think the response you wanted, warnings from the validator but still offered the upload

Contacting CTAN for validation:
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 1155k    0    86  100 1155k    113  1522k --:--:-- --:--:-- --:--:-- 1522k
Warnings from CTAN package validation:
[["INFO","TDS archive found","sdaps.tds.zip"],["WARNING","Missing PDF documentation"]]
Do you want to upload to CTAN? [y/n]

davidcarlisle avatar May 04 '20 08:05 davidcarlisle

Aha! That makes a lot of sense. The patch looks good to me.

The fact that an ERROR can be downgraded to a WARNING using a note also makes sense. Maybe we could also add a note to the error message to say some thing like "Some error conditions may be considered a warning if sufficiently explained in the upload notes"?

benzea avatar May 04 '20 08:05 benzea

Looks like we fixed but forgot to close ...

josephwright avatar May 06 '24 12:05 josephwright