openzim / zim-tools

Various ZIM command line tools
https://download.openzim.org/release/zim-tools/
GNU General Public License v3.0
133 stars 35 forks source link

Don't check the existing of favicon by zimcheck #334

Closed pavel-karatsiuba closed 1 year ago

pavel-karatsiuba commented 1 year ago

The specification requires not creating the favicon inside zim file. Please remove checking of favicon by zimcheck.

Right now zimcheck returns the error: [ERROR] Favicon: Favicon is missing

veloman-yunkan commented 1 year ago

The specification requires not creating the favicon inside zim file.

@pavel-karatsiuba Will you please point to the exact location in the specification supporting your statement?

kelson42 commented 1 year ago

@pavel-karatsiuba Your bug description is not correct

pavel-karatsiuba commented 1 year ago

@kelson42 I read your comment and thought that favicon doesn't need anymore because of specification. https://github.com/openzim/mwoffliner/pull/1809#issuecomment-1464856983 What is wrong with my description? Can you please help me to fix it?

kelson42 commented 1 year ago

@pavel-karatsiuba A proper formulation is: If in the past -/Favicon was a mandatory entry. The very same content (PNG) is now called M/Illustration_48x48@1 and it is still mandatory. We need:

@veloman-yunkan We are currently in a middle of an effort to sanitize the ZIM metadata. This has become very necessary as we start to be really better in ZIM generation/management. We run this effort at all levels (specification, libzim, scrapers, zim-tools and other solutions).

Unfortunately the zim-tools are pretty much imperfect (considering this metadata) like the rest of the software stack. Unfortunatley we have scrapers (like MWoffliner) which rely on zimcheck in their CI. As a consequence, we need urgently to get zimcheck working fine first so we can then improve the scrapers.

But this might be less straight forward than first though. Actually we have a few tickets related to metadata and checking if they are correct, see for example #332 or #336. If we don't want to deal with an endless list of bugs/improvements to do around this we should probably do it right now. I would propose:

What do you think? Would you be able to quickly implement something like this so we can release 3.2.0 and use the latest zimcheck in MWoffliner CI?

veloman-yunkan commented 1 year ago

I would propose:

  • Have a proper table/list of Medata with their properties (like: name, mandatory, max length, value maching regex, ...)

  • This list could be used by zimcheck to... check...

  • This list could be used by zimwriterfs to verify the command line arguments

  • Woudl be easy to update

What do you think? Would you be able to quickly implement something like this so we can release 3.2.0 and use the latest zimcheck in MWoffliner CI?

@kelson42 I will work on it.