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

Add a simple Makefile-based build system. #602

Merged
merged 2 commits into from
Apr 22, 2020
Merged

Add a simple Makefile-based build system. #602

merged 2 commits into from
Apr 22, 2020

Conversation

eli-schwartz
Copy link
Contributor

This produces static/shared libraries as well as a pkg-config file, and installs them to the standard Unix hierarchy. GNU Make is assumed as features like $(wildcard ...) or $(shell uname) may not work elsewhere, but it should otherwise build on most/all Unix platforms.

Note that we assume the following POSIX default rules/macros exist, so we don't bother redefining them:

  • pattern rules for compiling .c -> .o
  • $(CC)
  • $(AR)

Special note on the .pc file generation: we do most variable replacements in one go, but delay @PREFIX@ so that we can first find existing substring instances of the prefix value (if libdir/includedir reside within the prefix), and update them to use a literal pkg-config variable ${prefix}. This is fairly compact (one single sed) while still letting us produce pkg-config files that support runtime redefinition à la cross-compilation.

Resolves #581

@eli-schwartz
Copy link
Contributor Author

/cc @anthraxx @FFY00 @SantiagoTorres

@eli-schwartz
Copy link
Contributor Author

eli-schwartz commented Apr 21, 2020

I've assumed here that soname versioning of the form X.Y will be used, starting with 1.0 because this is stable and the first soname-versioned release.

Headers are installed directly to (default) /usr/include and should be used via the pkg-config cflags and #include <tree_sitter/api.h>, I believe this is correct?

The general release process would essentially boil down to bumping the VERSION variable in sync with the repository tag, and deciding whether to bump SONAME_MAJOR on ABI-breaking versions, or SONAME_MINOR on ABI-compatible versions.

Due to the use of wildcards, new source files in lib/src/ will be automatically picked up, and you can optionally do an amalgamated build with make AMALGAMATED=1, because I figured hey, why not -- the amalgamated source already exists, lets make it usable.

Copy link
Contributor

@dcreager dcreager left a comment

Choose a reason for hiding this comment

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

Love how clean this is! Could you also add an extra step to the .travis.yml file that runs make as a basic sanity check as part of CI? I don't think we'd need to do that in the appveyor CI, too — no need to jump through hoops to get make and friends installed there, since this Makefile is intended for Linux packagers.

Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@maxbrunsfeld maxbrunsfeld left a comment

Choose a reason for hiding this comment

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

This is great. I really appreciate the simplicity that you were able to achieve, while still meeting the requirements.

After we merge this, I will probably remove the build-lib and mention building via make in the docs.

Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@eli-schwartz
Copy link
Contributor Author

I pushed changes for two of the review comments, tell me what you want for the other two (if anything).

@eli-schwartz
Copy link
Contributor Author

eli-schwartz commented Apr 22, 2020

http://make.mad-scientist.net/papers/advanced-auto-dependency-generation/

DEPFLAGS = -MT $@ -MMD -MP -MF $*.d
CFLAGS += $(DEPFLAGS)
-include lib/src/*.d

This works on at least gcc and clang to generate Makefile fragments and include them, which cause the object files to depend on all the header files that went into them. As those are the only two compilers I use, I'm not familiar with which compilers might not support it, but they're pretty useful and fairly common, I think... it would be able to be suppressed with make DEPFLAGS= if so. Thoughts?

(Note that autotools is confident enough to include it in Makefile.in, before $CC is detected from the environment.)

@maxbrunsfeld
Copy link
Contributor

When developing the library, we mostly are using the rust build system to compile it, since the test suite and the CLI both use the rust binding. we do a clean build of the C code every time, because the cost is negligible compared to the rust build time.

So I don’t think we’ll be doing a lot of incremental builds via the Makefile. Maybe it’d even be better to default to the amalgamated build, because it has less possibility of stale object files.

Overall, I favor making the makefile as simple and compatible as possible for users of the library, versus adding features to help in developing the library.

This produces static/shared libraries as well as a pkg-config file, and
installs them to the standard Unix hierarchy. GNU Make is assumed as
features like $(wildcard ...) or $(shell uname) may not work elsewhere,
but it should otherwise build on most/all Unix platforms.

Note that we assume the following POSIX default rules/macros exist, so
we don't bother redefining them:
- pattern rules for compiling .c -> .o
- $(CC)
- $(AR)

Special note on the .pc file generation: we do most variable
replacements in one go, but delay @Prefix@ so that we can first find
existing substring instances of the prefix value (if libdir/includedir
reside within the prefix), and update them to use a literal pkg-config
variable '${prefix}'. This is fairly compact (one single sed) while
still letting us produce pkg-config files that support runtime
redefinition à la cross-compilation.
@eli-schwartz
Copy link
Contributor Author

All right then, I won't bother. I've just resolved the other two review comments and thrown in a make clean target.

@maxbrunsfeld
Copy link
Contributor

Thanks for your work on this, and the RFC.

@maxbrunsfeld maxbrunsfeld merged commit c393591 into tree-sitter:master Apr 22, 2020
@eli-schwartz eli-schwartz deleted the makefile branch April 22, 2020 04:48
XVilka pushed a commit to radareorg/radare2 that referenced this pull request Apr 23, 2020
As of tree-sitter/tree-sitter#602 it is possible
to install tree-sitter as a system shared library, and distributions that want to
be able to disable vendored code.
Emi1305 pushed a commit to Emi1305/radare2 that referenced this pull request Jul 12, 2020
As of tree-sitter/tree-sitter#602 it is possible
to install tree-sitter as a system shared library, and distributions that want to
be able to disable vendored code.
@eli-schwartz eli-schwartz mentioned this pull request Aug 27, 2021
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.

RFC: provide a build system for Linux distribution integration
5 participants