shinyblink / sled

Satanic/Sexy/Stupid/Silly/Shiny LED matrix controller
https://shinyblink.github.io/sled/
ISC License
121 stars 25 forks source link

mod_mod : The Modening #83

Closed 20kdc closed 5 years ago

20kdc commented 5 years ago

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:

  1. 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.
  2. 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?
  3. 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.

  1. Blows up hilariously on CI / Mac OS X for no reason.
  2. Other potential for unknown explosions.
  3. 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.
  4. Because k2link is mandatory, you get to deal with the dependency graph mess that results. You'll need to "make clean" often, for starters...
  5. 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...
  6. 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.

fridtjof commented 5 years ago

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

vifino commented 5 years ago

@fridtjof any luck?

fridtjof commented 5 years ago

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

20kdc commented 5 years ago

...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 commented 5 years ago

...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 commented 5 years ago

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

fridtjof commented 5 years ago

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 commented 5 years ago

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:

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

vifino commented 5 years ago

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

On that build, 10.12 built fine.

vifino commented 5 years ago

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