mod_mod : The Modening #83

20kdc · 2019-04-09T20:59:43Z

Rearchitecture of module system. I'm not sure this is really refined enough, but vifino would like it as a PR so it gets a PR.
This also includes mod_farbherd since this was derived from that.
In particular I'm concerned about what happens to the CI on Mac OS X.
Advantages:

Good basis for further work on any form of module management system, including meta-modules that run VMs (mod_nes_nrom256, anyone?) and the like. Also, mod_farbherd and similar 'video as module' components.
I tried to clean up what I could along the way. Not sure I really succeeded, and I'm sure my comments will be out of date within the week, but... I tried?
I've made it so you can apply the available filters multiple times without requiring a way to double-load dynamic libraries.

(EDIT : dammit markdown stop messing with my numbers)

Disadvantages: REALLY BIG ONES.

Blows up hilariously on CI / Mac OS X for no reason.
Other potential for unknown explosions.
k2link is mandatory to bootstrap this whole thing.
Y'know, the system based off of a shell script.
It's at LEAST mandatory for mod_dl.
Because k2link is mandatory, you get to deal with the dependency graph mess that results.
You'll need to "make clean" often, for starters...
Any filter modules that didn't get caught in my own rewrites need modification. You should be able to see the pattern easily enough, but...
I modified ASL along the way to make the API simpler, but this means for anyone using ASL, your compatibility is now broken. It's not complex to fix but it'll annoy people.

This fixes #66.

Needs tons of cleanup, eventually, but apparently we're on a time limit. I'll be sending this commit on data, so further communications will not be present until tomorrow.

This only works with STATIC=1 right now, with a limited set of modules. Modules which will need PARTICULAR work are filters. All filters need to be properly setup with PGCTX stuff. Luckily, PGCTX also reduces the amount of boilerplate code for you. PGCTX macros are defined in plugin.h, also, stop using mod.h when you mean plugin.h, which includes mod.h anyway now.

Of these, I've only *fully* tested flt_rot_90 because I did major changes there

This won't help the CI problems, unfortunately... ... is there a way to get better traces from that?

This reverts commit 1f57069. Reason: I'm very stupid and need to sleep. - 20kdc

…it this time

Finally, we're back where we started

fridtjof · 2019-04-10T08:41:34Z

I couldn't verify the macOS issues on my machine, let's see what CI says again

vifino · 2019-04-10T15:26:15Z

@fridtjof any luck?

fridtjof · 2019-04-10T19:35:58Z

Seems like there's a memleak on linux now. macOS looks fine though

20kdc · 2019-04-11T14:54:09Z

...Does the CI build actually have debug symbols?
CFLAGS = -fsanitize=address,undefined might be eliminating them.
It'd be very useful if AddressSanitizer could return precise debug information...

20kdc · 2019-04-11T14:55:59Z

...actually, the debug info is added with:
CFLAGS += -Og -ggdb
so it should remain intact. New theory: The stacktraces just can't handle mod_dl.
In this case, is there a way to get library base addresses out of dl?
It's a more manual process but will help for tracking down the responsible module.

vifino · 2019-04-11T16:41:00Z

Well, we could log the addresses the libraries get loaded at with dlopen, that could help.

vifino

After looking at this again, I spotted some small things.
You've done awesome work, @20kdc.

vifino · 2019-04-13T18:59:37Z

GNUmakefile

@@ -35,7 +35,7 @@ BGMMODS_DEFAULT += bgm_fish bgm_pixelflut
 FLTMODS_DEFAULT += flt_gamma_correct flt_flip_x flt_flip_y flt_scale flt_rot_90
 FLTMODS_DEFAULT += flt_smapper flt_channel_reorder

-MODULES_DEFAULT += $(BGMMODS_DEFAULT) $(FLTMODS_DEFAULT) $(GFXMODS_DEFAULT)
+MODULES_DEFAULT += $(BGMMODS_DEFAULT) $(FLTMODS_DEFAULT) $(GFXMODS_DEFAULT) mod_farbherd


This is ugly, I propose this.

Suggested change

MODULES_DEFAULT += $(BGMMODS_DEFAULT) $(FLTMODS_DEFAULT) $(GFXMODS_DEFAULT) mod_farbherd

LOADERS_DEFAULT := mod_farbherd

MODULES_DEFAULT += $(BGMMODS_DEFAULT) $(FLTMODS_DEFAULT) $(GFXMODS_DEFAULT) $(LOADERS_DEFAULT)

vifino · 2019-04-13T19:01:40Z

GNUmakefile

+
+ifeq ($(OS),Linux)
+src/modules/mod_dl.c.libs:
+	echo -ldl > src/modules/mod_dl.c.libs


Is this actually mod_dl.c.libs and not mod_dl.libs?

vifino · 2019-04-13T19:01:55Z

GNUmakefile

- $(MODULES_WC) src/slloadcore.gen.c: $(MODULES_C) static/kslink
-	cd static ; ./kslink $(addsuffix .c, $(addprefix ../src/modules/, $(MODULES))) > ../src/slloadcore.gen.c
+src/modules/mod_dl.c.libs:
+	echo "" > src/modules/mod_dl.c.libs


Same as above.

vifino · 2019-04-13T19:03:40Z

src/asl.c

 }
+
+void asl_test_av_validity(asl_av_t * self) {


Is this test supposed to be in at all times?
We might wanna make this CIMODE only and run it if possible.

Unit testing sled, huh..

fridtjof · 2019-04-13T19:15:59Z

Before merging this, I'd like to match the macOS versions built against on travis and azure pipelines, to identify on which versions it fails to build.

fridtjof · 2019-04-13T19:18:58Z

However, Azure Pipelines doesn't have a build agent for macOS 10.12. This is the version that travis builds are failing on. This means we will have to either:

drop macOS < 10.13 or
investigate build issues on 10.12

I can't assist with option 2, because my machine is running 10.14.

vifino · 2019-04-13T19:37:50Z

It fails on 10.13 too.
https://travis-ci.org/shinyblink/sled/jobs/519066166

On that build, 10.12 built fine.

This kind of breaks more things involving dependencies but at least keeps the build working

vifino · 2019-04-13T21:02:24Z

Awesome, @20kdc fixed the build issue. It was not macOS specific, it had to do with Make parallelity and k2link.

vifino and others added 22 commits March 24, 2019 13:50

[1/2] Add module userdata, store next out module.

1c2d45a

[2/2] Add module userdata, store next out module.

84dbfd4

Allow non-.so's as modules.

06cfc17

mod_farbherd: WIP farbherd loader.

0867744

Get a farbherd modloader to a workingish state

47a7212

Needs tons of cleanup, eventually, but apparently we're on a time limit. I'll be sending this commit on data, so further communications will not be present until tomorrow.

Stop farbherd files randomly being ignored

37a714b

Clarify modloader startup text to include origin modloader

7d8e6f6

Fix bugs with the mod_mod implementation and get mod_dl working!

660a10d

More cleanup now mod_dl works

701f5e1

Some very FIShy fixes

8bd7f02

static/k2link: made it sh compliant.

e0b1f1c

Update all the filters!

90f3503

Of these, I've only *fully* tested flt_rot_90 because I did major changes there

Bring in the random picker again.

f0f85ac

modules: adapt deinit.

cdc743c

main: free moddir correctly.

75ef5d2

Simplify and possibly fix leaks in argument parsing

1f57069

This won't help the CI problems, unfortunately... ... is there a way to get better traces from that?

Revert "Simplify and possibly fix leaks in argument parsing"

c654f06

This reverts commit 1f57069. Reason: I'm very stupid and need to sleep. - 20kdc

Restore the previous-previous-simplification commit but actually fix …

cc4cf86

…it this time

Fix flip_y filter crash and 2 GFX module init/deinit leaks

79fa412

Port mod_farbherd to the new modloader system

21e83ee

Finally, we're back where we started

ext/farbherd.h: prefix last be32toh.

b8347c5

vifino requested a review from fridtjof April 9, 2019 21:05

vifino added the enhancement Enhancement suggestions to the project. label Apr 9, 2019

vifino added this to In progress in Core Apr 9, 2019

Merge branch 'master' into mod-mod-the-modening

bfc0414

gfx_candyswarm: fix memleak

15ebc60

vifino mentioned this pull request Apr 13, 2019

Module userdata. #81

Closed

vifino approved these changes Apr 13, 2019

View reviewed changes

20kdc added 2 commits April 13, 2019 21:49

Break apart k2link steps to improve compatibility with parallel builds

9c81676

The %.incs file doesn't always exist so the previous commit broke stuff.

20571aa

This kind of breaks more things involving dependencies but at least keeps the build working

vifino merged commit 5eff418 into master Apr 13, 2019

Core automation moved this from In progress to Done Apr 13, 2019

vifino deleted the mod-mod-the-modening branch January 29, 2021 12:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mod_mod : The Modening #83

mod_mod : The Modening #83

20kdc commented Apr 9, 2019 •

edited by vifino

fridtjof commented Apr 10, 2019

vifino commented Apr 10, 2019

fridtjof commented Apr 10, 2019 •

edited

20kdc commented Apr 11, 2019

20kdc commented Apr 11, 2019

vifino commented Apr 11, 2019

vifino left a comment

vifino Apr 13, 2019

vifino Apr 13, 2019

vifino Apr 13, 2019

vifino Apr 13, 2019

fridtjof commented Apr 13, 2019

fridtjof commented Apr 13, 2019

vifino commented Apr 13, 2019

vifino commented Apr 13, 2019

-MODULES_DEFAULT += $(BGMMODS_DEFAULT) $(FLTMODS_DEFAULT) $(GFXMODS_DEFAULT) mod_farbherd
+LOADERS_DEFAULT := mod_farbherd
+MODULES_DEFAULT += $(BGMMODS_DEFAULT) $(FLTMODS_DEFAULT) $(GFXMODS_DEFAULT) $(LOADERS_DEFAULT)

mod_mod : The Modening #83

mod_mod : The Modening #83

Conversation

20kdc commented Apr 9, 2019 • edited by vifino

fridtjof commented Apr 10, 2019

vifino commented Apr 10, 2019

fridtjof commented Apr 10, 2019 • edited

20kdc commented Apr 11, 2019

20kdc commented Apr 11, 2019

vifino commented Apr 11, 2019

vifino left a comment

Choose a reason for hiding this comment

vifino Apr 13, 2019

Choose a reason for hiding this comment

vifino Apr 13, 2019

Choose a reason for hiding this comment

vifino Apr 13, 2019

Choose a reason for hiding this comment

vifino Apr 13, 2019

Choose a reason for hiding this comment

fridtjof commented Apr 13, 2019

fridtjof commented Apr 13, 2019

vifino commented Apr 13, 2019

vifino commented Apr 13, 2019

20kdc commented Apr 9, 2019 •

edited by vifino

fridtjof commented Apr 10, 2019 •

edited