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

standardise mspack source file layout #2103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

p01arst0rm
Copy link

standardise the layout of the mspack source files, simplifies the process of creating distributions.

@Triang3l
Copy link
Member

The original library doesn't have this layout, this will make updating it more complicated.

@p01arst0rm
Copy link
Author

The original library doesn't have this layout, this will make updating it more complicated.

the current implementation of /third_party/crypto also does not follow upstream source code layout, the current project appears inconsistent on whether this is important or not.

@Triang3l
Copy link
Member

Triang3l commented Dec 29, 2022

The crypto directory contains a collection of various encryption algorithms, not a single library or a subset of it, though it might be nice to separate it, especially if at some point it gets replaced with multiple submodules. But overall, what are the real-life use cases that are handled by the movement of files between arguably equally subjectively questionable locations?

@p01arst0rm
Copy link
Author

the locations are not subjective, and follow the layout of other code within the project itself. separating header files from library files can help avoid incidental including of the incorrect files, especially if header file names have the same name as a common system header (the xenia project does).

This PR aligns the codebase with the style of other code within the project. xenia currently does not have any style guide for build system or project maintenance, and vibe checking PRs as they come seems to be somewhat questionable as a form of project standardisation.

@Triang3l
Copy link
Member

While maintaining consistency within the Xenia project is important at least for code navigation simplicity, I'm not sure if this applies to third-party components — as they're by design not the Xenia project, but rather, projects of other developers maintained not by us. We are merely importing them, and any modifications — be they directory tree changes, or something larger like code formatting or naming changes — make the process of importing new versions of them more difficult as they introduce merge conflicts, while not providing any noticeable gains — one more or less directory with a name like src to click while opening the files in the rare cases when someone needs to browse the code of a third-party module is unlikely to cause any inconvenience, and header path issues like the math.h one (though it was not some misuse from our side, but rather a dangerous architectural decision in Android's ndk-build that adds the module directory as the highest-priority system search path) can be solved on a case-by-case basis.

Moreover, we already occasionally get bitten by the fact that we're using our own Premake scripts for third-party modules instead of the original build pipelines of the projects. Updating something like FFmpeg in our case is a story in itself — we already need to run our custom converter of its build files into Premake, and also to generate the configuration header files for multiple targets manually, and to verify that nothing is broken in the end (for example, we've already experienced issues building its assembly code for Android, as unlike the build system originally used by FFmpeg, we initially didn't have the hidden visibility compiler option specified by our Premake scripts, thus ended up with some runtime relocations which are not allowed in Android libraries as they must be position-independent). However, this is the price we have to pay to avoid having to make all new and existing contributors deal with the total, much bigger chaos of setting up the build systems for all the dependencies separately in whichever OS environment they're using, or having to automate this process which is roughly as complicated. The directory structure inconsistencies don't result in anything like that, however.

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.

None yet

2 participants