Add support for the YouCompleteMe vim plugin

RESOLVED FIXED in Firefox 39

Status

Firefox Build System
Mach Core
RESOLVED FIXED
5 years ago
5 months ago

People

(Reporter: BenWa, Assigned: Ehsan)

Tracking

Trunk
mozilla40
x86
Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(firefox39 fixed, firefox40 fixed, firefox-esr38 fixed)

Details

Attachments

(2 attachments, 7 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 774635 [details] [diff] [review]
patch

YCM pretty much supersedes clang-complete. Here's a mach command to generate the flags.

I found that showbuild doesn't give us the proper build flags for LIBRARY_LIBXUL=1 code which is the majority of gecko so I've hardcoded what I though was needed.

I tested the completion results and they work very well inside gfx and layout. We still get errors because of VPATH, LOCAL_INCLUDE and local defines but this is a good start.
Attachment #774635 - Flags: review?(gps)
It seems wrong to do that in mach.
Comment on attachment 774635 [details] [diff] [review]
patch

Review of attachment 774635 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/mach_commands.py
@@ +602,5 @@
> +        print('\'-I%s/ipc/chromium/src\',' % self.topsrcdir)
> +        print('\'-I%s/ipc/glue\',' % self.topsrcdir)
> +        print('\'-I%s/ipc/ipdl/_ipdlheaders\',' % self.topobjdir)
> +        # VPATH makes include more difficult,
> +        print('\'-I%s/gfx/layers\',' % self.topsrcdir)

especially this is wrong on so many levels...
(Reporter)

Comment 3

5 years ago
(In reply to Mike Hommey [:glandium] from comment #1)
> It seems wrong to do that in mach.

Can you elaborate what is wrong and why?

(In reply to Mike Hommey [:glandium] from comment #2)
> especially this is wrong on so many levels...

I'd like to get the arguments for each file but until then I think working around these issues is the most practical thing to do for now.

Comment 4

5 years ago
Comment on attachment 774635 [details] [diff] [review]
patch

Review of attachment 774635 [details] [diff] [review]:
-----------------------------------------------------------------

Looking at the YCM docs, I /think/ it supports per-directory config files. If so, we could do this right and write out a config file in each source directory. That would give us correct variables and address some of glandium's concerns.

Now that we have two mach commands doing similar things, I'd like the common code to be moved into a common function, preferably outside of mach_commands.py. The original idea of mach_command.py is that they should be simple proxies to other "library" code. This facilitates code reuse and testing. The idea is for code to land in mach_commands.py and then graduate to core libraries as it matures. Or, if code is obviously the domain of another system (e.g. core build code), then it should definitely not be in mach_commands.py. This principle has been violated a lot (including by me), so I hope I don't come across as hypocritical when I ask that you do it right.

I think all I'm asking for is a basic helper function somewhere that can read the output of |make showbuild| and possibly combine this with data from moz.build files. If you go with the multiple config file approach, you could probably implement a custom "backend" that consumes the moz.build traversal output and writes out individual ycm config files for each directory containing source files. We don't currently have an easy way of plugging into an arbitrary build backend. I could build that for you if you ask. Or you could make a one-off pretty easily based on the code in build/ConfigStatus.py (search for BuildReader).

::: python/mozbuild/mozbuild/mach_commands.py
@@ +527,5 @@
>          return exit_code
>  
>  @CommandProvider
>  class ClangCommands(MachCommandBase):
> +    @Command('ycm_extra_config', category='devenv',

Nit: We typically use hyphens in command names.

@@ +544,5 @@
> +            build_vars[elements[0]] = elements[1]
> +
> +        try:
> +            old_logger = self.log_manager.replace_terminal_handler(None)
> +            self._run_make(target='showbuild', log=False, line_handler=on_line)

Python has access to the raw data in config.status. I wish we could obtain the important bits from there. Unfortunately, I bet config.mk and friends are defining some variables that you need.

@@ +603,5 @@
> +        print('\'-I%s/ipc/glue\',' % self.topsrcdir)
> +        print('\'-I%s/ipc/ipdl/_ipdlheaders\',' % self.topobjdir)
> +        # VPATH makes include more difficult,
> +        print('\'-I%s/gfx/layers\',' % self.topsrcdir)
> +        print(']')

Since you are printing code that will be interpreted by a Python interpreter, you can use the pprint module (http://docs.python.org/2/library/pprint.html).

::: python/mozbuild/mozbuild/ycm_extra_config_base.txt
@@ +1,1 @@
> +import os

Needs moar MPL.

@@ +3,5 @@
> +
> +compilation_database_folder = ''
> +
> +if compilation_database_folder:
> +  database = ycm_core.CompilationDatabase( compilation_database_folder )

4 space indent, please.

I recommend running flake8 (https://bitbucket.org/tarek/flake8/wiki/Home) on Python code to help enforce good style.
Attachment #774635 - Flags: review?(gps) → feedback+
(Reporter)

Comment 5

5 years ago
(In reply to Gregory Szorc [:gps] from comment #4)
> Comment on attachment 774635 [details] [diff] [review]
> patch
> 
> Review of attachment 774635 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looking at the YCM docs, I /think/ it supports per-directory config files.
> If so, we could do this right and write out a config file in each source
> directory. That would give us correct variables and address some of
> glandium's concerns.

The configuration is a python script so we can even have a single YCM configuration script that prepare the build flags on demand.

> 
> Now that we have two mach commands doing similar things, I'd like the common
> code to be moved into a common function, preferably outside of
> mach_commands.py. The original idea of mach_command.py is that they should
> be simple proxies to other "library" code. This facilitates code reuse and
> testing. The idea is for code to land in mach_commands.py and then graduate
> to core libraries as it matures. Or, if code is obviously the domain of
> another system (e.g. core build code), then it should definitely not be in
> mach_commands.py. This principle has been violated a lot (including by me),
> so I hope I don't come across as hypocritical when I ask that you do it
> right.

No that's fine. I was considering doing that while writing the code. The problem is making changes to clang-complete is difficult because we don't have anything to test the changes.

> 
> I think all I'm asking for is a basic helper function somewhere that can
> read the output of |make showbuild| and possibly combine this with data from
> moz.build files. If you go with the multiple config file approach, you could
> probably implement a custom "backend" that consumes the moz.build traversal
> output and writes out individual ycm config files for each directory
> containing source files. We don't currently have an easy way of plugging
> into an arbitrary build backend. I could build that for you if you ask.

Even if we had that moz.build doesn't currently include all the information yet to figure out the build flags
without running Make. We still rely on config.mk rules.mk and many unconverted variable such as VPATH, LIBXUL_LIBRARY, DEFINES, LOCAL_INCLUDE. Some of these will be converted soon but until moz.build can compute the build flag for each flag I don't think it's an option. I think for now that means we're stuck getting this information via make unless I'm missing something.

make showbuild only give us the general build flag, even from sub directories. Perhaps that could be fixed? If that is fixed there is still the problem of finding which objdir/.../Makefile to use for srcdir/.../Foo.cpp.

We could always just process the build log to get the build flags per file like the warnings as well but that really sucks.

If we can't come up with something that can done within a few man days I think we should take the patch like this + review comments. It works really well in practice. There's no harm in specifying include paths that aren't required for others files other then missing compile warnings. If that's really a problem I can remove it from the patch the results are still good without that hack.

Comment 6

5 years ago
I'm sympathetic to the "perfect is the enemy of done" arguments you are making, especially since doing it right would require a lot of work in the core build system.

Part of the advantage of having code outside of mach_commands.py is that it is easily testable. See the mozbuild.test.* modules. These get run as part of |make check| (or |mach python-check|). I highly encourage you to move the common code outside of mach_commands.py and add basic tests. Ehsan knows the clang complete code and can likely provide assistance.
(Reporter)

Comment 7

5 years ago
Created attachment 775986 [details] [diff] [review]
patch

So apparently when I checked if showbuild return the right flags for the subdir I checked wrong. It does work.

Here's a very simple patch that works everywhere I tried without any hacks. Since my mach command is only a few lines I'm not going to move it out. We can probably remove clang_complete command and get a net win in simplicity as well.
Assignee: nobody → bgirard
Attachment #774635 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #775986 - Flags: review?(gps)
(Reporter)

Comment 8

5 years ago
Created attachment 775987 [details] [diff] [review]
patch

Proper patch this time
Attachment #775986 - Attachment is obsolete: true
Attachment #775986 - Flags: review?(gps)
Attachment #775987 - Flags: review?(gps)
(Reporter)

Comment 9

5 years ago
Created attachment 775989 [details] [diff] [review]
patch

Ohh forgot to remove my paths
Attachment #775987 - Attachment is obsolete: true
Attachment #775987 - Flags: review?(gps)
Attachment #775989 - Flags: review?(gps)
Comment on attachment 775989 [details] [diff] [review]
patch

Review of attachment 775989 [details] [diff] [review]:
-----------------------------------------------------------------

You don't need to add a mach command to generate a ycm config. Just checkin a ycm config that calls showbuild appropriately.
Attachment #775989 - Flags: review?(gps) → review-
(Reporter)

Comment 11

5 years ago
The problem is that the ycm config shouldn't live in the root of the tree but in my case it needs to live one directory up (shared between my object directory and tree). The ycm config needs to have absolute paths to the objdir/srcdir and so when jumping between different mozconfig/objdir it requires hand editing. AFAIK just checking in a ycm config file in the tree make this manual and error prone when you can simply do 'mac ycm > ../.ycm_extra_conf'.
(In reply to Benoit Girard (:BenWa) from comment #11)
> The problem is that the ycm config shouldn't live in the root of the tree
> but in my case it needs to live one directory up (shared between my object
> directory and tree).

Why would you need to share between the objdir and the source tree? Are you editing source files in the objdir? Why?

> The ycm config needs to have absolute paths to the
> objdir/srcdir and so when jumping between different mozconfig/objdir it
> requires hand editing. AFAIK just checking in a ycm config file in the tree
> make this manual and error prone when you can simply do 'mac ycm >
> ../.ycm_extra_conf'.

The ycm config file is python code. It can use the same logic as mach. In fact, it can import the logic from mach (provided a small change to mach). That is, there is no need for a hardcoded objdir and srcdir in the ycm config file.
Also note the MakeRelativePathsInFlagsAbsolute function is used wrongly, it considers the relative paths relative to the path containing the ycm config, not relative to the path containing the source file. Anyways, I have a WIP patch that does what i think should be done.
(Reporter)

Comment 14

5 years ago
My tree structure is the following:
mozilla/<tree name>/.ycm_extra_config.py
mozilla/<tree name>/tree
mozilla/<tree name>/builds/objdir-opt64
mozilla/<tree name>/builds/objdir-debugt64
mozilla/<tree name>/builds/etc...

When I jump to a definition in a header file in mozilla/<tree name>/builds/objdir-opt64/dist/include/*.h the ycm no longer applies to that directory if it had been place to my srcdir and from there I can't make any more jumps.
Created attachment 776144 [details] [diff] [review]
Add support for the YouCompleteMe vim plugin

This works for me in both the srcdir and the objdir, although it's far from using the right flags everywhere. Note it would be better if it was possible to get the data out of mach directly instead of having to parse it again, but that can be handled later. It should work well enough without.
Attachment #776144 - Flags: review?(gps)
Assignee: bgirard → mh+mozilla
Summary: Add ycm_extra_config command → Add support for the YouCompleteMe vim plugin
Attachment #775989 - Attachment is obsolete: true
Comment on attachment 776144 [details] [diff] [review]
Add support for the YouCompleteMe vim plugin

I would like the two patch writers to agree on a plan of attack before I look at the mach and build bits. Otherwise, it kinda/sorta feels like I'm picking sides and I don't want to do that.
Attachment #776144 - Flags: review?(bgirard)
(Reporter)

Comment 17

5 years ago
Comment on attachment 776144 [details] [diff] [review]
Add support for the YouCompleteMe vim plugin

Review of attachment 776144 [details] [diff] [review]:
-----------------------------------------------------------------

Ok this addresses my concern. Much better then what I had.

::: .ycm_extra_conf.py
@@ +21,5 @@
> +    out = StringIO()
> +    out.encoding = None
> +    mach.run(['compileflags', filename], stdout=out, stderr=out)
> +
> +    with open("/tmp/log", "a") as f:

This is left over from debugging right?

In case you didn't know you can use ':YcmDiag' and ':YcmDebugInfo'.

::: python/mozbuild/mozbuild/mach_commands.py
@@ +623,5 @@
> +        finally:
> +            self.log_manager.replace_terminal_handler(old_logger)
> +
> +        if what.endswith('.c'):
> +            name = 'COMPILE_CFLAGS'

Nice. I was meaning to add something like that as well.

@@ +630,5 @@
> +
> +        if name not in build_vars:
> +            return
> +
> +        flags = ['-isystem', '-I', '-include', '-MF']

FYI I've had trouble with the -MF argument but maybe it's because something else was wrong. I decided to strip it since it added no value to competition results.
Attachment #776144 - Flags: review?(bgirard) → review+
Created attachment 776189 [details] [diff] [review]
Add support for the YouCompleteMe vim plugin
Attachment #776189 - Flags: review?(gps)
Attachment #776144 - Attachment is obsolete: true
Attachment #776144 - Flags: review?(gps)
(In reply to Benoit Girard (:BenWa) from comment #17)
> Comment on attachment 776144 [details] [diff] [review]
> Add support for the YouCompleteMe vim plugin
> 
> Review of attachment 776144 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Ok this addresses my concern. Much better then what I had.
> 
> ::: .ycm_extra_conf.py
> @@ +21,5 @@
> > +    out = StringIO()
> > +    out.encoding = None
> > +    mach.run(['compileflags', filename], stdout=out, stderr=out)
> > +
> > +    with open("/tmp/log", "a") as f:
> 
> This is left over from debugging right?

Oops

> In case you didn't know you can use ':YcmDiag' and ':YcmDebugInfo'.

I found it afterwards.

> ::: python/mozbuild/mozbuild/mach_commands.py
> @@ +623,5 @@
> > +        finally:
> > +            self.log_manager.replace_terminal_handler(old_logger)
> > +
> > +        if what.endswith('.c'):
> > +            name = 'COMPILE_CFLAGS'
> 
> Nice. I was meaning to add something like that as well.
> 
> @@ +630,5 @@
> > +
> > +        if name not in build_vars:
> > +            return
> > +
> > +        flags = ['-isystem', '-I', '-include', '-MF']
> 
> FYI I've had trouble with the -MF argument but maybe it's because something
> else was wrong. I decided to strip it since it added no value to competition
> results.

I see errors in ycmdiag because of it after a simple make export because the .deps directory doesn't exist. But it apparently doesn't impact ycm itself. I think such argument filtering should be done on ycm's end, though (in .ycm_extra_conf.py, or preferably in ycm itself).
Comment on attachment 776189 [details] [diff] [review]
Add support for the YouCompleteMe vim plugin

Review of attachment 776189 [details] [diff] [review]:
-----------------------------------------------------------------

The code looks mostly good. I'd like questions answered before granting r+.

I'm guessing someone will complain that we're polluting the root directory with a file that won't be used by many people. This is a slippery slope argument, but what happens when we have 5 or 10 similar files in the root directory? Is that acceptable? Alternatively, we could have a mach command that writes out .ycm_extra_conf.py and we could add this file to the version control ignore list.

I wish we could do this patch without touching the main mach file. The reason is a lot of people have copied this file to a directory on PATH and we currently have no way of detecting when the source-controlled file varies from the copied file in PATH. Although, we could do this in build/mach_bootstrap.py in a follow-up bug, so it's not blocking r+.

::: .ycm_extra_conf.py
@@ +9,5 @@
> +import shlex
> +
> +path = os.path.join(os.path.dirname(__file__), 'mach')
> +
> +if not os.path.exists(path):

When will this branch be taken? If this file is in the root directory, there will *always* be a mach file, no?

@@ +12,5 @@
> +
> +if not os.path.exists(path):
> +    path = os.path.join(os.path.dirname(__file__), 'config.status')
> +    config = imp.load_module('_buildconfig', open(path), path, ('', 'r', imp.PY_SOURCE))
> +    path = os.path.join(config.topsrcdir, 'mach')

We have this code in mozbuild IIRC. However, that would necessitate sys.path munging from this file. Am I correct in assuming that .ycm_extra_conf.py is invoked by vim and we have no or little control over the sys.path or Python interpreter used?

@@ +13,5 @@
> +if not os.path.exists(path):
> +    path = os.path.join(os.path.dirname(__file__), 'config.status')
> +    config = imp.load_module('_buildconfig', open(path), path, ('', 'r', imp.PY_SOURCE))
> +    path = os.path.join(config.topsrcdir, 'mach')
> +mach_module = imp.load_module('_mach', open(path), path, ('', 'r', imp.PY_SOURCE))

Can we not load build/mach_bootstrap.py instead? The main mach script really just gets us searching for the topsrcdir, which I /think/ is a solved problem.

::: python/mozbuild/mozbuild/mach_commands.py
@@ +586,5 @@
> +    @Command('compileflags', category='devenv',
> +        description='Display the compilation flags for a given source file')
> +    @CommandArgument('what', default=None,
> +        help='Source file to display compilation flags for')
> +    def compileflags(self, what):

I'd feel much better if most of the logic in this function were in mozbuild, not in the body of a mach command.

@@ +633,5 @@
> +
> +        flags = ['-isystem', '-I', '-include', '-MF']
> +        new_args = []
> +        path = os.path.join(self.topobjdir, make_dir)
> +        for arg in shlex.split(build_vars[name]):

Can you please document via comments what's going on here - it's a bit difficult to follow.
Attachment #776189 - Flags: review?(gps) → feedback+
(In reply to Gregory Szorc [:gps] from comment #20)
> Comment on attachment 776189 [details] [diff] [review]
> Add support for the YouCompleteMe vim plugin
> 
> Review of attachment 776189 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The code looks mostly good. I'd like questions answered before granting r+.
> 
> I'm guessing someone will complain that we're polluting the root directory
> with a file that won't be used by many people. This is a slippery slope
> argument, but what happens when we have 5 or 10 similar files in the root
> directory? Is that acceptable? Alternatively, we could have a mach command
> that writes out .ycm_extra_conf.py and we could add this file to the version
> control ignore list.

I think that's the most horrible thing to do.

> I wish we could do this patch without touching the main mach file. The
> reason is a lot of people have copied this file to a directory on PATH and
> we currently have no way of detecting when the source-controlled file varies
> from the copied file in PATH. Although, we could do this in
> build/mach_bootstrap.py in a follow-up bug, so it's not blocking r+.

For this particular change, it actually doesn't matter that people may have copied the older version somewhere else. They both do the same thing, and it doesn't matter that the copies are stil on the older version.

> ::: .ycm_extra_conf.py
> @@ +9,5 @@
> > +import shlex
> > +
> > +path = os.path.join(os.path.dirname(__file__), 'mach')
> > +
> > +if not os.path.exists(path):
> 
> When will this branch be taken? If this file is in the root directory, there
> will *always* be a mach file, no?

There's no mach file in the objdir (the file is copied at the root of the objdir, which is necessary when the objdir is not below the source dir)

> @@ +12,5 @@
> > +
> > +if not os.path.exists(path):
> > +    path = os.path.join(os.path.dirname(__file__), 'config.status')
> > +    config = imp.load_module('_buildconfig', open(path), path, ('', 'r', imp.PY_SOURCE))
> > +    path = os.path.join(config.topsrcdir, 'mach')
> 
> We have this code in mozbuild IIRC. However, that would necessitate sys.path
> munging from this file. Am I correct in assuming that .ycm_extra_conf.py is
> invoked by vim and we have no or little control over the sys.path or Python
> interpreter used?

Indeed
 
> @@ +13,5 @@
> > +if not os.path.exists(path):
> > +    path = os.path.join(os.path.dirname(__file__), 'config.status')
> > +    config = imp.load_module('_buildconfig', open(path), path, ('', 'r', imp.PY_SOURCE))
> > +    path = os.path.join(config.topsrcdir, 'mach')
> > +mach_module = imp.load_module('_mach', open(path), path, ('', 'r', imp.PY_SOURCE))
> 
> Can we not load build/mach_bootstrap.py instead? The main mach script really
> just gets us searching for the topsrcdir, which I /think/ is a solved
> problem.

I didn't want to copy 90% of the mach code, which is essentialy why i changed the mach script.

> ::: python/mozbuild/mozbuild/mach_commands.py
> @@ +586,5 @@
> > +    @Command('compileflags', category='devenv',
> > +        description='Display the compilation flags for a given source file')
> > +    @CommandArgument('what', default=None,
> > +        help='Source file to display compilation flags for')
> > +    def compileflags(self, what):
> 
> I'd feel much better if most of the logic in this function were in mozbuild,
> not in the body of a mach command.

Where?
Flags: needinfo?(gps)
There is a mozbuild.compilation sub-package - I'd create a new module there. mozbuild.compilation.codecomplete? We can always refactor things later if the name doesn't work forever.
Flags: needinfo?(gps)
(Reporter)

Comment 23

5 years ago
Are you still interested in finishing this glandium? This approach provides very accurate vim completion. I don't have any attachment to YouCompleteMe in particular, but I'd like to see completion support for any decent clang complete vim plugin.
(Assignee)

Comment 24

4 years ago
(In reply to Benoit Girard (:BenWa) from comment #23)
> Are you still interested in finishing this glandium? This approach provides
> very accurate vim completion. I don't have any attachment to YouCompleteMe
> in particular, but I'd like to see completion support for any decent clang
> complete vim plugin.

Mike?
Flags: needinfo?(mh+mozilla)
I won't be working on this in the immediate future. Feel free to take it and give it a refresh. There shouldn't be much left to do.
Assignee: mh+mozilla → nobody
Flags: needinfo?(mh+mozilla)
(Assignee)

Comment 26

3 years ago
I addressed the rest of gps' comments and got a new patch to work.
Assignee: nobody → ehsan
(Assignee)

Comment 27

3 years ago
Created attachment 8597638 [details] [diff] [review]
Add support for the YouCompleteMe vim plugin
Attachment #776189 - Attachment is obsolete: true
Attachment #8597638 - Flags: review?(gps)
Comment on attachment 8597638 [details] [diff] [review]
Add support for the YouCompleteMe vim plugin

Review of attachment 8597638 [details] [diff] [review]:
-----------------------------------------------------------------

I'll r+ this, but I'm not a huge fan of introducing a mach command just to be used for .ycm_extra_conf.py. Unless you have others consumers in mind, I would have added a function somewhere in python/mozbuild and made .ycm_extra_conf.py do a simple function call instead of a full mach dispatch. That being said, getting a full build system context from an arbitrary Python script isn't the easiest thing in the world. Although, if you launch said Python script with the Python interpreter from the virtualenv (objdir/_virtualenv/bin/python or objdir/_virtualenv/Scripts/python.exe), this work is done for you.
Attachment #8597638 - Flags: review?(gps) → review+
(Assignee)

Comment 29

3 years ago
(In reply to Gregory Szorc [:gps] from comment #28)
> Comment on attachment 8597638 [details] [diff] [review]
> Add support for the YouCompleteMe vim plugin
> 
> Review of attachment 8597638 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'll r+ this, but I'm not a huge fan of introducing a mach command just to
> be used for .ycm_extra_conf.py. Unless you have others consumers in mind, I
> would have added a function somewhere in python/mozbuild and made
> .ycm_extra_conf.py do a simple function call instead of a full mach
> dispatch. That being said, getting a full build system context from an
> arbitrary Python script isn't the easiest thing in the world. Although, if
> you launch said Python script with the Python interpreter from the
> virtualenv (objdir/_virtualenv/bin/python or
> objdir/_virtualenv/Scripts/python.exe), this work is done for you.

As glandium said in comment 21, we cannot control what Python is used to load that script, since it is loaded via YCM.  If you have other ideas on how to use a simple function, please file a follow-up and explain what else I can do.  Thanks!
https://hg.mozilla.org/mozilla-central/rev/1bfcd43acd3c
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Argh. This broke my local YCM installation. =(
Scratch that. Updating to the latest version fixed it. I suppose this .ycm_extra_conf.py tickled some bug in the particular version I had.
(Assignee)

Comment 34

3 years ago
(In reply to Seth Fowler [:seth] from comment #33)
> Scratch that. Updating to the latest version fixed it. I suppose this
> .ycm_extra_conf.py tickled some bug in the particular version I had.

Let me know if something doesn't work for you with this.  This should give you pretty good YCM support, except that it's not aware of unified builds yet -- I'm not quite sure how to solve that!
(Reporter)

Comment 37

3 years ago
Created attachment 8632277 [details]
Tentative setup instructions for OS X (See comment)

I got this working today but it was a bit tedious. The YCM instruction were difficult to navigate. See the attachment for what I did.

I'd like others to help iron out a better set of instruction before we tell more users to use it.
(Reporter)

Comment 38

3 years ago
Created attachment 8632319 [details]
Tentative setup instructions for OS X & Ubuntu
Attachment #8632277 - Attachment is obsolete: true
(Assignee)

Comment 39

3 years ago
(In reply to Benoit Girard (:BenWa) from comment #38)
> Created attachment 8632319 [details]
> Tentative setup instructions for OS X & Ubuntu

This worked well for me on OS X, except that I used the `./install.sh --clang-completer` command in order to build YouCompleteMe.
I'm trying to get YCM work with my gecko under b2g source tree.
Objdir is located in B2G/objdir-gecko, can anyone point me where I can configure objdir for YCM?

It doesn't load 'objdir-gecko', but i tried regular 'mach build', it works, it can load obj-x86_64-unknown-linux-gnu.
(Reporter)

Comment 42

3 years ago
You're going to want to file a new bug for this. But you can find the file at '.ycm_extra_conf.py'. You will need to teach it how to call ./mach compileflags while finding the right configuration.
Hi! Just tried setting up YouCompleteMe with VIM, but it looks like I get lots of errors. Is there something that's changed in the meantime? I tried on OS X and on Linux, so I wanted to see if it still worked for others.
(Reporter)

Comment 44

2 years ago
Try following the tips here and showing your diagnostics. Might be worth filing a bug with more details. Looks like my setup might be broken but I have a lot of unusual stuff in my environment.
Hi Benoit, thanks for the pointer. I followed the :YcmDiag tip and I ended up getting a lot of errors related to C++:

  1 /Users/jchan/projects/mozilla-central/mfbt/Char16.h|213 col 15 warning| 'char16_t' is a keyword in C++11
  2 /Users/jchan/projects/mozilla-central/mfbt/Char16.h|213 col 15 error| unknown type name 'char16_t'
  3 /Users/jchan/projects/mozilla-central/mfbt/Char16.h|227 col 1 warning| 'static_assert' is a keyword in C++11
  ...
  9 /Users/jchan/projects/mozilla-central/mfbt/Char16.h|229 col 1 error| C++ requires a type specifier for all declarations
 10 /Users/jchan/projects/mozilla-central/mfbt/Char16.h|229 col 22 error| pasting formed 'u'A'', an invalid preprocessing token

Would you happen to recognize where these might be coming from in particular? If not, I can get more details & file a new bug.
(Reporter)

Comment 47

2 years ago
:YcmDebugInfo should tell you what flags the completion is running with. We receive the flags by invoking the python script .ycm_extra_conf.py at the root of the tree which in turns just calls ./mach compileflags <FILE>. There certainly a big mismatch with your compile flags and what .ycm_extra_conf.py (:YcmDebugInfo) is reporting.
I think I managed to get it to work for me (?)
:YcmDebugInfo gives

    Printing YouCompleteMe debug information...
    -- Server has Clang support compiled in: True
    -- Clang version: clang version 3.8.0 (tags/RELEASE_380/final)
    -- Flags for /Users/jchan/projects/mozilla-central/dom/base/nsGlobalWindow.cpp loaded from /Users/jchan/projects/mozilla-central/.ycm_extra_conf.py:
    -- ['-fvisibility=hidden', '-fvisibility-inlines-hidden', '-DDEBUG=1', '-DTRACING=1', '-DOS_POSIX=1', '-DOS_MACOSX=1', '-DHAVE_SIDEBAR', '-DSTATIC_EXPORTABLE_JS_API', '-DMOZILLA_INTERNAL_A
    PI', '-DIMPL_LIBXUL', '-I/Users/jchan/projects/mozilla-central/dom/base', '-I/Users/jchan/projects/mozilla-central/obj-x86_64-apple-darwin15.5.0/dom/base', '-I/Users/jchan/projects/mozilla
    -central/dom/battery', '-I/Users/jchan/projects/mozilla-central/dom/bluetooth/common', '-I/Users/jchan/projects/mozilla-central/dom/bluetooth/common/webapi', '-I/Users/jchan/projects/mozil
    la-central/dom/events', '-I/Users/jchan/projects/mozilla-central/dom/media', '-I/Users/jchan/projects/mozilla-central/dom/network', '-I/Users/jchan/projects/mozilla-central/dom/time', '-I/
    Users/jchan/projects/mozilla-central/caps', '-I/Users/jchan/projects/mozilla-central/docshell/base', '-I/Users/jchan/projects/mozilla-central/dom/base', '-I/Users/jchan/projects/mozilla-ce
    ntral/dom/geolocation', '-I/Users/jchan/projects/mozilla-central/dom/html', '-I/Users/jchan/projects/mozilla-central/dom/ipc', '-I/Users/jchan/projects/mozilla-central/dom/storage', '-I/Us
    ers/jchan/projects/mozilla-central/dom/svg', '-I/Users/jchan/projects/mozilla-central/dom/u2f', '-I/Users/jchan/projects/mozilla-central/dom/workers', '-I/Users/jchan/projects/mozilla-cent
    ral/dom/xbl', '-I/Users/jchan/projects/mozilla-central/dom/xml', '-I/Users/jchan/projects/mozilla-central/dom/xslt/xpath', '-I/Users/jchan/projects/mozilla-central/dom/xul', '-I/Users/jcha
    n/projects/mozilla-central/gfx/2d', '-I/Users/jchan/projects/mozilla-central/image', '-I/Users/jchan/projects/mozilla-central/js/xpconnect/src', '-I/Users/jchan/projects/mozilla-central/js
    /xpconnect/wrappers', '-I/Users/jchan/projects/mozilla-central/layout/base', '-I/Users/jchan/projects/mozilla-central/layout/forms', '-I/Users/jchan/projects/mozilla-central/layout/generic
    ', '-I/Users/jchan/projects/mozilla-central/layout/style', '-I/Users/jchan/projects/mozilla-central/layout/svg', '-I/Users/jchan/projects/mozilla-central/layout/xul', '-I/Users/jchan/proje
    cts/mozilla-central/netwerk/base', '-I/Users/jchan/projects/mozilla-central/security/manager/ssl', '-I/Users/jchan/projects/mozilla-central/widget', '-I/Users/jchan/projects/mozilla-centra
    l/xpcom/ds', '-I/Users/jchan/projects/mozilla-central/netwerk/sctp/datachannel', '-I/Users/jchan/projects/mozilla-central/obj-x86_64-apple-darwin15.5.0/ipc/ipdl/_ipdlheaders', '-I/Users/jc
    han/projects/mozilla-central/ipc/chromium/src', '-I/Users/jchan/projects/mozilla-central/ipc/glue', '-I/Users/jchan/projects/mozilla-central/obj-x86_64-apple-darwin15.5.0/dist/include', '-
    I/Users/jchan/projects/mozilla-central/obj-x86_64-apple-darwin15.5.0/dist/include/nspr', '-I/Users/jchan/projects/mozilla-central/obj-x86_64-apple-darwin15.5.0/dist/include/nss', '-fPIC',
    '-DMOZILLA_CLIENT', '-include', '/Users/jchan/projects/mozilla-central/obj-x86_64-apple-darwin15.5.0/mozilla-config.h', '-Qunused-arguments', '-Qunused-arguments', '-Wall', '-Wc++11-compat
    ', '-Wempty-body', '-Wignored-qualifiers', '-Woverloaded-virtual', '-Wpointer-arith', '-Wsign-compare', '-Wtype-limits', '-Wunreachable-code', '-Wwrite-strings', '-Wc++11-compat-pedantic',
     '-Wc++14-compat', '-Wc++14-compat-pedantic', '-Wclass-varargs', '-Wimplicit-fallthrough', '-Wloop-analysis', '-Wstring-conversion', '-Wthread-safety', '-Wno-invalid-offsetof', '-Wno-inlin
    e-new-delete', '-Wno-error=deprecated-declarations', '-Wno-error=array-bounds', '-Wno-unknown-warning-option', '-Wno-return-type-c-linkage', '-fno-exceptions', '-fno-strict-aliasing', '-st
    dlib=libc++', '-fno-rtti', '-fno-exceptions', '-fno-math-errno', '-pthread', '-pipe', '-g', '-fno-omit-frame-pointer', '-Wno-error=shadow', '-resource-dir=/Users/jchan/.vim/bundle/YouCompl
    eteMe/third_party/ycmd/ycmd/../clang_includes', '-isystem', '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1', '-isystem', '/Library/Devel
    oper/CommandLineTools/usr/include/c++/v1', '-isystem', '/usr/local/include', '-isystem', '/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include', '-isy
    stem', '/Library/Developer/CommandLineTools/usr/include', '-isystem', '/usr/include', '-isystem', '/System/Library/Frameworks', '-isystem', '/Library/Frameworks', '-isystem', '/Application
    s/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/7.3.0/include']
    -- Server running at: http://127.0.0.1:56010
    -- Server process ID: 65832
    -- Server logfiles:
    --   /var/folders/hy/qdy7j3px64147_6wmn_7vqbw0000gn/T/ycm_temp/server_56010_stdout.log
    --   /var/folders/hy/qdy7j3px64147_6wmn_7vqbw0000gn/T/ycm_temp/server_56010_stderr.log

I noticed the official example .ycm_extra_conf.py file had -std=c++11 and -xc++, so I tried adding those to the local .ycm_extra_conf.py and now it seems to work.
However, I removed them to see if they were what fixed it, and YCM continued to work for a short while before running into the same issue. I added them back and the error hasn't come back either. Maybe the server persists for a short while?

Here is the change I made:

    diff --git a/.ycm_extra_conf.py b/.ycm_extra_conf.py
    --- a/.ycm_extra_conf.py
    +++ b/.ycm_extra_conf.py
    @@ -28,13 +28,14 @@ def FlagsForFile(filename):
         mach.run(['compileflags', filename], stdout=out, stderr=out)
    
         flag_list = shlex.split(out.getvalue())
    
         # This flag is added by Fennec for android build and causes ycmd to fail to parse the file.
         # Removing this flag is a workaround until ycmd starts to handle this flag properly.
         # https://github.com/Valloric/YouCompleteMe/issues/1490
         final_flags = [x for x in flag_list if not x.startswith('-march=armv')]
    +    final_flags = ['-std=c++11', '-xc++'] + final_flags
    
         return {
             'flags': final_flags,
             'do_cache': True
         }

Anyways, glad it seems to work now. Thanks for the help! Do you think that these flags were what was missing?
Never mind, seems there are still missing includes for the definitions of things like nscoord and nsTArray (btw, sorry for bugspam). Looks like it is more complicated than just those two flags.

Should I open a new bug?
(Reporter)

Comment 50

2 years ago
Yes

Comment 51

a year ago
Installed YCM following the instructions here, and when I opened a HTML existing code in vim to check if there's some autocompletion. I don't saw anything. Vim just acted like nothing was installed. So I try ~/.vim/bundle/YouCompleteMe/install.py and worked!
The instructions are not incomplete?

Updated

5 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.