Skip to content
This repository was archived by the owner on Aug 30, 2020. It is now read-only.

Two small additions #65

Open
BKoller wants to merge 2 commits into
base: stable
Choose a base branch
from
Open

Two small additions #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2e8f06a
Treat include paths as relative to CWD of CC command
BKoller May 16, 2016
8837813
Account for valueless #defines
BKoller May 16, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions config_gen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,8 @@ def parse_flags(build_log):

# macro definitions should be handled separately, so we can resolve duplicates
define_flags = dict()
define_regex = re.compile("-D([a-zA-Z0-9_]+)=(.*)")
define_regex = re.compile("-D\s*([a-zA-Z0-9_]+)(=.*)?")
define_flag = '-D'

# Used to only bundle filenames with applicable arguments
filename_flags = ["-o", "-I", "-isystem", "-include", "-imacros", "-isysroot"]
Expand All @@ -337,8 +338,12 @@ def parse_flags(build_log):
line_count += 1
words = split_flags(line)

cmd_pwd = './'

for (i, word) in enumerate(words):
if(word[0] != '-' or not flags_whitelist.match(word)):
if(word.find('YCM_CONFIG_GEN_PWD') == 0):
Copy link
Owner

@rdnetto rdnetto Aug 21, 2016

Choose a reason for hiding this comment

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

Nit: word.startswith() would be simpler.

Copy link
Author

@BKoller BKoller Aug 21, 2016

Choose a reason for hiding this comment

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

Agreed.

cmd_pwd = word[word.find("=") + 1:]
continue

# handle macro definitions
Expand All @@ -351,9 +356,13 @@ def parse_flags(build_log):

continue

if(word == define_flag):
flags.add(word + words[i+1])
continue

# include arguments for this option, if there are any, as a tuple
if(i != len(words) - 1 and word in filename_flags and words[i + 1][0] != '-'):
flags.add((word, words[i + 1]))
flags.add((word, os.path.realpath(os.path.join(cmd_pwd, words[i + 1]))))
else:
flags.add(word)

Expand All @@ -374,7 +383,10 @@ def parse_flags(build_log):
print("WARNING: {} distinct definitions of macro {} found".format(len(values), name))
values.sort()

flags.add("-D{}={}".format(name, values[0]))
if values[0] != None:
flags.add("-D{}{}".format(name, values[0]))
else:
flags.add("-D{}".format(name))

return (line_count, skip_count, sorted(flags))

Expand Down
2 changes: 1 addition & 1 deletion fake-toolchain/Unix/cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ elif [ "$1" = "-v" ] || [ "$1" = "--version" ]; then
$YCM_CONFIG_GEN_CC_PASSTHROUGH $@

else
echo "$@" >> $YCM_CONFIG_GEN_CC_LOG
echo "YCM_CONFIG_GEN_PWD=$(pwd) $@" >> $YCM_CONFIG_GEN_CC_LOG
Copy link
Owner

@rdnetto rdnetto Aug 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

You need to quote the directory here, otherwise the parsing will fail if it contains spaces.

Copy link
Owner

@rdnetto rdnetto Aug 21, 2016

Choose a reason for hiding this comment

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

Sorry for the delay in replying, real life has been quite hectic lately.

Copy link
Author

@BKoller BKoller Aug 21, 2016

Choose a reason for hiding this comment

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

Oops, I'll fix that. No worries about the delay!

Copy link
Author

@BKoller BKoller Aug 21, 2016

Choose a reason for hiding this comment

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

On second thought, while having spaces in a path does break this commit, I don't think adding quotes is the fix. YCM_CONFIG_GEN_PWD is not a shell variable. YCM_CONFIG_GEN_PWD=<path> is just a special word that gets created and parsed here. This is where the issue is, because there is no support for spaces in words :)

In addition to breaking this commit, I suspect that this limitation of words also prevents proper parsing of filename_flags with spaces in the path such as -I "a directory with spaces".

Copy link
Owner

@rdnetto rdnetto Aug 21, 2016

Choose a reason for hiding this comment

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

The flags are split up / tokenized by split_flags(), which uses unbalanced_quotes() to concatenate adjacent words containing quotes. Since YCM_CONFIG_GEN_PWD= is checked for in a flag, it will have the same logic applied to it.

Copy link
Author

@BKoller BKoller Aug 21, 2016

Choose a reason for hiding this comment

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

Ah I see. While that will work for YCM_CONFIG_GEN_PWD, it seems that spaces in paths coming from the compiler invocation will still parse incorrectly since the quotes are dropped by $@. I eagerly await your explanation of what I've missed this time ;)

fi