-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add alternate container engine support to Justfile #7374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Added the unstable `script` attribute to the `optimize-email` recipe. This should allow compatibility with WSL environments where shebang recipes struggle due to `noexec` directories.
Removed the script to generate the optimized email (`create_optimized_email.sh`) since it got inlined into the `justfile`
|
Thank you so much! I will review this this weekend! |
|
My guess is that you need a newer version of
|
|
Looks like the |
|
Good catch! I'm going to check with that maintainer to see whether that removal was intentional. Also I apologize for taking so long to review this, life got busy, but I will get it done soon. |
|
that was indeed an accident, it's back now! |
Glad I could help!
Truly, it's no problem! This is a quality-of-life PR more than anything, it kinda defeats the purpose if it makes your life more difficult. I imagine that when working on a weekly-release publication, it's tough to make time for something that doesn't contribute to the next release. Whenever it gets reviewed is great, it's not urgent. |
|
This overall looks good to me. @tzilist, since you are using Podman, would you be interested in testing this one out? |
Updated the Building section of the README to include information about using Docker alternatives. A couple other parts of the Building section also seemed outdated or slightly misleading, so I updated those as well (such as the section about viewing generated newsletter formats).
|
I realized that the "Building" section of the README should also be updated to include instructions about using Docker alternatives, so I added a commit updating that section with instructions using Podman as an example alternative. As I was updating the "Building" section, I noticed a couple other bits that seemed outdated or slightly misleading, so I ended up updating those as well, though that was perhaps outside the scope of this PR. If it's preferred, I could move that to its own PR. |
|
Ty so much! I do believe that is in the scope of this PR - I've designated some time tomorrow to do a review of this :) |
|
@chris-t-jansen most of this seems to be working thankfully! Unfortunately, when i run Would you mind taking a look when you have a sec? Thank you for all of your work on this so far, this is quite nice! |
|
@tzilist That's interesting! And unfortunate, given that those are kind of the core functions of the
DEFAULT_LANG = u'en'
'''
FEED_DOMAIN = SITEURL
FEED_ALL_ATOM = 'atom.xml'
FEED_ALL_RSS = 'rss.xml'
FEED_MAX_ITEMS = 4
CATEGORY_FEED_ATOM = 'categories/{slug}/atom.xml'
CATEGORY_FEED_RSS = 'categories/{slug}/rss.xml'
RSS_FEED_SUMMARY_ONLY = False
'''
DEFAULT_PAGINATION = 10
CMD ["pelican", "--delete-output-directory", "--verbose", "content"]Apologies for the laundry-list of requests! Hopefully it sheds some light on what's going on, because after that my next best idea is to try bumping the Pelican version to see if it's something that got fixed later, but that's more of a hopeful solution than a logical one. |

Overview
PR for #7324. Adds alternate container engine support to the
justfilefor TWiR. This PR was written with Podman in mind and was tested with that, but should work with any drop-in Docker replacement. Use it with any recipe that previously called Docker like so:For example:
Warning
Though I feel that what I've written is sound, additionally testing is immensely appreciated just to be sure, especially by the TWiR editors since they are the people who will use it the most.
Docker by default
Docker remains the default container engine, so passing in no arguments (i.e.
just website,just email) will use Docker as it always has. This is achieved by having Docker set as a global Just variable here:Which then gets used as the default value for the newly added
container-engineparameter on any recipe that used to calldocker. For example:Cons
A refactor like this usually comes with a downside or two, and this one is no exception. The obvious one here is that the
justfileis fairly more verbose, with the inclusion of parameters and default values for most recipes. This makes it less easily readable, which may hurt maintainability in the long run.The other downside has to do with the
optimize-emailrecipe. This recipe originally just called a shell script (create_optimized_email.sh), which itself explicitly calleddocker. To make it engine-agnostic, I inlined the script into thejustfileas a script recipe. Originally I implemented this as a shebang recipe, but due tonoexecissues on WSL (see here and here), I had to opt for the unstable[script(COMMAND)]attribute instead. The defaultCOMMANDofsh -eudidn't work for me in testing on WSL, so I opted forbashinstead. This could cause issues with portability on machines that don't include or symlink abashexecutable, but I had no trouble when testing it on my Windows machine with WSL and with my macOS machine.The
optimize-emailrecipe issue could also be resolved by refactoring the original script to support command line arguments (e.g.create_optimized_email.sh -c podman), but keeping things in thejustfileseemed a more elegant solution to me.Concerns
If you have any questions, comments, concerns, etc., feel free to let me know. As I said in the original issue I opened, I think this kind of compatibility support is important, but totally understand if the TWiR editors aren't interested in it.