getpelican / pelican-themes

Themes for Pelican
https://getpelican.com/
2.12k stars 1.09k forks source link

Themes are failing to build for the new pelicanthemes.com domain #755

Closed ployt0 closed 8 months ago

ployt0 commented 1 year ago

50 themes are failing to build. I'd like to extend coverage to them:

themes_failing_to_build

Starting with these two:

is_undefined_for_i18n_users

Maybe the other faults are easier, but I've proposed a PR for this one. I couldn't automate installation of the i18n extension these themes demand, but I don't believe demonstrating extensions is in scope or required.

PennRobotics commented 1 year ago

Since all of the build error logs point to __init__.py:566, would it make sense to generate a full stack trace? Perhaps with traceback.TracebackException unless the log generator already supports printing more verbose errors? (contribute.rst suggests setting exc_info to True, or what happens if you set verbosity to logging.DEBUG or greater?)

Then if you see which of the argument conversions is causing 28 themes to fail rather than, someone new to the codebase (e.g. me) would have an easier time creating a pull request to lighten your workload.


(The build log is equally laconic:)

[08:12:39] INFO     using pelican: 4.8.0                                        
           INFO     using shot-scraper: shot-scraper, version 1.2               
           INFO     processing 128 themes...                                    
[08:12:40] INFO     successfully generated : aboutwilson                        
[08:12:41] ERROR    failed to generate     : alchemy                            
[08:12:42] ERROR    failed to generate     : apricot                            
[08:12:43] ERROR    failed to generate     : attila                             
           ERROR    failed to generate     : backdrop                           
[08:12:44] INFO     successfully generated : basic                              
[08:12:45] INFO     successfully generated : blue-penguin                 
ployt0 commented 1 year ago

It was a learning experience for me customising my local pull to generate more insights. It is very noisy to leave this in to run each time, on 50 failing cases. When passing becomes the expectation it would make more sense. I'm going to want to develop locally, not on Github Actions, so it isn't overly burdensome to leave in the line that does this.

Here's the stack trace for the brick theme:

Traceback (most recent call last):
  File "C:\Users\ployt0\PycharmProjects\pelican\pelican\__init__.py", line 565, in main
    pelican.run()
  File "C:\Users\ployt0\PycharmProjects\pelican\pelican\__init__.py", line 128, in run
    p.generate_output(writer)
  File "C:\Users\ployt0\PycharmProjects\pelican\pelican\generators.py", line 704, in generate_output
    self.generate_pages(writer)
  File "C:\Users\ployt0\PycharmProjects\pelican\pelican\generators.py", line 606, in generate_pages
    self.generate_articles(write)
  File "C:\Users\ployt0\PycharmProjects\pelican\pelican\generators.py", line 475, in generate_articles
    write(article.save_as, self.get_template(article.template),
  File "C:\Users\ployt0\PycharmProjects\pelican\pelican\writers.py", line 269, in write_file
    _write_file(template, localcontext, self.output_path, name,
  File "C:\Users\ployt0\PycharmProjects\pelican\pelican\writers.py", line 202, in _write_file
    output = template.render(localcontext)
  File "C:\Users\ployt0\PycharmProjects\pelican\venv\lib\site-packages\jinja2\environment.py", line 1301, in render
    self.environment.handle_exception()
  File "C:\Users\ployt0\PycharmProjects\pelican\venv\lib\site-packages\jinja2\environment.py", line 936, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "C:\Users\ployt0\PycharmProjects\pelican\_assets\bricks\templates\article.html", line 1, in top-level template code
    {% extends "base.html" %}
  File "C:\Users\ployt0\PycharmProjects\pelican\_assets\bricks\templates\base.html", line 10, in top-level template code
    {% block head %}
  File "C:\Users\ployt0\PycharmProjects\pelican\_assets\bricks\templates\base.html", line 26, in block 'head'
    <link href="{{ FEED_DOMAIN }}/{{ CATEGORY_FEED_ATOM|format(category.slug) }}" type="application/atom+xml" rel="alternate" title="{{ SITENAME }} Categories Atom Feed" />
  File "C:\Users\ployt0\PycharmProjects\pelican\venv\lib\site-packages\jinja2\filters.py", line 1002, in do_format
    return soft_str(value) % (kwargs or args)
TypeError: not all arguments converted during string formatting

I know you didn't ask for it, but for the record, this, line 570, is the line to get the trace:

print_traceback

avaris commented 1 year ago

but I don't believe demonstrating extensions is in scope or required.

As the person who coded the preview generation, this is precisely why I didn't "fix" these issues.

Some themes require plugins, some themes require certain variables to be defined and some are just broken and needs updates for the current pelican. I was not in the mood for hunting down these, nor did I think it was the job for this generation script to cater the needs for every theme. It was meant to mimic an "out of the box" experience.

PS: Argument error is from a (very old) breaking change introduced in pelican 4.0. See changelog for details.

ployt0 commented 1 year ago

but I don't believe demonstrating extensions is in scope or required.

As the person who coded the preview generation, this is precisely why I didn't "fix" these issues.

I can't tell if that is net praise or criticism for my PR. I don't suppose I will "fix" all the other issues, I just felt like opening an issue to give my PR more impetus.

I was interested in seeing pelican-bootstrap3 be run. I think it presents the most modern "bootstrap" theme. brick was just easy to type, and remember.

I know one theme, Flex, does accept the "_" sequence from i18n.

I wasn't sure what the job of the generator script was, other than to exercise the themes. I don't know what I was seeing but I wanted more of it.

If you know about "Argument error" I won't try to fix it.

avaris commented 1 year ago

I can't tell if that is net praise or criticism for my PR. I don't suppose I will "fix" all the other issues, I just felt like opening an issue to give my PR more impetus. ... I wasn't sure what the job of the generator script was, other than to exercise the themes. I don't know what I was seeing but I wanted more of it.

Sorry, I realized I probably sound a bit crass. It was not aimed at you. Thanks for looking into and working on it.

I just don't think it's the right way, hence the quotes around the "fix". Sure, this script can do a lot more, workaround some issues of themes or even monkey-patch them so that they "work". But... First, this is not sustainable or maintainable. If a new theme is added with different requirements, it would need to be added to the list of workarounds. Second, this probably highlights an issue in the theme and would better be handled there (e.g. all the argument errors), rather than employing band-aids in the generation script. Third, a bit of criticism for the way themes are handled in pelican, which I'd gladly own :). Requirements, configuration etc could probably be a more smooth experience.

PS: I would not be opposed to installing certain (common) plugins like assets (required in some themes) or i18n enablers for the themes (see my comment)

PennRobotics commented 1 year ago

Ooof, I see what you mean. Some of the themes are simply 5 to 10 years old. I turned on the full stack trace and tried fixing "apricot", but it's not just one "change format(slug) to format(slug=slug)" and done. This led to another error in pagination and then another error in date handling.

I think there's a most reliable way forward with least amount of work and without forcing maintainers to fix themes (at least for those without plugins). I'm not sure how to automate it but know it could be done: For each theme folder, look at the last commit date and then checkout pelican for that date. Unfortunately, this would probably require pip installing whatever version of every dependency was current. I imagine that would drastically lengthen builds (even if you sort by date).

If only I had more free time, I'd volunteer to fork any of the old themes (5 years or older), add a requirements.txt with pinned versions, and then archive right away to avoid inevitable issue/pull request maintainer hell.

Sorry, before I even got one pull request up, I'm already close to tapping out.

PennRobotics commented 1 year ago

By the way, for anyone new to this repo but wanting to help:

To get the full stack trace into failed.html, just add "-D", to the subprocess arguments when pelican is called in build-theme-previews.py. It can be more helpful to skip every theme except the one you're interested in and open the .html source itself, as the log output is sometimes interpreted as an HTML tag, which brings me to…

Speed up builds. Skip any themes with an existing output folder right after output_path is defined via:

        if os.path.isdir(output_path):
            logger.info(f"[yellow](skipped {theme})", extra={"markup": True})
            continue
ployt0 commented 1 year ago

My reading of this issue is that the output from build-theme-preview should be considered authoritative. Any theme failing to build is non-compliant, unmaintained, and should probably not be chosen for new projects.

The problem I had is that the themes are so old that pelican-bootstrap3 was the most modern sounding one, particularly considering the prominence it gives bootstrap in its name. It sounds like the theme to use for bootstrap in Pelican.

I don't know, has bootstrap fallen out of favour, or is what we have here a lot of old, unmaintained themes? Apologies if that comes across as rude.

The reason I wanted to change build-theme-preview is that it is recent, looked like it could use help getting off the ground, and the PR wouldn't just languish unacknowledged, like it would on every other unmaintained repo.

I thought I'd explained I couldn't install the i18n extension the PR offers a workaround for. No? I guess it got lost in my edits.

justinmayer commented 8 months ago

@avaris was kind enough in #766 to do the work needed to get six additional themes to build properly for the pelicanthemes.com site. If anyone would like to volunteer to fix some of the other themes that do not currently build properly, your contributions to that end would be most welcome.