Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dependency restore mode and refactoring #39

Draft
wants to merge 6 commits into
base: feat/static-shared-combine
Choose a base branch
from

Conversation

Fernthedev
Copy link
Collaborator

@Fernthedev Fernthedev commented Jan 1, 2024

  • Use QPM.Package 1.0.0
  • Properly support dynamicLibOut and staticLibOut
  • Store libDependencyType in qpm.shared.json
  • Refactor qpm qmod build for more flexibility
  • Refactor qpm install to use dynamicLibOut and/or staticLibOut
  • Allow choosing to restore header only, static or dynamic
  • No longer excluding modloader by default
  • Remove debug dynamic libraries
  • Remove overrideSoName
  • CMake COMPILE_ID now uses qpm.json id
  • Versioned library names are no longer generated by qpm

Because of these changes, the following will cause existing packages (both for usage and restoring) to break.

  • Requirement to use dynamicLibOut, a path to the dynamic binary
  • Requirement to use staticLibOut, a path to the static binary
  • No longer using overrideSoName, therefore CMake id is now tied to mod id in qpm_defines.cmake
  • Debug dynamic libraries are no longer supported. One must expect the published so to include debug symbols.
  • Versioned libraries are no longer handled by qpm

Supersedes #31

- Use QPM.Package 1.0.0
- Properly support `dynamicLibOut` and `staticLibOut`
- Store `libDependencyType` in `qpm.shared.json`
- Refactor qpm qmod build for more flexibility
- Refactor `qpm install` to use `dynamicLibOut` and/or `staticLibOut`
- Allow choosing to restore header only, static or dynamic
- No longer excluding `modloader` by default
- Remove debug dynamic libraries
- Remove `overrideSoName`
- CMake `COMPILE_ID` now uses `qpm.json` id
@darknight1050
Copy link

What do you mean by Allow choosing to restore header only, static or dynamic? A option for the restore command or something in qpm.json?

@RedBrumbler
Copy link
Contributor

RedBrumbler commented Feb 20, 2024

Debug dynamic libraries are no longer supported. One must expect the published so to include debug symbols.

this seems like a bad idea, some of the libraries, if they include debug symbols (not stripped) are 10x as large as the stripped version.

example: BSML is 124 MB unstripped, and 5 MB stripped

@Fernthedev
Copy link
Collaborator Author

Fernthedev commented Feb 20, 2024

Debug dynamic libraries are no longer supported. One must expect the published so to include debug symbols.

this seems like a bad idea, some of the libraries, if they include debug symbols (not stripped) are 10x as large as the stripped version.

example: BSML is 124 MB unstripped, and 5 MB stripped

That's up to the discretion of the distributor in this instance, because maybe you think the consumers wouldn't need the baggage of those debug symbols. Keeping both in the current state seems reasonable but would have to be reworked in this PR, and am not sure it's as desirable as it sounds.

@Fernthedev
Copy link
Collaborator Author

What do you mean by Allow choosing to restore header only, static or dynamic? A option for the restore command or something in qpm.json?

This would allow you to specify for dependency restore, say, static linking flamingo as so:

{
  "id": "flamingo",
  "version": "*",
  "additionalData": {
    "libDependencyType": "static"
  }
}

@RedBrumbler
Copy link
Contributor

Debug dynamic libraries are no longer supported. One must expect the published so to include debug symbols.
this seems like a bad idea, some of the libraries, if they include debug symbols (not stripped) are 10x as large as the stripped version.
example: BSML is 124 MB unstripped, and 5 MB stripped

That's up to the discretion of the distributor in this instance, because maybe you think the consumers wouldn't need the baggage of those debug symbols. Keeping both in the current state seems reasonable but would have to be reworked in this PR, and am not sure it's as desirable as it sounds.

well you want the debug symbols somewhere, given that it makes debugging easier with the crash reporter mod. although you could argue that the crash reporter mod can just get the debug binary manually without qpm. for building with a mod it's obviously not required to exist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants