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

Move all parsers C code to parsers directory #286

Closed
wants to merge 2 commits into from

Conversation

masatake
Copy link
Member

This is the initial step for solving #63.
All concrete parsers are moved to parsers directory.

This directory structure may help up to merge/share the code with geany.

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

ffes commented Apr 16, 2015

In general I like the idea, but the PR as it is right now completely break all compiling on Windows.

It becomes a long command to type just to compile using Cygwin or cross-compile on Linux, but that is not the big issue. Both make variants can't find the files it needs:

$ make -f makefiles/mk_mingw.mak CC=i686-w64-mingw32-gcc
makefiles/mk_mingw.mak:3: source.mak: No such file or directory
make: *** No rule to make target 'source.mak'.  Stop.

Compiling with MSVC command line compiler doesn't work for basically the same reason.

C:\Tmp\masatake-ctags>nmake -f makefiles\mk_mvc.mak

Microsoft (R) Program Maintenance Utility Version 12.00.21005.1
Copyright (C) Microsoft Corporation.  All rights reserved.

NMAKE : fatal error U1073: don't know how to make 'ada.c'
Stop.

And the Visual Studio project files needs to be patched as well.

What do you suggest to solve these issues?

A patch for the project files is a bit of work, but easy since that is complete independent from the makefiles.

A patch for the mk_mingw.mak is not difficult as well. It uses GNU make, so VPATH can be used as well.

Have to think about mk_mvc.mak though. nmake has no support for VPATH and adjusting sources.mak would break all the other builds.

@ffes
Copy link
Member

ffes commented Apr 16, 2015

And we I will try to create a patch for travis as well. But for that mk_mingw.mak needs to work properly again.

@ffes
Copy link
Member

ffes commented Apr 16, 2015

I've created a patch for mk_mingw.mak. What is the easiest way to get this into your PR? Should I e-mail you the output of git format-patch?

@ffes
Copy link
Member

ffes commented Apr 16, 2015

About building with MSVC... I think we should remove mk_mvc.mak.

It appears to be quite a job to get that to work with this new directory structure. And when the Visual Studio project files are updated msbuild can be used just as easy. See windows.rst line 45 for more about MSBuild.

@ffes
Copy link
Member

ffes commented Apr 16, 2015

And did you just oversee rust.c and perl.c or are they not move with a reason?

@ffes
Copy link
Member

ffes commented Apr 16, 2015

Patches for Visual Studio project files and travis are ready as well.

@b4n
Copy link
Member

b4n commented Apr 16, 2015

What do you suggest to solve these issues?

IMO we should just properly prefix all relevant entries in source.mak with parsers/

@b4n
Copy link
Member

b4n commented Apr 16, 2015

IMO we should just properly prefix all relevant entries in source.mak with parsers/

…which is a little more subtle than what I initially thought as the current build rule in Makefile.in always generates the .os in the builddir's root. But should be quite easy to fix slightly altering the .c.o rule.

@b4n
Copy link
Member

b4n commented Apr 16, 2015

This works (given perl.c and rust.c are moved into parsers/)

diff --git a/Makefile.in b/Makefile.in
index 9efeae6..37764c8 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -27,6 +27,7 @@ pkgconfdir=@sysconfdir@/@PACKAGE_NAME@
 libexecdir=@libexecdir@
 pkglibexecdir=@libexecdir@/@PACKAGE_NAME@
 SLINK  = @LN_S@
+MKDIR_P = @MKDIR_P@
 STRIP  = @STRIP@
 CC = @CC@
 DEFS   = @DEFS@
@@ -68,7 +69,7 @@ include $(srcdir)/makefiles/source.mak
 .SUFFIXES:
 .SUFFIXES: .c .$(OBJEXT)

-VPATH  = $(srcdir) $(srcdir)/parsers $(srcdir)/data/corpora \
+VPATH  = $(srcdir) $(srcdir)/data/corpora \
    $(srcdir)/data/optlib $(srcdir)/gnu_regex $(srcdir)/fnmatch

 INSTALL        = cp -p
@@ -347,7 +348,8 @@ maintainerclean: distclean
 # implicit rules
 #
 .c.$(OBJEXT):
-   $(CC) -I. -I$(srcdir) $(DEFS) $(ALL_CPPFLAGS) $(ALL_CFLAGS) -c $<
+   dir="$$(dirname $@)"; [ -d "$$dir" ] || $(MKDIR_P) "$$dir"
+   $(CC) -I. -I$(srcdir) $(DEFS) $(ALL_CPPFLAGS) $(ALL_CFLAGS) -o $@ -c $<
 $(addsuffix .$(OBJEXT),$(basename $(notdir $(REGEX_SOURCES)))): ALL_CPPFLAGS += \
    -D_GNU_SOURCE -D__USE_GNU -Dbool=int -Dfalse=0 -Dtrue=1

diff --git a/configure.ac b/configure.ac
index 28ad6da..8544fb3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -263,6 +263,7 @@ case `uname` in
 esac

 AC_PROG_LN_S
+AC_PROG_MKDIR_P
 AC_CHECK_PROG(STRIP, strip, strip, :)
 AC_SYS_LARGEFILE

diff --git a/makefiles/source.mak b/makefiles/source.mak
index 5f3e19b..ef8702e 100644
--- a/makefiles/source.mak
+++ b/makefiles/source.mak
@@ -5,72 +5,75 @@ HEADERS = \
    main.h options.h parse.h parsers.h pcoproc.h read.h routines.h sort.h \
    strlist.h trashbox.h vstring.h

+PARSER_SOURCES = \
+   parsers/ada.c \
+   parsers/ant.c \
+   parsers/asm.c \
+   parsers/asp.c \
+   parsers/awk.c \
+   parsers/basic.c \
+   parsers/beta.c \
+   parsers/c.c \
+   parsers/clojure.c \
+   parsers/css.c \
+   parsers/cobol.c \
+   parsers/dosbatch.c \
+   parsers/eiffel.c \
+   parsers/erlang.c \
+   parsers/falcon.c \
+   parsers/flex.c \
+   parsers/fortran.c \
+   parsers/go.c \
+   parsers/html.c \
+   parsers/jscript.c \
+   parsers/json.c \
+   parsers/lisp.c \
+   parsers/lua.c \
+   parsers/make.c \
+   parsers/matlab.c \
+   parsers/objc.c \
+   parsers/ocaml.c \
+   parsers/pascal.c \
+   parsers/perl.c \
+   parsers/php.c \
+   parsers/python.c \
+   parsers/rexx.c \
+   parsers/ruby.c \
+   parsers/rust.c \
+   parsers/scheme.c \
+   parsers/sh.c \
+   parsers/slang.c \
+   parsers/sml.c \
+   parsers/sql.c \
+   parsers/tcl.c \
+   parsers/tex.c \
+   parsers/verilog.c \
+   parsers/vhdl.c \
+   parsers/vim.c \
+   parsers/windres.c \
+   parsers/yacc.c
+
 SOURCES = \
-   ada.c \
    args.c \
-   ant.c \
-   asm.c \
-   asp.c \
-   awk.c \
-   basic.c \
-   beta.c \
-   c.c \
-   clojure.c \
-   css.c \
-   cobol.c \
-   dosbatch.c \
-   eiffel.c \
    entry.c \
-   erlang.c \
-   falcon.c \
    flags.c \
-   flex.c \
-   fortran.c \
    get.c \
-   go.c \
-   html.c \
    htable.c \
-   jscript.c \
-   json.c \
    keyword.c \
-   lisp.c \
    lregex.c \
-   lua.c \
    lxcmd.c \
    main.c \
-   make.c \
-   matlab.c \
-   objc.c \
-   ocaml.c \
    options.c \
    parse.c \
-   pascal.c \
    pcoproc.c \
-   perl.c \
-   php.c \
-   python.c \
    read.c \
-   rexx.c \
    routines.c \
-   ruby.c \
-   rust.c \
-   scheme.c \
-   sh.c \
-   slang.c \
-   sml.c \
    sort.c \
-   sql.c \
    strlist.c \
-   tcl.c \
-   tex.c \
    tg.c \
    trashbox.c \
-   verilog.c \
-   vhdl.c \
-   vim.c \
-   windres.c \
-   yacc.c \
-   vstring.c
+   vstring.c \
+   $(PARSER_SOURCES)

 ENVIRONMENT_HEADERS = e_msoft.h

@masatake
Copy link
Member Author

Thank you for comments. I should have more tests.
I pushed a bit updated version(adding perl.c and rust.c) to sepdir branch of fishman/ctags.
Temporary, I dropped a patch related to makefiles directory to make previews simple.

@ffes, @b4n, could directory push your code about this topic to the branch?
After improving at the branch, we can merge it to master.

@ffes
Copy link
Member

ffes commented Apr 22, 2015

I just pushed my commits to the fishman/sepdir branch.

It builds again with mk_mingw.mak and with Visual Studio IDE and MSBuild.
And as a result Travis works again as well.

@techee
Copy link
Contributor

techee commented Apr 22, 2015

Maybe a suggestion - it would be nice to have also the rest of the sources outside the root (having them mixed with all the READMEs etc. is a bit messy and hard to find the actual source file). One possibility would be having directories

src - non-parser sources would be here
src/parsers - parsers subdirectory for parsers

or something similar.

@masatake
Copy link
Member Author

@ffes, thank you. I will merge sepdir branch of fishman/ctags to master.
@techee, thank you for comment. It is next step.
This is small but important change for merging the code base with geany.

@masatake
Copy link
Member Author

Pushed to master.
If you got a trouble, please open another issue.

@masatake masatake closed this Apr 23, 2015
@masatake masatake deleted the sepdir branch April 23, 2015 03:52
@masatake
Copy link
Member Author

@techee, I agree.
The output of ls at ctags source dir is still too long.
In stead of src, I will use the name core.
So we can reuse the same technique used in this thread.
@ffes, I will open sepdir2 branch and ask you to work related to windows, again.

@techee
Copy link
Contributor

techee commented Apr 23, 2015

@masatake Thanks, sounds good to me.

@masatake
Copy link
Member Author

I will apply the patch submitted by @b4n after merging sepdir-core branch.
Of course you can commit directly to the branch.
But I will not do it to the branch to avoid making you confused.

masatake added a commit that referenced this pull request Apr 27, 2015
Though *.c files are moved to subdirectories, *.o files are generated
on the top src directory when running make.

This patch is mostly based on @b4n's comment at #286 for fixing the
above issue.

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.

4 participants