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

Check cache fc_version.h at configure time #1638

Merged
merged 3 commits into from
Jan 1, 2023

Conversation

jwrober
Copy link
Collaborator

@jwrober jwrober commented Dec 27, 2022

For devs who do not clean their cache directory often, we do a check for the generated fc_version.h and if the patch version is different then we delete the cache file. Closes #1564

@lmoureaux
Copy link
Contributor

I'm afraid that this will trigger a full rebuild after every local commit, which is definitely too much, but maybe I misremember what we put in the version string?

@jwrober
Copy link
Collaborator Author

jwrober commented Dec 27, 2022

I'm afraid that this will trigger a full rebuild after every local commit, which is definitely too much, but maybe I misremember what we put in the version string?

I didn't test that scenario. Let me see what happens.

@jwrober
Copy link
Collaborator Author

jwrober commented Dec 28, 2022

It works while you are working on a bunch of local changes. Multiple commits are fine in a branch while you are working. You might run into an issue if you switch to a different branch that has a diff hash from master branch, that will force a "mostly" recompile, but not a total recompile. The code is essentially getting the current hash turned into a integer from the last pull, so based on what that is in the branch you are working on would cause a recompile. I think this better than ignoring it altogether like we did in the past. One recompile with a fresh git pull is not horrible.

@lmoureaux
Copy link
Contributor

You might run into an issue if you switch to a different branch that has a diff hash from master branch, that will force a "mostly" recompile, but not a total recompile.

I switch between branches all the time, so this will slow me down significantly :( I think we keep this out for now and revisit after 3.0 is out, moving the info to a file that gets compiled exactly once (and fast).

@jwrober
Copy link
Collaborator Author

jwrober commented Dec 28, 2022

Yea I was thinking something about that too. I wonder if it would be better to check the tag and do the removal of the cached file when a new tag is done. That would be a more rare event and allow branch switching.

Copy link
Contributor

@lmoureaux lmoureaux left a comment

Choose a reason for hiding this comment

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

Just to prevent immediate merge

For devs who do not clean their cache directory often, we do a check for the generated fc_version.h and if the patch version is different
then we delete the cache file. Closes longturn#1564
lmoureaux added a commit to jwrober/freeciv21 that referenced this pull request Jan 1, 2023
This makes for a simpler implementation and is also more general: a change in
the network capstring will also trigger a rebuild of the relevant parts of the
code.

See longturn#1638.
Copy link
Contributor

@lmoureaux lmoureaux left a comment

Choose a reason for hiding this comment

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

Fixed my concerns and rebased for the new CI checks

Instead of including in every single source file through fc_config.h,
selectively include it where it's needed. Changing the version used to trigger
a full rebuild, now only two dozens files are rebuilt.

See longturn#1564.
This makes for a simpler implementation and is also more general: a change in
the network capstring will also trigger a rebuild of the relevant parts of the
code.

See longturn#1638.
@jwrober jwrober enabled auto-merge (rebase) January 1, 2023 16:56
@jwrober jwrober merged commit 22988ac into longturn:master Jan 1, 2023
@jwrober jwrober deleted the bugfix/version-check branch January 8, 2023 02:12
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.

fc_version.h not updated automatically
2 participants