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.

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

Problems with Page.source_filename, Page.iter_source_filenames with alternatives are enabled #959

Closed
wants to merge 10 commits into from

Conversation

dairiki
Copy link
Contributor

@dairiki dairiki commented Dec 13, 2021

In playing around to look at another issue, I noticed that when building the example project, the blog pages are always rebuilt, even if they have not been changed.

I.e.

$ cd example
$ lektor build
Started build
U de/index.html
[... lots of other pages ...]
U static/css/nix.css
Finished build in 1.07 sec
Started prune
Finished prune in 0.00 sec

Then, do it again. Since no source has been changed, nothing should be rebuilt. But:

$ lektor build
Started build
U de/blog/blog-post-5/index.html
U de/blog/blog-post-4/index.html
U de/blog/blog-post-3/index.html
U de/blog/blog-post-2/index.html
U de/blog/first-post/index.html
U blog/blog-post-5/index.html
U blog/blog-post-4/index.html
U blog/blog-post-3/index.html
U blog/blog-post-2/index.html
U blog/first-post/index.html
U fr/blog/blog-post-5/index.html
U fr/blog/blog-post-4/index.html
U fr/blog/blog-post-3/index.html
U fr/blog/blog-post-2/index.html
U fr/blog/first-post/index.html
Finished build in 0.32 sec
Started prune
Finished prune in 0.01 sec

The blog posts are always rebuilt.

Issue(s) Resolved

I believe the root cause of this is that Page.source_filename is, in some cases, returning bogus values — specifically
the name of non-existing files. E.g. if only a contents.lr exists, .source_filename still returns contents+de.lr for the alt="de" version of the page. Since this is used for dependency tracking, things break if this points to a non-existing file.

Looking further, I think that SourceObject.source_filename is, or should be deprecated in favor of SourceObject.iter_source_filenames. Again, since this is used for dependency tracking, we usually (always?) care about all the source files, not just the "primary" one.

Related Issues / Links

Not sure.

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Link to corresponding documentation pull request for getlektor.com

@dairiki dairiki force-pushed the bug.siblings-dependency-tracking branch 2 times, most recently from 4d7a523 to 2752049 Compare December 13, 2021 04:01
When alts are enabled, Page.source_file was being generated based on
the page.alt.  This is not correct, since the alt-specific file may
not exists.  E.g. if there is only a `contents.lr`, for `alt="en"`
`source_file` was returning the non-existent file `contents+en.lr`.

In general, `.source_filename` and `.iter_source_filenames` should
return only extant source filenames.

Dependency tracking for the blog pages of the example site is not
working.  They get rebuilt every build.  This has something to do with
the `Siblings` virtual source object for the blog pages not listing
the correct source filenames due to the issue with Page.source_file
described above.
@dairiki dairiki force-pushed the bug.siblings-dependency-tracking branch from 2752049 to 720cd74 Compare December 13, 2021 16:50
@dairiki
Copy link
Contributor Author

dairiki commented Mar 2, 2022

Some of the assumptions I made in this PR are incorrect.

Primarily: I now believe that SourceObject.iter_source_filenames should list all files which are checked for when reading the source — whether or not each of those files actually exists. When I wrote this PR, I was thinking that iter_source_filenames should list only those files that actually exist.

Closing in favor of #1007

@dairiki dairiki closed this Mar 2, 2022
dairiki added a commit to dairiki/lektor that referenced this pull request Feb 27, 2023
This exercises the issue with dependencies on VirtualSourceObjects
when alts are enable that is described in lektor#1007 and lektor#959.
@dairiki dairiki deleted the bug.siblings-dependency-tracking branch March 3, 2023 21:16
dairiki added a commit that referenced this pull request Mar 8, 2023
* test(builder): add test for virtual source dependency tracking issue

This exercises the issue with dependencies on VirtualSourceObjects
when alts are enable that is described in #1007 and #959.

* The identity of VirtualSourceObjects depends on their alt.

* refactor: make VirtualSourceInfo a dataclass

* refactor: add is_changed method to FileInfo and VirtualSourceInfo

* test: exclude pure virtual methods from coverage testing
dairiki added a commit to dairiki/lektor that referenced this pull request Sep 11, 2023
* test(builder): add test for virtual source dependency tracking issue

This exercises the issue with dependencies on VirtualSourceObjects
when alts are enable that is described in lektor#1007 and lektor#959.

* The identity of VirtualSourceObjects depends on their alt.

* refactor: make VirtualSourceInfo a dataclass

* refactor: add is_changed method to FileInfo and VirtualSourceInfo

* test: exclude pure virtual methods from coverage testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant