Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Issue 17596: Version bindings so that they work for both FreeBSD 11 and 12 #3271

Merged
merged 5 commits into from Nov 24, 2020

Conversation

ibuclaw
Copy link
Member

@ibuclaw ibuclaw commented Nov 13, 2020

Rebase of #2280, with __FreeBSD_version nits.

Haven't yet tested the second commit against gdc/freebsd. But the first passes the testsuite and all unittests.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @ibuclaw!

Bugzilla references

Auto-close Bugzilla Severity Description
17596 blocker dmd d 2.073.2 and 2.074.1 interim generated dmd segfaults on FreeBSD 12-CURRENT

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + druntime#3271"

@ibuclaw ibuclaw force-pushed the freebsd/12 branch 2 times, most recently from 725a0c1 to 37805b5 Compare November 13, 2020 17:30
@kinke
Copy link
Contributor

kinke commented Nov 13, 2020

I like core.sys.freebsd.config. 👍

The existing symvers, i.e., custom mangle strings would need to be adapted too - either by simply getting rid of them (#3270, but not restricted to version (GNU)) or using the according versions for version (FreeBSD_11) etc. (non-GDC only), in the hopes that the latter makes programs compiled on a FBSD 11 host also run on FBSD 12+.

@ibuclaw
Copy link
Member Author

ibuclaw commented Nov 13, 2020

I like core.sys.freebsd.config. 👍

The existing symvers, i.e., custom mangle strings would need to be adapted too - either by simply getting rid of them (#3270, but not restricted to version (GNU)) or using the according versions for version (FreeBSD_11) etc. (non-GDC only), in the hopes that the latter makes programs compiled on a FBSD 11 host also run on FBSD 12+.

I guess that would be for later, when dmd catches up perhaps.

@kinke
Copy link
Contributor

kinke commented Nov 13, 2020

Nope - as it is here, pre-inode64 functions expecting the old structs are used with LDC too, regardless of whether FreeBSD_12 is defined.

@ibuclaw
Copy link
Member Author

ibuclaw commented Nov 15, 2020

I can deal with that in #3270, so as not to add conflicts between the two. Though you may have so elaborate how you want it to look like, as I guess either the versioned symbol should be explicit for all versions of FreeBSD, or latest should not have any special mangling.

@kinke
Copy link
Contributor

kinke commented Nov 18, 2020

Though you may have so elaborate how you want it to look like

I'm not really suited, the last time I've shortly played with FreeBSD was something like a decade ago. ;) - In case the regular symbols are just aliases, e.g., fstat resolving to fstat@FBSD_1.0 or so when linking on FBSD 11, we probably don't need any custom mangles. IIUC, that would only allow to compile a FBSD 11 version of druntime on FBSD 12 and link against it on FBSD 12 and so enable cross-compiling a FBSD 11 version of a program on more recent versions. Not sure if there's justified interest in something like that.

@ibuclaw
Copy link
Member Author

ibuclaw commented Nov 19, 2020

@kinke - I can't see any problem from here.

#include <sys/mount.h>
int main()
{
  struct statfs fs;
  statfs("42", &fs);
}
$  nm a.out  | grep statfs
         U statfs@@FBSD_1.5

@jmdavis I guess there is zero reason to have pragma(mangle) if this gets merged in then?

@ibuclaw ibuclaw force-pushed the freebsd/12 branch 3 times, most recently from 86df9f3 to 7a9c9f8 Compare November 20, 2020 00:17
@ibuclaw
Copy link
Member Author

ibuclaw commented Nov 20, 2020

dlang/dmd#11982 got merged into master, so now the version 12 path this PR is now enabled on the FreeBSD 12.1 CI pipeline.

@ibuclaw
Copy link
Member Author

ibuclaw commented Nov 20, 2020

@kinke - I've just gone with explicit versions for now. Waiting for CI pipeline results.

@thewilsonator
Copy link
Contributor

Buildkie failure is unrelated.

Copy link
Member

@jmdavis jmdavis left a comment

Choose a reason for hiding this comment

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

The mistakes here are probably taken from what I did before, but there are a few things that need fixing.

src/core/sys/posix/dirent.d Show resolved Hide resolved
src/core/sys/posix/sys/stat.d Outdated Show resolved Hide resolved
src/core/sys/posix/sys/types.d Outdated Show resolved Hide resolved
@ibuclaw ibuclaw force-pushed the freebsd/12 branch 3 times, most recently from bf65125 to 706592f Compare November 21, 2020 14:31
@ibuclaw ibuclaw added System Headers Bug Fix Include reference to corresponding bugzilla issue labels Nov 21, 2020
@ibuclaw
Copy link
Member Author

ibuclaw commented Nov 21, 2020

@jmdavis addressed nits.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Fix Include reference to corresponding bugzilla issue System Headers
Projects
None yet
6 participants