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

Preprocessor errors corrupts cache #1924

Open
Vultraz opened this Issue Aug 28, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@Vultraz
Member

Vultraz commented Aug 28, 2017

See conversation below from IRC:

5:47 PM [zookeeper] Undefined macro in #ifver/#ifnver first argument: 'WESNOTH_VERSION' at ~add-ons/Dragon_Lair/_main.cfg:35
5:47 PM the line being just this: #ifver WESNOTH_VERSION >= 1.13.2
5:47 PM didn't do anything with my previous month-old build, but latest master complains about that
5:50 PM [shadowm] Force a cache refresh?
5:51 PM [zookeeper] huh. yeah, i deleted the cache dir, that helped.
5:52 PM [shadowm] It's a bug I've been observing since as far back as 1.9.x but I've never gotten around to properly documenting it in a bug report. If a WML read fails due to a parser or preprocessor error, the cache gets poisoned with an incomplete preprocessor configuration unit for that tree.

@Vultraz Vultraz added the Bug label Aug 28, 2017

@gfgtdf

This comment has been minimized.

Show comment
Hide comment
@gfgtdf

gfgtdf Aug 29, 2017

Contributor

I also see exactly this error message Undefined macro in #ifver/#ifnver first argument: 'WESNOTH_VERSION' quite often, not in the same addon of course.

Contributor

gfgtdf commented Aug 29, 2017

I also see exactly this error message Undefined macro in #ifver/#ifnver first argument: 'WESNOTH_VERSION' quite often, not in the same addon of course.

@Vultraz

This comment has been minimized.

Show comment
Hide comment
@Vultraz

Vultraz Sep 8, 2017

Member

I wonder if this call: https://github.com/wesnoth/wesnoth/blob/master/src/config_cache.cpp#L133-L134 should be wrapped in a try {} block. But I can't figure out how to reliably repro this so I can't test.

Member

Vultraz commented Sep 8, 2017

I wonder if this call: https://github.com/wesnoth/wesnoth/blob/master/src/config_cache.cpp#L133-L134 should be wrapped in a try {} block. But I can't figure out how to reliably repro this so I can't test.

@gfgtdf

This comment has been minimized.

Show comment
Hide comment
@gfgtdf

gfgtdf Oct 7, 2017

Contributor

possible dublicate #1634

Contributor

gfgtdf commented Oct 7, 2017

possible dublicate #1634

@singalen

This comment has been minimized.

Show comment
Hide comment
@singalen

singalen Nov 28, 2017

Contributor

Got it reproduced on iOS every time, with "AI Demos" addon.
Let me give it a shot!
iOS version crashes when a cache is corrupt, so it's particularly nasty one.

Contributor

singalen commented Nov 28, 2017

Got it reproduced on iOS every time, with "AI Demos" addon.
Let me give it a shot!
iOS version crashes when a cache is corrupt, so it's particularly nasty one.

@shikadiqueen shikadiqueen self-assigned this Jun 18, 2018

shikadiqueen added a commit that referenced this issue Jun 18, 2018

config/cache: Ensure built-in symbols like WESNOTH_VERSION are always…
… defined

Half-fix for issue #1634 and issue #1924.

The issue that remains to be solved is that on subsequent sessions after
a campaign has failed to load, it is possible for the game to generate a
cache entry for it that only contains the main menu WML for it.
Apparently the config cache transactions mechanism causes the game to
try to generate a cache entry with the wrong defines (a define set that
doesn't include the campaign's symbol, for instance) instead of the ones
that are actually needed and used to match the cache entry's filename
via checksumming. As a result, on subsequent sessions the failed
campaign is aborted with "failed to load the scenario" instead of
displaying the real WML error again (since the error is not hit again if
it depends on the campaign's symbol being defined).

In the meantime, this at least removes the red herring error and makes
the underlying issue a bit more visible. It's a very crude hack but it
does the job.

shikadiqueen added a commit that referenced this issue Jun 18, 2018

config/cache: Ensure built-in symbols like WESNOTH_VERSION are always…
… defined

Half-fix for issue #1634 and issue #1924.

The issue that remains to be solved is that on subsequent sessions after
a campaign has failed to load, it is possible for the game to generate a
cache entry for it that only contains the main menu WML for it.
Apparently the config cache transactions mechanism causes the game to
try to generate a cache entry with the wrong defines (a define set that
doesn't include the campaign's symbol, for instance) instead of the ones
that are actually needed and used to match the cache entry's filename
via checksumming. As a result, on subsequent sessions the failed
campaign is aborted with "failed to load the scenario" instead of
displaying the real WML error again (since the error is not hit again if
it depends on the campaign's symbol being defined).

In the meantime, this at least removes the red herring error and makes
the underlying issue a bit more visible. It's a very crude hack but it
does the job.

jostephd added a commit to jostephd/wesnoth that referenced this issue Oct 6, 2018

config/cache: Ensure built-in symbols like WESNOTH_VERSION are always…
… defined

Half-fix for issue wesnoth#1634 and issue wesnoth#1924.

The issue that remains to be solved is that on subsequent sessions after
a campaign has failed to load, it is possible for the game to generate a
cache entry for it that only contains the main menu WML for it.
Apparently the config cache transactions mechanism causes the game to
try to generate a cache entry with the wrong defines (a define set that
doesn't include the campaign's symbol, for instance) instead of the ones
that are actually needed and used to match the cache entry's filename
via checksumming. As a result, on subsequent sessions the failed
campaign is aborted with "failed to load the scenario" instead of
displaying the real WML error again (since the error is not hit again if
it depends on the campaign's symbol being defined).

In the meantime, this at least removes the red herring error and makes
the underlying issue a bit more visible. It's a very crude hack but it
does the job.

jostephd pushed a commit to jostephd/wesnoth that referenced this issue Oct 7, 2018

config/cache: Ensure built-in symbols like WESNOTH_VERSION are always…
… defined

Half-fix for issue wesnoth#1634 and issue wesnoth#1924.

The issue that remains to be solved is that on subsequent sessions after
a campaign has failed to load, it is possible for the game to generate a
cache entry for it that only contains the main menu WML for it.
Apparently the config cache transactions mechanism causes the game to
try to generate a cache entry with the wrong defines (a define set that
doesn't include the campaign's symbol, for instance) instead of the ones
that are actually needed and used to match the cache entry's filename
via checksumming. As a result, on subsequent sessions the failed
campaign is aborted with "failed to load the scenario" instead of
displaying the real WML error again (since the error is not hit again if
it depends on the campaign's symbol being defined).

In the meantime, this at least removes the red herring error and makes
the underlying issue a bit more visible. It's a very crude hack but it
does the job.

(cherry-picked from commit 0c2298d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment