-
-
Notifications
You must be signed in to change notification settings - Fork 594
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 bugzilla issue 24493: FreeBSD_14 version identifier missing #16368
Conversation
Thanks for your pull request, @jmdavis! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#16368" |
compiler/src/dmd/target.d
Outdated
@@ -206,6 +208,7 @@ void addPredefinedGlobalIdentifiers(const ref Target tgt) | |||
case 11: predef("FreeBSD_11"); break; | |||
case 12: predef("FreeBSD_12"); break; | |||
case 13: predef("FreeBSD_13"); break; | |||
case 14: predef("FreeBSD_14"); break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we use FreeBSD_%d
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be fine with me. I was just doing it the same way that it was already done. However, what would I use to format the string given that we can't use Phobos in the compiler code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
snprintf can do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. I figured that I'd ask in case there were some other way that something like that was typically done in the compiler, since I'm really not familiar with the dmd code base and what helper functions it has.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. It now uses snprintf
. One other difference in behavior though is that it no longer defaults to FreeBSD 11 if the major OS version hasn't been set (which seems to typically be set by defining one of the TARGET_FREEBSD* versions when compiling dmd, and with these changes, build.d will now set it with the version of the OS being built on if it's not explicitly set). Instead, it just doesn't set the version-specific FreeBSD specifier, which will result in an error in druntime, which will make it easier to catch when we're missing a new version.
Apparently, there were two problems that needed fixing. 1. The code for the FreeBSD_14 version identifier had not been added yet. 2. The build did nothing to detect the version of FreeBSD, which makes it very easy to build for the wrong version. The CI apparently sets the version - e.g. TARGET_FREEBSD13 - but unless one of those TARGET_FREEBSD* versions is set, the build defaults to FreeBSD 11, which isn't even supported any longer. So, this adds the code for the FreeBSD_14 identifier, and it makes it so that the build queries the OS version on FreeBSD if TARGET_FREEBSD* has not been set. So, anything that sets the TARGET_FREEBSD* version when building will control the target version as before, but if it's not set, then it will target whatever the current system is.
Apparently, there were two problems that needed fixing.
The code for the FreeBSD_14 version identifier had not been added yet.
The build did nothing to detect the version of FreeBSD, which makes it very easy to build for the wrong version. The CI apparently sets the version - e.g. TARGET_FREEBSD13 - but unless one of those TARGET_FREEBSD* versions is set, the build defaults to FreeBSD 11, which isn't even supported any longer.
So, this adds the code for the FreeBSD_14 identifier, and it makes it so that the build queries the OS version on FreeBSD if TARGET_FREEBSD* has not been set. So, anything that sets the TARGET_FREEBSD* version when building will control the target version as before, but if it's not set, then it will target whatever the current system is.