Skip to content

Add script for generating an amalgamated distribution #302

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

Merged
merged 3 commits into from
Jan 9, 2022

Conversation

athre0z
Copy link
Member

@athre0z athre0z commented Jan 7, 2022

This PR adds a script that creates an amalgamated build of Zydis. In other words: the script merges all sources files and internal headers into a single .c file and all public headers into a single .h files. The term "amalgamated" was inspired by libraries like SQLite that provide similar distributions.

I'll add a CI build (and perhaps automate attaching such a build to releases) in a separate PR. (amended to this PR)

Note: this PR is based on the remove-generateexportheader branch, because the export config was previously causing problems in the amalgamated builds. (merged and rebased on master)

@athre0z athre0z added C-enhancement Category: Enhancement of existing features P-medium Priority: Medium A-build Area: Build system labels Jan 7, 2022
@athre0z athre0z requested a review from flobernd January 7, 2022 17:32
@@ -1,4 +1,4 @@
static const ZydisShortString STR_REGISTER[] =
static const ZydisShortString STR_REGISTERS[] =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name clashes with another STR_REGISTER somewhere in the generated formatter strings when doing amalgamated builds, so I had to rename it. I'll push a commit to also change that in gen_registers.py once this PR is merged.

@athre0z athre0z force-pushed the single-header-dist branch 2 times, most recently from 25a0881 to 066e9f0 Compare January 7, 2022 18:34
Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@athre0z athre0z force-pushed the remove-generateexportheader branch from dfc3847 to 9d1cee5 Compare January 9, 2022 12:47
Base automatically changed from remove-generateexportheader to master January 9, 2022 12:55
@athre0z athre0z force-pushed the single-header-dist branch from 066e9f0 to 7d25c64 Compare January 9, 2022 13:03
@athre0z
Copy link
Member Author

athre0z commented Jan 9, 2022

Figured that it probably makes more sense to also add CI in this PR, so added a commit for that. Turned out that the CI's Python version isn't latest, so I had to downgrade a few things. Will clean up history after approval.

@athre0z athre0z requested a review from flobernd January 9, 2022 14:54
@athre0z athre0z force-pushed the single-header-dist branch from c82846c to 690dedd Compare January 9, 2022 18:05
@athre0z athre0z merged commit 6963628 into master Jan 9, 2022
@athre0z athre0z deleted the single-header-dist branch January 9, 2022 18:09
@Tachi107
Copy link
Contributor

Tachi107 commented Jan 13, 2022

Turned out that the CI's Python version isn't latest, so I had to downgrade a few things.

In case you didn't know, GitHub's CI offers the possibility to run jobs in Docker images, so you're not limited to using Ubuntu LTS; if you need updated packages you could use Debian Testing, it's really nice :)

Here's a quick example (taken from Tachi107/cloudflare-ddns/.github/workflows/muon.yam):

jobs:
  muon:
    runs-on: ubuntu-latest
    container:
      image: debian:testing

    steps:
      ...build steps...

Edit: another advantage is that your build gets executed in a cleaner environment compared to the default one (as GitHub ships A LOT of software in their CI images), so the possibility of strange bugs related to the tests using unknown versions of installed library when they shouldn't is reduced

@athre0z
Copy link
Member Author

athre0z commented Jan 13, 2022

That's good to know, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build Area: Build system C-enhancement Category: Enhancement of existing features P-medium Priority: Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants