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

support excude directories or files #252

Closed
wants to merge 1 commit into from
Closed

support excude directories or files #252

wants to merge 1 commit into from

Conversation

liugang
Copy link

@liugang liugang commented May 28, 2019

Fix or Enhancement?

For some large projects, we don't care too many subprojects, such as the Linux kernel, third-party libraries, and so on.
So let the bear support the excude directory or file, then reduce the cache size and speed up the index.

e.g. ccls, cquery

  • New tests added
  • All tests passed

Environment

  • OS:
  • Bear version:

Copy link
Owner

@rizsotto rizsotto left a comment

Choose a reason for hiding this comment

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

Hey @liugang , thanks for this. I agree that this functionality might be desired. Although I would not solve it as you did. Can I ask you for a change?

bear/main.py.in Outdated
@@ -296,9 +296,9 @@ def intercept_build():
if args.append and os.path.isfile(args.cdb):
Copy link
Owner

Choose a reason for hiding this comment

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

Could you filter out the excluded files before calling the CompilationDatabase.save method? I would like to keep the save functionality as is. It can be a simple function like this:

def exclude(candidates, directories):
     # type: (Iterable[Compilation], List[str]) -> Iterable[Compilation]

    def is_no_excluded(candidate):
         return not any(candidate.is_in(directory) for directory in directories)

    return (candidate for candidate in candidates if is_no_excluded(candidate))

This would require to create an is_in method on the Compilation class. Please use the os.path module to check if the directory contains the file or not. (String comparision might be misleading.)

@liugang
Copy link
Author

liugang commented Jun 3, 2019

Hi @rizsotto I add exclude after if os.path.isfile(result.source) , because it is added here, the Compilation object has legal directory and file attributes. And we can easily determine relative and absolute paths whether it is part of result.sourece

Copy link
Owner

@rizsotto rizsotto left a comment

Choose a reason for hiding this comment

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

Thanks for the update.

  • Please extract the logic which decide about which files to exclude.
  • I would still prefer the exclude to work on iterators. And not part of any existing logic.
  • I would like to have a few test cases to prove that it's actually working as expected.

bear/main.py.in Outdated
@@ -576,7 +580,32 @@ class Compilation:
flags=candidate.flags,
output=output)
Copy link
Owner

Choose a reason for hiding this comment

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

Could you make it a bit simpler? Like...

if os.path.isfile(result.source) and is_not_excluded(result.source, excludes):
    yield result

bear/main.py.in Outdated
@@ -555,15 +560,14 @@ class Compilation:
return cls.iter_from_execution(execution, category)

@classmethod
def iter_from_execution(cls, execution, category):
def iter_from_execution(cls, execution, category, args=None):
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't use default value.

@rizsotto rizsotto added this to To do in 2.4.2 Aug 16, 2019
@liugang
Copy link
Author

liugang commented Aug 17, 2019

Hi @rizsotto It has been modified.

  1. Added a project_root, because sometimes run build command is not in source directory

  2. test case with project root is changed

mkdir build;

# source root is changed
 cd build; cmake ..

# if project_root changed, without `-p project_root`, not exclude anything
 make clean ; bear -e ear.c make ; cat compile_commands.json 
 make clean ; bear -e libear/ear.c make ; cat compile_commands.json 

# specify project_root with `-p`, it will exclude ear.c or any file under libear
 make clean ; bear -p `pwd`/.. -e libear/ear.c make ; cat compile_commands.json 
 make clean ; bear -p `pwd`/.. -e libear make ; cat compile_commands.json

# file or directory is not exist, will ignore it 
 make clean ; bear -p `pwd`/.. -e libear1 make ; cat compile_commands.json 
  1. test case with project root is not change
  cd ..

  make -C build/ clean ; bear -e libear make -C build ; cat compile_commands.json 
  make -C build/ clean ; bear -e libear1 make -C build ; cat compile_commands.json 
  make -C build/ clean ; bear -e libear/ear.c make -C build ; cat compile_commands.json 
  make -C build/ clean ; bear -e libear/ear.cc make -C build ; cat compile_commands.json 

@liugang
Copy link
Author

liugang commented Aug 17, 2019

In the actual big project, the effect is very obvious

bear -e opensources -e driver -e kernel -e sdk -e tools make

  • without exclude
    du -lsh ../compile_commands.json
    10M ../compile_commands.json

  • with exclude
    du -lsh compile_commands.json
    1.1M compile_commands.json

  • without exclude
    grep directory ../compile_commands.json |wc
    7833 23502 724206

  • with exclude
    grep directory compile_commands.json |wc
    1054 3162 77926

@rizsotto rizsotto moved this from To do to In progress in 2.4.2 Aug 18, 2019
@rizsotto
Copy link
Owner

I was looking into this PR. Was decided to go in a different way. Created the ticket #261 to draft the design for this problem.

@rizsotto rizsotto closed this Aug 18, 2019
@rizsotto rizsotto moved this from In progress to Done in 2.4.2 Aug 18, 2019
@liugang liugang deleted the support-exclude-file-or-directory branch August 26, 2019 01:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
2.4.2
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants