OpenPrinting / cups

OpenPrinting CUPS Sources
https://openprinting.github.io/cups
Apache License 2.0
1k stars 178 forks source link

cups/ppd-cache.c: Add cupsUrfSupported to generated PPD (see #804) #805

Closed alexpevzner closed 9 months ago

alexpevzner commented 10 months ago

It fixes driverless printing on Pantum BM5100ADW Series and, probably, on many other devices.

See #804 for details.

michaelrsweet commented 10 months ago

I don't have a hard objection to this, but the bug is in cups-filters - the ColorModel option code, when executed, specified sGray as the colorspace. And the PPD specifies ColorDevice: False...

alexpevzner commented 10 months ago

Hi!

If there is no serious objections against this PR, what can I do to get this PR merged?

Technically, I can press the Merge button by myself, but not sure it will be OK if I will do so...

P.S. I've created the corresponding issue in cups-filters (https://github.com/OpenPrinting/libcupsfilters/issues/38) and plan to spend some time investigating it, but anyway I'd like this PR to be merged as a hotfix for the really existing problem

zdohnal commented 10 months ago

TBH I would pursue the fix in libcupsfilters as well - PPD generated by CUPS does not define any color support, so it is really filter bug if the color is generated. I can put this as downstream patch in Fedora for now, but I would like to have a fix in proper place in upstream projects...

alexpevzner commented 10 months ago

I completely agree that the real problem is in the libcupsfilters. So my position is following:

If everybody agrees, somebody needs to push the "Merge" button. Should I do it by myself? ;)

alexpevzner commented 10 months ago

Ping

@tillkamppeter, WDYT?

tillkamppeter commented 10 months ago

@alexpevzner in both cases, hotfix on CUPS or real fix on libcupsfilters, distros have tpo issue a bug fix update on one package, so for distros it is not a big difference what to do right now, in the hot fix case, distros will later on have to update again (but this they can do on a distro release, no stable release update needed) to switch to the real fix and remove the hotfix. Doing the real fix is the easier and more straightforward solution for the distros.

A hotfix would be the right way if we have no idea where and how to do the real fix, or we discover we have to change the complete architecture of the package to fix the bug. This is not the case, bugs like this happened often in libcupsfilters, and they always turned out to be fixed with a small change. Therefore, @alexpevzner I recommend you to continue your investigation on the filter functions and once found a solution we will do stable releasse updates of libcupsfilters in the distros.

Also CUPS-generated PPD files describe the discovered printers correctly, yours actually as monochrome printer, with only monochrome color modes (in the ColorModel option), so filters should consider it as such.

alexpevzner commented 10 months ago

Hi @tillkamppeter,

I'd prefer to leave this hotfix in cups even when cups-filters will be fixed and not to remove it from there.

It passes a piece of information, the urf-supported attribute content from IPP to PPD and it is consistent with PPDs, generated by the driverless command (i.e., by libcups + libppd, as fas as I can guess).

jsmeix commented 10 months ago

A generic side note only FYI (from me who knows nothing about the implementation details here):

In general I think it depends if what is called a "hotfix" is really only a makeshift at a wrong place that should be removed when the proper fix is applied at the right place to keep the CUPS code clean.

But when what is called a "hotfix" makes CUPS behave more robust in general, then it could stay in CUPS.

Additionally I am wondering when this issue gets fixed only in cups-filters if it is really impossible that an issue like this may also happen without cups-filters (e.g. when some other filtering software is used for CUPS) so that it might be in general good when GUPS itself behaves more robust against such kind of issues?

alexpevzner commented 10 months ago

Hi @jsmeix,

But when what is called a "hotfix" makes CUPS behave more robust in general, then it could stay in CUPS.

This patch exposes content of the urf-supported IPP printer attribute into the generated PPD. Taking in account that cups-filters later reconstruct IPP printer attributes from the PPD, this patch makes sense so stay forever in the CUPS, in order not to loose information during the double conversion (IPP->PPD->IPP).

And it is consistent with PPDs, generated by driverless cat

Please note, I will continue investigation of the cups filters and hope to fix it soon, but regardless of that I'd prefer to see this patch merged too.

alexpevzner commented 9 months ago

@michaelrsweet, @tillkamppeter,

could you please review when time allows?

alexpevzner commented 9 months ago

Thanks!