Fix LibGodot build errors on Linux.

[zorbathut]

This fixes LibGodot shared_library build to work properly on Linux; there was unfortunately some missing functionality in the original PR, #110863.

There's two fixes here:

• The -fPIC flag needs to be introduced during the platform configure() step so that modules will be built with it.
• The x11 and wayland platform sub-modules shouldn't use env.Object because it doesn't have the correct flags attached to it; instead they need to be added to linuxbsd/SCSub's common_linuxbsd array so that add_library, add_shared_library, or add_program can do the right thing. This basically just means that we need to return a path instead of an env.Object, but we do need to wrap it with a constant path to get the right complete filename. (There might be a cleaner way to do this.)

I can untangle these into separate PRs if you'd prefer them separate, this is just how I wrote 'em.

To test, run scons library_type=shared_library on Linux. (Shared libraries tend to be the pain point.)

[zorbathut]

Should be ready now, apologies for my minor tiff with Git.

[deralmas]

There might be a cleaner way to do this.

I'm no build system expert, but I suppose that we could at least iterate over source_files and append base in a loop instead of doing it every time?

[zorbathut]

I'm no build system expert, but I suppose that we could at least iterate over source_files and append base in a loop instead of doing it every time?

It's a little more complicated than this because x11 includes a file that actually has an absolute path ("#thirdparty/glad/glx.c") so there needs to be a little logic to avoid that one, or it needs to be appended afterwards, which itself is complicated because it's only added in a conditional. I'll give it a shot though.

[deralmas]

It's a little more complicated than this because x11 includes a file that actually has an absolute path ("#thirdparty/glad/glx.c")

Ooooh aight that's fair then. Nw if accounting for it is a mess, it's fine if we append base manually too.

[deralmas]

Hi, I've taken a look at the SCons docs and it looks like we might also use a File node. From what I can tell, it resolves the path on initialization and then can be used anywhere, so that we don't have to hardcode the platform's path.

File Nodes can be used anywhere you would supply a string as a file name to a Builder method or function.

Seems quite handy, let me know if that does the job :D

[zorbathut]

Alright, tried them all :V Github appears to be having technical issues right now so they're not showing up on this page, but here's direct links:

Manual base-prepend

Postprocess base-prepend

Manual Path

I didn't do a "postprocess Path", I could do that too.

They're all a little ugly in slightly different ways. I have no strong feelings here. Is there a person I should ping on the build team to figure out which they prefer? Happy to do whatever.

[fire]

Personally I use list each path as a string so things aren't too abstracted, but I'd go with the reviewer commentary.

From my point of view I only start looking into optimizations when I have lists of 100s of filepaths.

[deralmas]

From my point of view I only start looking into optimizations when I have lists of 100s of filepaths.

This isn't really much of an optimization, more like avoiding to hardcode paths into platforms dirs. While that shouldn't matter, it's still good practice, as platform implementations are supposed to be reasonably self-contained from the rest of the engine.

[zorbathut]

Yeah, I'm zero-percent concerned about performance here, and one-hundred-percent concerned about code cleanliness. I'm just not sure which of these is the cleanest.

[deralmas]

Personally I find this File thing pretty clean, and good code hygiene in general, since you're making it explicit that you're referring to a file :)

That said I don't really specialize in maintaining the build system so TIWAGOS.

The build team already got pinged around thanks to CODEOWNERS but it's also true that most of it is like, perennially busy.

[deralmas]

BTW I Think that it should work for top-relative path names too

[zorbathut]

Good call, I'll put that in for the sake of consistency.

[deralmas]

Personally, I think that this looks great :D

[zorbathut]

Just gonna squash down to that one then, I think it's probably the best of the batch.

[akien-mga]

Seems fine.

[Repiteo]

The X11/Wayland SCsub files are definitely unconventional here, but that was apparently already the case, so I won't consider it a blocker. Will absolutely give them a pass in a followup PR though

[Repiteo]

Thanks!