Spec URL: https://mavit.fedorapeople.org/rpm/tree-sitter-java.spec SRPM URL: https://mavit.fedorapeople.org/rpm/tree-sitter-java-0.20.1-1.fc37.src.rpm Description: Add support for Java to Tree-sitter, an incremental parsing system for programming tools. Fedora Account System Username: mavit
I like the short generic spec! Given that we are likely going to see dozens of those, we should set a good example. Things to look after according to fedora-review: libtree-sitter-java.x86_64: E: shared-library-without-dependency-information /usr/lib64/libtree-sitter-java.so.0.0 libtree-sitter-java.x86_64: E: explicit-lib-dependency libtree-sitter libtree-sitter-java-devel.x86_64: E: double-slash-in-pkgconfig-path /usr/lib64/pkgconfig/tree-sitter-java.pc URL: https///github.com/tree-sitter/tree-sitter-java could you take a look?
(In reply to Marc-Andre Lureau from comment #1) > I like the short generic spec! Given that we are likely going to see dozens > of those, we should set a good example. If this works out okay, I intend to turn it into a template: https://pagure.io/fork/mavit/rpmdevtools/diff/main..tree-sitter > Things to look after according to fedora-review: > libtree-sitter-java.x86_64: E: shared-library-without-dependency-information > /usr/lib64/libtree-sitter-java.so.0.0 I don't know enough about C development to answer this, but I think this is somehow expected because this isn't a stand-alone library, but rather something you have to load with libtree-sitter. Perhaps we shouldn't be installing it directly into %{_libdir}? > libtree-sitter-java.x86_64: E: explicit-lib-dependency libtree-sitter I think we need the explicit dependency here, because of the above: libtree-sitter-java.so doesn't have a dependency on libtree-sitter.so even though it requires it. > libtree-sitter-java-devel.x86_64: E: double-slash-in-pkgconfig-path > /usr/lib64/pkgconfig/tree-sitter-java.pc URL: > https///github.com/tree-sitter/tree-sitter-java This is an bug in the upstream Makefile, which assumes that all Git URLs look like git:tree-sitter/tree-sitter-java.git rather than https://github.com/tree-sitter/tree-sitter-java.git: ifeq (, $(PARSER_URL)) PARSER_URL := $(subst :,/,$(PARSER_REPO_URL)) PARSER_URL := $(subst git@,https://,$(PARSER_URL)) PARSER_URL := $(subst .git,,$(PARSER_URL)) endif I'll prepare a patch.
(In reply to Peter Oliver from comment #2) > (In reply to Marc-Andre Lureau from comment #1) > > I like the short generic spec! Given that we are likely going to see dozens > > of those, we should set a good example. > > If this works out okay, I intend to turn it into a template: > https://pagure.io/fork/mavit/rpmdevtools/diff/main..tree-sitter > looks ok (I wish they would use meson instead, but a bit late now..) > > Things to look after according to fedora-review: > > libtree-sitter-java.x86_64: E: shared-library-without-dependency-information > > /usr/lib64/libtree-sitter-java.so.0.0 > > I don't know enough about C development to answer this, but I think this is > somehow expected because this isn't a stand-alone library, but rather > something you have to load with libtree-sitter. Perhaps we shouldn't be > installing it directly into %{_libdir}? yeah, it looks like tree-sitter is not explicit about where the libraries should be located, and users of the libraries can decide that. This could probably be changed... emacs seems to look at the same location as libtree-sitter... it should rather be $libdir/tree-sitter/ instead imho. > > > libtree-sitter-java.x86_64: E: explicit-lib-dependency libtree-sitter > > I think we need the explicit dependency here, because of the above: > libtree-sitter-java.so doesn't have a dependency on libtree-sitter.so even > though it requires it. Well, it doesn't link to it, then no. However there are is specific tree-sitter ABI (TSLanguage etc). > > > libtree-sitter-java-devel.x86_64: E: double-slash-in-pkgconfig-path > > /usr/lib64/pkgconfig/tree-sitter-java.pc URL: > > https///github.com/tree-sitter/tree-sitter-java > > This is an bug in the upstream Makefile, which assumes that all Git URLs > look like git:tree-sitter/tree-sitter-java.git rather than > https://github.com/tree-sitter/tree-sitter-java.git: > > ifeq (, $(PARSER_URL)) > PARSER_URL := $(subst :,/,$(PARSER_REPO_URL)) > PARSER_URL := $(subst git@,https://,$(PARSER_URL)) > PARSER_URL := $(subst .git,,$(PARSER_URL)) > endif > > I'll prepare a patch. thanks
SRPM URL: https://mavit.fedorapeople.org/rpm/tree-sitter-java-0.20.1-2.fc37.src.rpm Fixes URL in tree-sitter-java.pc.
(In reply to Marc-Andre Lureau from comment #3) > (In reply to Peter Oliver from comment #2) > > (In reply to Marc-Andre Lureau from comment #1) > > > I like the short generic spec! Given that we are likely going to see dozens > > > of those, we should set a good example. > > > > If this works out okay, I intend to turn it into a template: > > https://pagure.io/fork/mavit/rpmdevtools/diff/main..tree-sitter > > looks ok (I wish they would use meson instead, but a bit late now..) Perhaps it’s not too late. The current approach of copying Makefiles into each grammar’s repository isn’t working well. https://github.com/tree-sitter/tree-sitter/issues/1488 > > > Things to look after according to fedora-review: > > > libtree-sitter-java.x86_64: E: shared-library-without-dependency-information > > > /usr/lib64/libtree-sitter-java.so.0.0 > > > > I don't know enough about C development to answer this, but I think this is > > somehow expected because this isn't a stand-alone library, but rather > > something you have to load with libtree-sitter. Perhaps we shouldn't be > > installing it directly into %{_libdir}? > > yeah, it looks like tree-sitter is not explicit about where the libraries > should be located, and users of the libraries can decide that. This could > probably be changed... emacs seems to look at the same location as > libtree-sitter... it should rather be $libdir/tree-sitter/ instead imho. Do we have a sense of how serious this is? Could we successfully explain to upstream the benefit of moving it?
SRPM URL: https://mavit.fedorapeople.org/rpm/tree-sitter-java-0.20.1-3.fc40.src.rpm Eliminate rpmlint explicit-lib-dependency error. I did some investigating, at it turns out that the shared-library-without-dependency-information issue doesn't occur upstream. It happens because we put '-Wl,--as-needed' in LDFLAGS.
Created attachment 1988187 [details] The .spec file difference from Copr build 5874390 to 6394970
lgtm
The Pagure repository was created at https://src.fedoraproject.org/rpms/tree-sitter-java
Thanks for the review!