search

xproc / 3.0-steps

Repository for change requests to the standard step library and for official extension steps
10 stars 7 forks source link

Is more than one archive document allowed on p:archive? #295

Closed ndw closed 4 years ago

ndw commented 4 years ago

The archive port (like the manifest port) is a sequence because empty is allowed. The description of the manifest port says explicitly that it's an error if more than one manifest document is provided. The description of archive doesn't. Should it? If not, what does it mean to provide several archives?

gimsieke commented 4 years ago

There might be an operation to merge archives, therefore I wouldn’t make multiple archives on the archive port an error unconditionally.

ndw commented 4 years ago

Ah, fair point. But for ZIP file operations, we should raise an error, agreed?

gimsieke commented 4 years ago

It’s certainly an uncommon use case for zip archives, but apparently they can be merged efficiently. So if some implementation chooses to provide a zip merge, we maybe shouldn’t make it an error. On the other hand, we shouldn’t require applications to support merge. We currently limit the allowed zip operations to update, create, freshen, and delete. For these operations, we say that there must be zero or one (exactly one for delete) documents on the archive port. In the interest of finishing the steps, I would leave it as it is now (that is, do not allow other zip operations). If your question aims at whether we should define new error codes for these four zip operations when there are too many documents on the archive port: I think we should. But we probably shouldn’t make it an error if a pipeline author uses an operation on zip files that is not contained in the list above. This way, implementations can provide their own operations without overtly violating the spec.

ndw commented 4 years ago

In as much as create and freshen say that they behave exactly like update, I suppose the spec says there can be at most one archive, but I think that could be clarified.

Proposal:

  1. Clarify that at most one ZIP file is allowed for the zip commands we identify.
  2. Add an explicit error for the case where more than one zip is supplied.
  3. Since I'll be touching the spec, add a note that says additional commands are implementation-defined
xml-project commented 4 years ago

What is wrong with XC0080? Obviously I missed the point because I thought this error is just for the case discussed.

gimsieke commented 4 years ago

It is, but I think Norm wants to be more specific if the format is zip.

Is it allowed for a processor to add explanatory text to error messages? Then it can raise C0080 and add: For Zip archives, the number of archives is exactly one for the delete command, and zero or one for the create, update, and freshen commands.

xml-project commented 4 years ago

Is it allowed for a processor to add explanatory text to error messages?

Why not?

Then it can raise C0080 and add: For Zip archives, the number of archives is exactly one for the delete command, and zero or one for the create, update, and freshen commands.

This is exactly what I implemented.

ndw commented 4 years ago

I think XC0080 is just fine. I may simply have overlooked that before I opened this issue. I still think it's a bit odd that delete says only one ZIP archive may appear when, AFAICT, only one may appear for any of the zip commands.

So I still think points 1 and 3 from my third comment hold.

ndw commented 4 years ago

Fixed by my fix for #294, I think.