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

fix: treat any data change as a modification #2542

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

lukehsiao
Copy link
Contributor

@lukehsiao lukehsiao commented Jun 23, 2024

fix: treat any data change as a modification

For unknown reasons, it seems some environments emit a DataChange::Any event,
rather than specifying content or size, when a file is edited. As an example:

[src/fs_utils.rs:72:9] &event = DebouncedEvent {
    event: Event {
        kind: Modify(
            Data(
                Any,
            ),
        ),
        paths: [
            "/home/redacted/content/blog/2024-06-23_example.md",
        ],
        attr:tracker: None,
        attr:flag: None,
        attr:info: None,
        attr:source: None,
    },
    time: Instant {
        tv_sec: 78544,
        tv_nsec: 936740729,
    },
}

Consequently, because this isn't treated by Zola as a modification, the
site is not rebuilt, which regresses on previous behavior.

This patch fixes this particular case by treating any data modification
events as a modification.

Closes: #2536


IMPORTANT: Please do not create a Pull Request adding a new feature without discussing it first.

The place to discuss new features is the forum: https://zola.discourse.group/
If you want to add a new feature, please open a thread there first in the feature requests section.

Sanity check:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Code changes

(Delete or ignore this section for documentation changes)

  • Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

  • Have you created/updated the relevant documentation page(s)?

@lukehsiao lukehsiao force-pushed the next branch 2 times, most recently from 96a0f39 to 5a9b22f Compare June 23, 2024 19:43
Copy link
Collaborator

@Keats Keats left a comment

Choose a reason for hiding this comment

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

that looks ok to me, if other people can confirm if it fixes the issue for them

@lukehsiao lukehsiao changed the title fix: treat closed files accessed in write mode as modifications fix: treat any data change as a modification Jun 23, 2024
@lukehsiao
Copy link
Contributor Author

lukehsiao commented Jun 23, 2024

@apiraino, @Jieiku, @cart, @martin-t, can any of you confirm this fix in your environment?

If you want to install:

$ git clone --branch next --single-branch https://github.com/lukehsiao-forks/zola.git
$ cd zola
$ cargo install --path . --locked
$ zola --version

Or, just do the first 2 steps then cargo build and specify the full path to the executable e.g., ../path/to/your/clone/target/debug/zola serve

It fixes mine.

@Jieiku
Copy link
Contributor

Jieiku commented Jun 23, 2024

Fixed for me on Arch/EndeavourOS:

git clone https://github.com/lukehsiao-forks/zola/
cd zola
git checkout next
cargo build --release
chmod a+rx target/release/zola
cp target/release/zola ~/zola

2024-06-23_13-09-16

For unknown reasons, it seems some environments emit a `DataChange::Any` event,
rather than specifying content or size, when a file is edited. As an example:

    [src/fs_utils.rs:72:9] &event = DebouncedEvent {
        event: Event {
            kind: Modify(
                Data(
                    Any,
                ),
            ),
            paths: [
                "/home/redacted/content/blog/2024-06-23_example.md",
            ],
            attr:tracker: None,
            attr:flag: None,
            attr:info: None,
            attr:source: None,
        },
        time: Instant {
            tv_sec: 78544,
            tv_nsec: 936740729,
        },
    }

Consequently, because this isn't treated by Zola as a modification, the
site is not rebuilt, which regresses on previous behavior.

This patch fixes this particular case by treating any data modification
events as a modification.

Closes: getzola#2536
@lukehsiao
Copy link
Contributor Author

lukehsiao commented Jun 23, 2024

Ty for checking! I've made the PR a little more complete by also adding a modification event for Other, so now we literally trigger a modification event for "any" of Any, Size, Content, Other types of DataChange: https://docs.rs/notify/6.1.1/notify/event/enum.DataChange.html.

@@ -202,7 +209,7 @@ mod tests {
];
for (case, expected) in cases.iter() {
let ek = get_relevant_event_kind(&case);
assert_eq!(ek, *expected);
assert_eq!(ek, *expected, "case: {:?}", case);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was just useful when debugging to actually see the case that failed when the assertion failed.

@Jieiku
Copy link
Contributor

Jieiku commented Jun 23, 2024

I am headed out to do some work, but just let me know if you need anything else tested, and I will test it once I get back home.

@lukehsiao
Copy link
Contributor Author

lukehsiao commented Jun 23, 2024

@Keats looks like we have confirmation from someone else on a different OS than me (arch vs fedora).

@phisch
Copy link

phisch commented Jun 23, 2024

Noticed the same issue while working on my website. Came across the issue and this PR, and I can also confirm that this fixes the problem and changes are properly detected again!

Thanks for the quick fix!

Edit: In case it's useful, I'm on arch linux as well.

@iamorphen
Copy link
Contributor

Thank you for this! I implemented filtering to make sure we don't overreact to some super fine-grained events (ex: notify::event::MetadataKind::Permissions). Watching for DataChange::Any as well seems okay to me since it belongs to a relatively narrow category.

@SIGSTACKFAULT
Copy link
Contributor

SIGSTACKFAULT commented Jun 24, 2024

while working on #2507 i discovered this bug and fixed it locally (I think i fixed it in the exact same way as this MR, too) but assumed it was just a weird NixOS edge case or something and didn't think to report it :P

@Keats Keats merged commit 4983854 into getzola:next Jun 24, 2024
5 checks passed
Keats pushed a commit that referenced this pull request Jun 24, 2024
For unknown reasons, it seems some environments emit a `DataChange::Any` event,
rather than specifying content or size, when a file is edited. As an example:

    [src/fs_utils.rs:72:9] &event = DebouncedEvent {
        event: Event {
            kind: Modify(
                Data(
                    Any,
                ),
            ),
            paths: [
                "/home/redacted/content/blog/2024-06-23_example.md",
            ],
            attr:tracker: None,
            attr:flag: None,
            attr:info: None,
            attr:source: None,
        },
        time: Instant {
            tv_sec: 78544,
            tv_nsec: 936740729,
        },
    }

Consequently, because this isn't treated by Zola as a modification, the
site is not rebuilt, which regresses on previous behavior.

This patch fixes this particular case by treating any data modification
events as a modification.

Closes: #2536
berdandy pushed a commit to berdandy/azola that referenced this pull request Sep 17, 2024
For unknown reasons, it seems some environments emit a `DataChange::Any` event,
rather than specifying content or size, when a file is edited. As an example:

    [src/fs_utils.rs:72:9] &event = DebouncedEvent {
        event: Event {
            kind: Modify(
                Data(
                    Any,
                ),
            ),
            paths: [
                "/home/redacted/content/blog/2024-06-23_example.md",
            ],
            attr:tracker: None,
            attr:flag: None,
            attr:info: None,
            attr:source: None,
        },
        time: Instant {
            tv_sec: 78544,
            tv_nsec: 936740729,
        },
    }

Consequently, because this isn't treated by Zola as a modification, the
site is not rebuilt, which regresses on previous behavior.

This patch fixes this particular case by treating any data modification
events as a modification.

Closes: getzola#2536
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants