Port upstream libffi 3.4.4 for x86_64

[andreittr]

This change ports the parts of libffi 3.4.4 relevant to x86 to unikraft.

Part of work on Python 3.10 (unikraft/lib-python3#15).

[StefanJum]

@andreittr Will we only support this on x86_64 for this release?

[StefanJum]

Suggested change

	Please refer to the `README.md` as well as the documentation in the
	`doc/` subdirectory of the main unikraft repository for further
	information.
	Please refer to the `README.md` as well as the documentation in the `doc/` subdirectory of the main unikraft repository for further information.

Use one sentence per line and one line per sentence.

[StefanJum]

Suggested change

	libffi for Unikraft
	========================
	# `libffi` for Unikraft

Use md format, not rst.

[andreittr]

@andreittr Will we only support this on x86_64 for this release?

Only the x86_64 work is done thus far (and getting arm done before release is unlikely); perhaps @razvand can opine whether to upstream this in current release or postpone until arm work is also done.

[andreittr]

Update: made changes as discussed in unikraft/lib-python3#15.

[mariasfiraiala]

Thanks @andreittr, I've tested this together with unikraft/lib-python3#15 and it builds just fine.

I am, however, reluctant to have a config option that disables something. I am aware of the usecase, but it still weirds me out; it's kinda counterintuitive to select an option in order to not have the definitions..

Anyway, it's a personal preference, I'll approve the changes once the app-python3 PR is out in order to have a run too.

[andreittr]

Thanks @andreittr, I've tested this together with unikraft/lib-python3#15 and it builds just fine.

I am, however, reluctant to have a config option that disables something. I am aware of the usecase, but it still weirds me out; it's kinda counterintuitive to select an option in order to not have the definitions..

Anyway, it's a personal preference, I'll approve the changes once the app-python3 PR is out in order to have a run too.

Oh I completely agree with you on this front, negative options are always more confusing than positive ones. Pity that Kconfig does not support forcefully unselecting a value. Unless and until that becomes a thing I'm afraid we're stuck with this inelegant solution. Other alternatives off the top of my head would be

(a) Leave things as before (positive option, py3 leaves it alone); upside: no negative config options; downside: if option is default 'y' -> py3 won't build unless tweaked, if option defaults to 'n' -> libffi won't build by itself unless tweaked; either way requires documentation and/or tribal knowledge

(b) Leave things as before but check & error out in the Makefiles; upside: no negative config options; downside: we leave to manual trial-and-error something that can and should be automated, and no amount of explaining Kconfig's limitations will lessen users' frustration

(c) Keep negative option but make it invisible; upside: no user confusion by default, libs can still opt out of ffi_type_* definitions when needed; downside: blackmagic™ happends during configuration, it's not clear where to disable ffi_type_* stuff if needed for e.g. a custom lib/app

(d) hack our own fork of Kconfig that can do "select CONFIGVAL=n" (I'm ~90% joking with this one, totally not volunteering to do smth like this)

Thoughts?

[mariasfiraiala]

Now that you've laid out the options we have, @andreittr, I'd go with the black magik stuff (October it's rapidly approaching 🧙‍♀️). It's the one that has the least damaging downsides, imo. @StefanJum opinions on this?

Btw, really really nice presentation of the alternatives @andreittr 💯

[StefanJum]

Oh I completely agree with you on this front, negative options are always more confusing than positive ones. Pity that Kconfig does not support forcefully unselecting a value. Unless and until that becomes a thing I'm afraid we're stuck with this inelegant solution.

Right on point. I would honestly keep the negative option as it is now. It is ugly, but at least it's clear and things work out of the box.
From the options you provided I think we could go with (c) (so everything will still work out of the box), but again, I think the current way is cleaner, as ugly as it is.

[mariasfiraiala]

We shall have it as it is then, all good!

[andreittr]

I'd go with the black magik stuff

I think we could go with (c) (so everything will still work out of the box), but again, I think the current way is cleaner, as ugly as it is.

Just thought of a decent compromise between this PR and option (c): keep the option visible but rename its user prompt to a positive statement like "Allow other libraries to supply ffi_type_* definitions". Best of both worlds?

[StefanJum]

Just thought of a decent compromise between this PR and option (c): keep the option visible but rename its user prompt to a positive statement like "Allow other libraries to supply ffi_type_* definitions". Best of both worlds?

Nice, I think we can do something like Use external ffi_type_* definitions and also provide a help message that will state that if the option is selected, the ffi_type_* are not defined, and are to be supplied by an external library.

[RaduNichita]

I also tested this as part of python3.10 PR series and it builds and runs fine. Thanks, @andreittr for this nice work! 🏅

I would also go for option c, I think it's the most appropriate for the reasons that @StefanJum and @mariasfiraiala have already mentioned.

[StefanJum]

Thank you @andreittr 🚀
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

[mariasfiraiala]

Thanks for this long journey!

Reviewed-by: Maria Sfiraiala maria.sfiraiala@gmail.com

[RaduNichita]

Let's merge this. Thanks @andreittr for the contribution! 🔝

Reviewed-by: Radu Nichita radunichita99@gmail.com

[razvand]

Approved-by: Razvan Deaconescu razvand@unikraft.io