Skip to content

CMake: Remove starting . from GODOTCPP_SUFFIX #1814

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

Merged
merged 1 commit into from
Aug 21, 2025

Conversation

enetheru
Copy link
Collaborator

@enetheru enetheru commented Jul 8, 2025

This is a tiny but kinda annoying issue I want to fix.

During the configuration phase to get the binary suffix(eg windows.editor.x86_64) cmake builds up a generator expression out of the relevant pieces, and that expression is put into GODOTCPP_SUFFIX. Which is analagous enough to how scons builds the binary suffix, it so that part is all good.

As it is currently, the first entry in the expression starts with a prepending . so that later when its used ike ${PROJECT_NAME}${GODOT_SUFFIX} the resulting binary looks like godot-cpp.linux.template_release.x86_64.so

This GODOTCPP_SUFFIX varible is then attached to the godot-cpp target so that downstream projects can fetch it and use it for themselves, avoiding having to build up the suffix from the pieces themselves manually which can be a pain.

So today I was wanting to use this suffix, but it comes with a leading ., and that's just not expected, and I think very rarely desired, considering all other use cases when getting parts of a whole, the delimiter isnt usually part of the resulting piece. ie a hypothetical function get_extension wouldn't provide .cpp, it would just provide cpp

So I want to remove the leading ., making the libname ~${PROJECT_NAME}.${GODOT_SUFFIX}.

The problem arises that it will effect consumer build artifacts if they use the GODOTCPP_SUFFIX variable, like I do, to name their libraries and configure related files. So if this change is accepted, I think it would need notice somehow provided to users, or fit into a specific release cycle of some sort. I'm not super familiar with release processes.

Cheers,
Samuel.

@enetheru enetheru requested a review from a team as a code owner July 8, 2025 06:15
@Calinou Calinou added the bug This has been identified as a bug label Jul 8, 2025
@dsnopek
Copy link
Collaborator

dsnopek commented Jul 10, 2025

The problem arises that it will effect consumer build artifacts if they use the GODOTCPP_SUFFIX variable, like I do, to name their libraries and configure related files. So if this change is accepted, I think it would need notice somehow provided to users, or fit into a specific release cycle of some sort.

Would it make sense to add a new variable without the dot, rather than removing the dot from the existing variable? The old variable could be set in terms of the new one.

I don't know how weird or annoying that would be in a CMake context, but after the big break, it'd be nice to not do breaks that don't solve important problems

@dsnopek dsnopek added this to the 4.x milestone Jul 10, 2025
@dsnopek dsnopek added cmake topic:buildsystem Related to the buildsystem or CI setup labels Jul 10, 2025
@enetheru
Copy link
Collaborator Author

Would it make sense to add a new variable without the dot, rather than removing the dot from the existing variable? The old variable could be set in terms of the new one.

That's entirely possible to do, and satisifies the possible breakage, GODOTCPP_SUFFIX2?
No matter what I name it I'm going to be unsatisfied, whats the godot convention for compatibility things like this?

In the future, would a major version be able to remove the old name? or will my mistakes live forever?
If its unlikely to change in the future I would want to use a name that at least makes sense.

  • GODOTCPP_OUTPUT_SUFFIX?
  • GODOTCPP_NAME_SUFFIX?
  • GODOTCPP_LIB_SUFFIX?

I just read the scons code, does it also include the leading '.' ? It seems that it does from a casual glance, so perhaps there was reason in my orginal process.

Apologies if I'm a bit wordy on this, pre-emptively dealing with consequences for changes that may break code is new to me, and more talking seems the best approach to build confidence, even for such a minor thing.

@enetheru
Copy link
Collaborator Author

A little bit of bike shedding can be fun, but not too much.

  • GODOTCPP_SUFFIX_TRIMMED?

This is the same as GODOTCPP_SUFFIX but without the leading '.'
GODOTCPP_SUFFIX is  then based on the above.
@dsnopek
Copy link
Collaborator

dsnopek commented Aug 20, 2025

That's entirely possible to do, and satisifies the possible breakage, GODOTCPP_SUFFIX2?
No matter what I name it I'm going to be unsatisfied, whats the godot convention for compatibility things like this?

I don't think we have a naming convention with regard to build system changes. In the past, with renamed variables in SCons, I think we've usually ended up with totally different names (not an incrementing number) and then an "alias" to the original version

I just read the scons code, does it also include the leading '.' ?

Yeah, the scons configuration seems to include the leading .

  • GODOTCPP_SUFFIX_TRIMMED?

This would be fine with me personally.

In the latest version of the PR it's using GODOTCPP_SUFFIX_GENEX - what's the reasoning behind GENEX? It's not immediately clear what that stands for. If it's something that would be clear in the context of CMake, then it's fine. Otherwise, something that uses full words (like GODOTCPP_SUFFIX_TRIMMED) would be my personal preference

@enetheru
Copy link
Collaborator Author

In the latest version of the PR it's using GODOTCPP_SUFFIX_GENEX - what's the reasoning behind GENEX?

Removing the . from the suffix post hoc was so much more painful than omitting it from the generation at first, so GENEX is short for generator expression and is the expression pre-. from which GODOT_CPP_SUFFIX is then created.

@dsnopek
Copy link
Collaborator

dsnopek commented Aug 21, 2025

Are developers going to interact with this variable? If so, I still think it would be better to use a normal, descriptive word, rather than an abbreviation like GENEX.

I don't think it matters so much how the variable was derived - I think we could still call it GODOTCPP_SUFFIX_TRIMMED even if it wasn't created via trimming, but there's plenty of other options that don't evoke trimming like GODOTCPP_SUFFIX_BASE or similar

All that said, I don't want to bike shed this too much and hold up progress. :-) If you think we should move forward with the current PR, I'd be OK with that

@enetheru
Copy link
Collaborator Author

All that said, I don't want to bike shed this too much and hold up progress. :-) If you think we should move forward with the current PR, I'd be OK with that

Thats what I ended up feeling originally :), lets move forward with the current.

@dsnopek dsnopek merged commit 449e37f into godotengine:master Aug 21, 2025
10 checks passed
@dsnopek
Copy link
Collaborator

dsnopek commented Aug 21, 2025

Cherry-picked for 4.4 in PR #1836

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This has been identified as a bug cmake topic:buildsystem Related to the buildsystem or CI setup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants