Skip to content
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.

Sign up for GitHub

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

Patch the baseline test new-year race condition. Fix #3723. #3724

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Patch the baseline test new-year race condition. Fix #3723. #3724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
tmp/
cache/
output/
baseline/
nikola-baseline-build/
/tags
v3.*.zip.*

# GitHub token
.pypt/gh-token
Expand Down
37 changes: 35 additions & 2 deletions scripts/baseline.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
#!/bin/bash

set -e -o pipefail -x

PYVER=$(scripts/getpyver.py short)
if [[ "$1" == "check" ]]; then
echo -e "\033[36m>> Downloading baseline for $PYVER...\033[0m"
Expand All @@ -19,8 +22,38 @@ rm "pages/creating-a-theme.rst" "pages/extending.rst" "pages/internals.rst" "pag
LC_ALL='en_US.UTF-8' PYTHONHASHSEED=0 nikola build --invariant
if [[ "$1" == "check" ]]; then
echo -e "\033[36m>> Testing baseline...\033[0m"
diff -ubwr ../baseline output
if [[ $? == 0 ]]; then
python3 -c '
# In-place edit of copyright notes to adjust the copyright year.
import time
YEAR = str(time.gmtime().tm_year)
for edit_me in [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn’t hardcode a list of files, it may change and cause the code to crash.

I would also prefer to use sed (with a slightly longer fragment to be sure we’re replacing the right thing) instead of doing it in Python via string manipulation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hard-coded list should or at least could be fixed. I agree to that.

Probably, the whole "patch" approach is not ideal.

What would you think about this idea: When the --invariant switch is given, the year should not be pulled from time.gmtime().tm_year, but instead some fixed number is used.

But then: Do you think this is worth it in the grand scheme of things?

It is sort of a reflex of mine to try to remove sources of change. My way of doing things: If Nikola version x.y.z once passed all the tests, I tend to fight hard so it will continue to pass all tests forever.

But that is only my preference. It does not seem to be this project's philosophy. For one, you do not pin dependencies, not even to major numbers.

So, what do you think? Is #3723 worth being fixed?

You seem to suggest, it's not.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the solution with --invariant would be a good solution to #3723 and one I would accept. This could be done by exposing a function somewhere in Nikola (utils.py?) and importing it in the sample conf.py file.

For one, you do not pin dependencies, not even to major numbers.

Pinning dependencies makes things difficult for downstream distributors, and there are a handful. We generally don’t support versions beyond the latest, and we don’t run tests for things other than the master branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense.

TMI, in case you are interested: Now retired, I lived through the age of big enterprise software some 20 years ago, with a new production release every three months only, on a schedule. That official picture was only half-true in reality: We had production bug fixes much more frequently. In that world, it was very important that all automated tests that you ran at official release time would still pass at bug fix time 10 weeks later. The bug-fix build should only change the handful lines of sources we consciously had changed. You bet we fixed dependencies with all the nails and glue we could get our hands on! - 'twas a different world from what Nikola is facing today.

"../baseline/rss.xml",
"../baseline/index.html",
"../baseline/galleries/index.html",
"../baseline/galleries/demo/index.html",
"../baseline/listings/index.html",
"../baseline/listings/hello.py.html",
"../baseline/listings/__pycache__/index.html",
"../baseline/pages/about-nikola/index.html",
"../baseline/pages/bootstrap-demo/index.html",
"../baseline/pages/dr-nikolas-vendetta/index.html",
"../baseline/pages/listings-demo/index.html",
"../baseline/pages/quickref/index.html",
"../baseline/pages/quickstart/index.html",
"../baseline/posts/welcome-to-nikola/index.html",
]:
with open(edit_me, "rt+") as rssf:
rss = rssf.read()
copyright_prelude = "Contents © "
copyright_prelude_position = rss.find(copyright_prelude)
if -1 == copyright_prelude_position:
raise RuntimeError(f"Could not find copyright note in {edit_me}")
copyright_position = copyright_prelude_position + len(copyright_prelude)
new_rss = rss[:copyright_position] + YEAR + rss[copyright_position+4:]
rssf.seek(0, 0)
rssf.write(new_rss)
'
if diff -ubwr ../baseline output; then
echo -e "\033[32;1m>> Baseline test successful\033[0m"
else
CODE=$?
Expand Down