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

Fixed build script for BSD systems #1554

Closed
wants to merge 2 commits into from

Conversation

mqudsi
Copy link
Contributor

@mqudsi mqudsi commented Sep 16, 2017

In autogen.sh, don't assume that the correct Makefile interpreter is
make. The syntax of the Makefile generated by autotools is not
compatible with BSD Make, and must be built with GNU Make (gmake)
instead.

In addition to detecting BSD platforms (via uname) and using gmake
instead of make, autogen.sh also generates a BSDmakefile in the
top-level directory. In its presence, BSD Make will use it instead of a
generic Makefile, and BSD users will be informed that they should use
gmake instead of make to buil the project.

Tested on FreeBSD.

In autogen.sh, don't assume that the correct Makefile interpreter is
`make`. The syntax of the Makefile generated by autotools is not
compatible with BSD Make, and must be built with GNU Make (gmake)
instead.

In addition to detecting BSD platforms (via uname) and using gmake
instead of make, autogen.sh also generates a BSDmakefile in the
top-level directory. In its presence, BSD Make will use it instead of a
generic Makefile, and BSD users will be informed that they should use
gmake instead of make to buil the project.

Tested on FreeBSD.
@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 16, 2017

There are no other issues building on FreeBSD. With this patch, I'm able to build it simply by executing ./autogen.sh; gmake; sudo gmake install.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.941% when pulling ded3b01 on mqudsi:bsdbuild into c37967d on universal-ctags:master.

@masatake
Copy link
Member

Thank you.
How about updating "How to build and install" section of README.md, and docs/autotools.rst ?

@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 16, 2017

Done.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.941% when pulling e49db10 on mqudsi:bsdbuild into c37967d on universal-ctags:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 85.941% when pulling e49db10 on mqudsi:bsdbuild into c37967d on universal-ctags:master.

Copy link
Member

@b4n b4n left a comment

Choose a reason for hiding this comment

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

The syntax of the Makefile generated by autotools is not compatible with BSD Make

Actually the Makefiles generated by Autotools are portable, but the custom rules we have (especially for the tests) are not, and use quite a few GNU Make extensions indeed.

else
#return false
return 1
fi
Copy link
Member

Choose a reason for hiding this comment

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

Could be simplified a lot with

is_bsd() {
	uname | grep -i bsd >/dev/null
}

ctags_files=`make -f makefiles/list-translator-input.mak --no-print-directory`
gen_bsdmakefile() {
set +xe
echo -e '.DONE:\n\t@echo "Please use GNU Make (gmake) to build this project"\n.DEFAULT:\n\t@echo "Please use GNU Make (gmake) to build this project"' > BSDmakefile
Copy link
Member

Choose a reason for hiding this comment

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

Can't you generate the 2 rules at once? like

.DONE .DEFAULT:
	@echo "Please use GNU Make (gmake) to build this project"

Copy link
Member

Choose a reason for hiding this comment

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

Also, would it be handy to forward the call to GNU Make automatically? If

.DONE .DEFAULT:
	@echo "Please use GNU Make (gmake) to build this project"
	make $@

is enough it seems handy. OK, maybe it's trickier because we'd like to forward some of the MAKEFLAGS but they are not all compatible between GNU and BSD make (e.g. I know NetBSD make adds a -J option GNU make doesn't understand to MAKEFLAGS when the user uses -j). Just a suggestion in case it wasn't already considered.

@@ -5,7 +5,31 @@ set -xe
type autoreconf || exit 1
type pkg-config || exit 1

ctags_files=`make -f makefiles/list-translator-input.mak --no-print-directory`
gen_bsdmakefile() {
set +xe
Copy link
Member

Choose a reason for hiding this comment

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

why changing those shell options? What is supposed to be the problem with e here, and why disabling x?

@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 17, 2017

This is a BSD Makefile that will I think call GNU Make successfully for most cases:

JARG=
.if "$(.MAKE.JOBS)" != ""
    JARG = -j$(.MAKE.JOBS)
.endif

.DONE .DEFAULT:
        gmake $(.TARGETS:S,.DONE,,) $(JARG)

I was trying to avoid being too smart in my original PR.

@codebrainz
Copy link
Contributor

Why not fix the build system to not rely on GNU-make-isms? Doesn't it sort of defeat the purpose of Automake?

@masatake
Copy link
Member

masatake commented Sep 17, 2017

Is there any CI service like travis to for testing ctags on FreeBSD?

@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 17, 2017

@codebrainz I suppose it depends on how much time one has on their hands for such an undertaking. The differences between posix Make and GNU Make are many, and sticking to the former means having to give up a lot of the things that make GNU Make easy to code in.

At the end of the day, it's quite easy to say how ctags should build on all platforms in theory, but it's another to pull it off in practice. This PR was just to introduce a quick and painless fix; if someone wants to reject this 24-line patch that improves the build experience on FreeBSD in favor of a potentially massive undertaking in overhauling the existing build scripts to be properly platform agnostic, honestly, more power to them.

Keep in mind that autotools, like every other GNU project, doesn't really treat non-GUN platforms and toolchains as first-class citizens and can't be guaranteed to "just work." See the OpenBSD documentation on autotools:

Autoconf is a GNU tool that is supposed to help in writing portable programs. It is often used together with automake (portable makefiles) and libtool (portable shared libraries).
Those tools do not work all that well, and often create specific challenges in porting software to OpenBSD.

@codebrainz
Copy link
Contributor

Keep in mind that autotools, like every other GNU project, doesn't really treat non-GUN platforms and toolchains as first-class citizens and can't be guaranteed to "just work."

It should "just work" if there aren't any GNU Make-isms in the Makefile.am files. It even has options which will effectively guarantee this (-Wportability/-Wextra-portability/etc).

@codebrainz
Copy link
Contributor

Ah, I didn't realize it doesn't use Automake, I only looked at the configure.ac file where it initializes it for some reason.

@b4n
Copy link
Member

b4n commented Sep 17, 2017

Ah, I didn't realize it doesn't use Automake

It used not to, but now it does, and the makefile in question is a remainder from those earlier times. We probably should replace these sub-makefiles with just SUBDIRS = .. or something like that if it works.

Anyway, the issue at hand: yes, Autotools generate portable makefiles, but we have quite a lot of rules that are written in GNU Make. I'm not sure how easy it'd be to make them portable, but it doesn't look totally straightforward.

I was trying to avoid being too smart in my original PR.

If it risks too much not working perfectly, you original mere warning seems good enough to me as well.

@codebrainz
Copy link
Contributor

Autotools generate portable makefiles, but we have quite a lot of rules that are written in GNU Make. I'm not sure how easy it'd be to make them portable, but it doesn't look totally straightforward.

The testing.mak is the only one that looks complicated, though it could probably be made cleaner/better using Automake test harnessing stuff (doesn't Geany run ctags tests using Autotools?).

@masatake
Copy link
Member

I found "bmake", The NetBSD make(1) tool, package in Fedora.

As @codebrainz, spotted, bamek reports an error at setting TIMEOUT in makefiles/testing.mak when I run "bmake all".

TIMEOUT is set to 10 when make command finds that timeout command is available. It is obvious this existence checking is done in configure. The change will not be so big.

@mqudsi, on your FreeBSD environment, doen "make all" fail?
From the experience running "bamke all", I guess "make all" on your FreeBSD is done successfully.

I'm thinking about using bmake for doing CI on circleci where Fedora runs.

@masatake masatake mentioned this pull request Sep 19, 2017
@masatake
Copy link
Member

@mqudsi, could you try the latest code? I removed lines depending on GNU make.

@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 20, 2017

@masatake

mqudsi@php ~/ctags> ./autogen.sh
+ type autoreconf
autoreconf is /usr/local/bin/autoreconf
+ type pkg-config
pkg-config is /usr/local/bin/pkg-config
+ make -f makefiles/list-translator-input.mak --no-print-directory
usage: make [-BeikNnqrstWwX]
            [-C directory] [-D variable] [-d flags] [-f makefile]
            [-I directory] [-J private] [-j max_jobs] [-m directory] [-T file]
            [-V variable] [variable=value] [target ...]
+ ctags_files=''

@mqudsi
Copy link
Contributor Author

mqudsi commented Sep 20, 2017

Removing --no-print-directory from autogen.sh lets that go through.

The only error from that output is this warning:

/usr/local/share/automake-1.15/am/tags.am: warning: redefinition of 'ctags' ...
/usr/local/share/automake-1.15/am/program.am: ... 'ctags$(EXEEXT)' previously defined here

but that occurs with gmake as well.

After ./configure, make and sudo make install continue just fine.

mqudsi added a commit to mqudsi/ctags that referenced this pull request Sep 20, 2017
This fixes the remaining BSD build issues after the recent bmake fixes
were merged into master. Discussed in universal-ctags#1554. Closes universal-ctags#1554.
@codebrainz
Copy link
Contributor

/usr/local/share/automake-1.15/am/tags.am: warning: redefinition of 'ctags'

I believe that's because Automake ships with builtin rules/targets called ctags for generating TAGS file using the build system. I guess it's kind of circular for this project :)

@masatake
Copy link
Member

masatake commented Oct 14, 2018

@mqudsi, I have a question about bmake.

I would like to use code-generators in ctags build-process as I have worked on #1847.
In #1847, I would like to use two code generators: optlib2c and packcc.
optlib2c generates foo.c from foo.ctags.
packcc generates bar.c from bar.peg.

I wrote in Makefile.am as follows.

%.c: %.ctags $(OPTLIB2C) Makefile
	$(optlib2c_verbose)$(OPTLIB2C) $< > $@
%.c: %.peg $(PACKCC) Makefile
	$(packcc_verbose)$(PACKCC) $<

GNU make works well with Makefile derrived from above Makefile.am.
However, bmake doesn't work well.

Do you have any idea for working around it?

@mqudsi
Copy link
Contributor Author

mqudsi commented Oct 14, 2018

@masatake the %.foo notation is GNU make specific. Under BSD, the percent signs are omitted. If it's not a suffix recognized by default (.ctags is probably not), you use the .SUFFIXES pseudo target to tell it to treat that pattern as a suffix (extension):

.SUFFIXES: .c .ctags .peg

.ctags.c: $(OPTLIB2C) Makefile
    $(optlib2c_verbose)$(OPTLIB2C) $< > $@

.peg.c: $(PACKCC) Makefile
    $(packcc_verbose)$(PACKCC) $<

(excuse any typos above)

I have zero experience with automake syntax, but I imagine there's some sort of provision for conditionally emitting Makefile code, which you can predicate on the version of Make (or fall back to the OS you are compiling under) to choose between two blocks, one containing GMake-specific code and the other containing bmake-specific code.

Alternatively, if you have access to the names of the files ahead of time then you can replace the suffix/pattern transformation with a (somehow dialect-agnostic) for loop, which I imagine is well-supported by automake.

masatake added a commit to masatake/ctags that referenced this pull request Oct 14, 2018
Suggested by @mqudsi in universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake
Copy link
Member

@mqudsi, thank you very much. I already tried the same.
However, it didn't work. ( #1847).

[yamato@master]~/var/ctags-peg% bmake
REPOINFO   main/repoinfo.h
bmake  all-am
REPOINFO   main/repoinfo.h
bmake[1]: don't know how to make peg/java.c. Stop

bmake[1]: stopped in /home/yamato/var/ctags-peg
*** Error code 2

I think I cannot ask you to debug my code. So I would like to add GNU make as build-requirement of Universal-ctags.

@b4n
Copy link
Member

b4n commented Oct 14, 2018

I already tried the same.
However, it didn't work.

You swapped the order of the suffixes: it should be .SOURCE.TARGET (as in @mqudsi's example above), but in 6ac1c00 you inverted it to .c.peg, which is then a rule for translation a .c file to a .peg one. It should be .peg.c.

Also, it's not possible to add prerequisites to a suffix rule; for that @mqudsi's example is incorrect. You should have:

SUFFIXES += .ctags
.ctags.c:
    $(optlib2c_verbose)$(OPTLIB2C) $< > $@
$(OPTLIB2C_INPUT): $(OPTLIB2C) Makefile

SUFFIXES += .peg
.peg.c:
    $(packcc_verbose)$(PACKCC) $<
$(PEG_INPUT): $(PACKCC) Makefile

@mqudsi
Copy link
Contributor Author

mqudsi commented Oct 14, 2018

@b4n nice catch about the prerequisites. (I wrote that on a Windows machine, in my defense 😄)

So I would like to add GNU make as build-requirement of Universal-ctags.

@masatake that was the case prior to this PR, the goal of which was to at least allow running make under BSD to work.

masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
Suggested by @b4n and @mqudsi in universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
@masatake
Copy link
Member

@b4n, it works!
Thank you very much.

masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
Suggested by @b4n and @mqudsi in universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
Suggested by @b4n and @mqudsi in universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Oct 15, 2018
Packcc is a compiler-compiler; it translates .peg grammar file to .c
file.  packcc was originally written by Arihiro Yoshida. Its source
repository is at sourceforge. It seems that packcc at sourceforge is
not actively maintained. Some derived repositories are at
github. Currently, our choice is https://github.com/enechaev/packcc.
It is extended to support unicode.

The source tree of packcc is grafted at misc/packcc directory.
Building packcc and ctags are integrated in the build-scripts of
Universal-ctags.

If misc/packcc directory is empty, run following command to get
source code before building ctags:
```
$ git submodule init misc/packcc
$ git submodule update misc/packcc
```

About the suffix rule about .c and .peg, @b4n and @mqudsi help me
on universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Feb 2, 2019
Suggested by @b4n and @mqudsi in universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Feb 3, 2019
Suggested by @b4n and @mqudsi in universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Feb 3, 2019
Packcc is a compiler-compiler; it translates .peg grammar file to .c
file.  packcc was originally written by Arihiro Yoshida. Its source
repository is at sourceforge. It seems that packcc at sourceforge is
not actively maintained. Some derived repositories are at
github. Currently, our choice is https://github.com/enechaev/packcc.
It is extended to support unicode.

The source tree of packcc is grafted at misc/packcc directory.
Building packcc and ctags are integrated in the build-scripts of
Universal-ctags.

If misc/packcc directory is empty, run following command to get
source code before building ctags:
```
$ git submodule init misc/packcc
$ git submodule update misc/packcc
```

About the suffix rule about .c and .peg, @b4n and @mqudsi help me
on universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Feb 3, 2019
Packcc is a compiler-compiler; it translates .peg grammar file to .c
file.  packcc was originally written by Arihiro Yoshida. Its source
repository is at sourceforge. It seems that packcc at sourceforge is
not actively maintained. Some derived repositories are at
github. Currently, our choice is https://github.com/enechaev/packcc.
It is extended to support unicode.

The source tree of packcc is grafted at misc/packcc directory.
Building packcc and ctags are integrated in the build-scripts of
Universal-ctags.

If misc/packcc directory is empty, run following command to get
source code before building ctags:
```
$ git submodule init misc/packcc
$ git submodule update misc/packcc
```

About the suffix rule about .c and .peg, @b4n and @mqudsi help me
on universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Feb 11, 2019
Packcc is a compiler-compiler; it translates .peg grammar file to .c
file.  packcc was originally written by Arihiro Yoshida. Its source
repository is at sourceforge. It seems that packcc at sourceforge is
not actively maintained. Some derived repositories are at
github. I put our version to https://github.com/universal-ctags/packcc,
wihch was forked from https://github.com/enechaev/packcc.
The reason I chose enechaev's repo is that the one was extended to
support unicode.

The source tree of packcc is grafted at misc/packcc directory.
Building packcc and ctags are integrated in the build-scripts of
Universal-ctags.

If misc/packcc directory is empty, run following command to get
source code before building ctags:
```
$ git submodule init misc/packcc
$ git submodule update misc/packcc
```

About the suffix rule about .c and .peg, @b4n and @mqudsi help me
on universal-ctags#1554.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
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.

5 participants