tools: Add a script to generate arch-specific ctags

tools: Add a script to generate arch-specific ctags

It would be better to generate ctags file for specified architecture so
this CL adds a script gen-tags.py to generate architecture specific
ctags.

  Usage:
      $ tools/dev/gen-tags.py [<arch>...]

  The example usage for 'x64' is as follows:
      $ tools/dev/gen-tags.py x64

  If no <arch> is given, it generates tags file for all arches:
      $ tools/dev/gen-tags.py

R=yangguo@chromium.org,jochen@chromium.org,jkummerow@chromium.org,clemensh@chromium.org

NOTRY=true

Review-Url: https://codereview.chromium.org/2762903002
Cr-Commit-Position: refs/heads/master@{#44032}
Committed: https://chromium.googlesource.com/v8/v8/+/2ff2a0c65d07bed1b8de3d77dce2e1848574e3d2

[honggyu.kim, 2017-03-21 03:59:34]

Hi, Please take a look at it!

[Yang, 2017-03-21 06:03:05]

yangguo@chromium.org changed reviewers:
+ jkummerow@chromium.org

[Yang, 2017-03-21 06:03:05]

The make workflow is going to be deprecated by the end of the year latest, once
we end support for GYP configs. I don't know how much sense it makes to add new
features at this point.

[honggyu.kim, 2017-03-21 06:38:00]

On 2017/03/21 06:03:05, Yang wrote:
> The make workflow is going to be deprecated by the end of the year latest,
once
> we end support for GYP configs. I don't know how much sense it makes to add
new
> features at this point.

Hm.. Can you give me some ideas how you usually generate ctags file for source
analysis?
If there's nothing good, what about writing a shell script in tools directory
for ctags?

[jochen (gone - plz use gerrit), 2017-03-21 10:32:29]

On 2017/03/21 at 06:38:00, honggyu.kp wrote:
> On 2017/03/21 06:03:05, Yang wrote:
> > The make workflow is going to be deprecated by the end of the year latest,
once
> > we end support for GYP configs. I don't know how much sense it makes to add
new
> > features at this point.
> 
> Hm.. Can you give me some ideas how you usually generate ctags file for source
analysis?
> If there's nothing good, what about writing a shell script in tools directory
for ctags?

yes, that sounds like a plan

[Jakob Kummerow, 2017-03-21 10:34:37]

jkummerow@chromium.org changed reviewers:
+ bmeurer@chromium.org, clemensh@chromium.org

[Jakob Kummerow, 2017-03-21 10:34:38]

Adding Benedikt and Clemens who have worked on existing ctags support in the
Makefile.

I would suggest putting a new, standalone Python script into tools/dev/. 

We could integrate ctags generation functionality into gm.py if there is logic
that can be shared; I suspect that there isn't, so a separate file would be
easier to maintain and understand.

Also, a question: what is the benefit of arch-specific ctags files? While I
don't use ctags myself, in my workflows I'm usually interested in all platforms.
(Of course, if this useful for only some people, we can still land it; I'm just
curious what problem it solves.)

[Clemens Hammacher, 2017-03-21 10:39:26]

On 2017/03/21 at 10:34:38, jkummerow wrote:
> Adding Benedikt and Clemens who have worked on existing ctags support in the
Makefile.
> 
> I would suggest putting a new, standalone Python script into tools/dev/. 
> 
> We could integrate ctags generation functionality into gm.py if there is logic
that can be shared; I suspect that there isn't, so a separate file would be
easier to maintain and understand.
> 
> Also, a question: what is the benefit of arch-specific ctags files? While I
don't use ctags myself, in my workflows I'm usually interested in all platforms.
(Of course, if this useful for only some people, we can still land it; I'm just
curious what problem it solves.)

+1 to everything you said :)

[honggyu.kim, 2017-03-21 14:57:49]

On 2017/03/21 10:34:38, Jakob Kummerow wrote:
> Adding Benedikt and Clemens who have worked on existing ctags support in the
> Makefile.
> 
> I would suggest putting a new, standalone Python script into tools/dev/. 

I've just uploaded gen-tags.py in tools/dev/ as you said.

> We could integrate ctags generation functionality into gm.py if there is logic
> that can be shared; I suspect that there isn't, so a separate file would be
> easier to maintain and understand.
> 
> Also, a question: what is the benefit of arch-specific ctags files? While I
> don't use ctags myself, in my workflows I'm usually interested in all
platforms.
> (Of course, if this useful for only some people, we can still land it; I'm
just
> curious what problem it solves.)

As a ctags user myself, when I jump to find some functions, I have to choose one
from many architectures.  I sometimes want to focus on some specific
architectures
to make code browsing easier.

In Linux kernel, there's "make tags" command to generate ctags file only for
pre-configured architecture.

Please have a look at the uploaded script.

[honggyu.kim, 2017-03-21 14:58:58]

Description was changed from

==========
Makefile: Introduce arch-specific ctags rule

It would be better to generate ctags file for specified architecture so
this CL supports build rules to generate architecture specific ctags in
Makefile.  The example usage for x64 is as follows:

  $ make x64.tags

R=yangguo@chromium.org, jochen@chromium.org
==========

to

==========
tools: Add a script to generate arch-specific ctags

It would be better to generate ctags file for specified architecture so
this CL adds a script gen-tags.py to generate architecture specific
ctags.

  Usage:
      $ tools/dev/gen-tags.py [<arch>...]

  The example usage for 'x64' is as follows:
      $ tools/dev/gen-tags.py x64

  If no <arch> is given, it generates tags file for all arches:
      $ tools/dev/gen-tags.py

R=yangguo@chromium.org,jochen@chromium.org,jkummerow@chromium.org,clemensh@ch...
==========

[Jakob Kummerow, 2017-03-21 16:14:43]

Looks good! Just a few minor comments.

https://codereview.chromium.org/2762903002/diff/20001/tools/dev/gen-tags.py
File tools/dev/gen-tags.py (right):

https://codereview.chromium.org/2762903002/diff/20001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:18: import os
nit: unused import

https://codereview.chromium.org/2762903002/diff/20001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:44: #    if not CheckIfTopDir(argv[0]):
nit: drop this

https://codereview.chromium.org/2762903002/diff/20001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:49: # if no argument is given, then generate ctags for all
arches
nit: Proper capitalization and punctuation please.

https://codereview.chromium.org/2762903002/diff/20001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:70: exclude_options = ' '.join(map(f, exclude_arches))
simpler:

exclude_options = ["--exclude=%s" % arch for arch in exclude_arches]

https://codereview.chromium.org/2762903002/diff/20001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:72: command = "ctags --fields=+l " + exclude_options + "
-R include/* src/*"
what about test/* ?

[honggyu.kim, 2017-03-22 04:31:31]

On 2017/03/21 16:14:43, Jakob Kummerow wrote:
> Looks good! Just a few minor comments.
> 
>
https://codereview.chromium.org/2762903002/diff/20001/tools/dev/gen-tags.py#n...
> tools/dev/gen-tags.py:72: command = "ctags --fields=+l " + exclude_options + "
> -R include/* src/*"
> what about test/* ?

I've added test/* as well and updated to use os feature rather than using
--exclude option in ctags.
Please take a look at it again. Thanks for your review!

Honggyu

[Jakob Kummerow, 2017-03-22 12:11:10]

Thanks. I have some more suggestions for further simplifications.

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py
File tools/dev/gen-tags.py (right):

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:35: def __init__(self):
You don't need this constructor.

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:55: user_arches.append(self.ParseArg(argstring))
If you inline ParseArg here, you don't need the class ArgumentParser at all.
Just make "ParseArguments(argv)" a top-level function. It would also be nice to
guard against typos here:

for argstring in argv:
  if argstring in ("-h", "--help", "help":
    PrintHelpAndExit()
  if argstring not in ARCHES:
    print("Invalid argument: %s" % argstring)
    sys.exit(1)
  user_arches.append(argstring)

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:73: gtags = open(gtags_filename, "w")
instead of a manual open/close pair, please use a with-block, which is simpler
and more robust:

with open(gtags_filename, "w") as gtags:
  # ...
  gtags.write(...)

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:80: exclude_flag = False
A more Pythonic way would be to pull out the inner loop into a function (can be
global, since this script is so small) and use "return" instead of
"break"+exclude_flag:

def Exclude(fullpath, exclude_arches):
  for arch in exclude_arches:
    if ("/%s/" % arch) in fullpath: return True
  return False

And then you can write the main part (with an explicit for-loop instead of
itertools.chain for increased simplicity) as:

with open(gtags_filename, "w") as gtags:
  for path in paths:
    for root, dirs, files in os.walk(path):
      for file in files:
        if not file.endswith(tuple(exts)): continue
        fullpath = os.path.join(root, file)
        if exclude(fullpath, exclude_arches): continue
        gtags.write(fullpath + os.linesep)

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:91: command = "ctags --fields=+l -L " + gtags_filename
nit: just inline this into _Call() below, it's cleaner.

[jeojohn93, 2017-03-22 12:24:11]

Thank you for sharing the great information with us.Generating the c tag from
the architecture technique is help me to improve my skills and solve some
problems in website -
http://petrotek.ae/lubricants/industrial-lubricants/13/spindle-oils

[honggyu.kim, 2017-03-22 15:17:24]

On 2017/03/22 12:11:10, Jakob Kummerow wrote:
> Thanks. I have some more suggestions for further simplifications.
> 
> https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py
> File tools/dev/gen-tags.py (right):
> 
>
https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
> tools/dev/gen-tags.py:35: def __init__(self):
> You don't need this constructor.
> 
>
https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
> tools/dev/gen-tags.py:55: user_arches.append(self.ParseArg(argstring))
> If you inline ParseArg here, you don't need the class ArgumentParser at all.
> Just make "ParseArguments(argv)" a top-level function. It would also be nice
to
> guard against typos here:
> 
> for argstring in argv:
>   if argstring in ("-h", "--help", "help":
>     PrintHelpAndExit()
>   if argstring not in ARCHES:
>     print("Invalid argument: %s" % argstring)
>     sys.exit(1)
>   user_arches.append(argstring)
> 
>
https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
> tools/dev/gen-tags.py:73: gtags = open(gtags_filename, "w")
> instead of a manual open/close pair, please use a with-block, which is simpler
> and more robust:
> 
> with open(gtags_filename, "w") as gtags:
>   # ...
>   gtags.write(...)
> 
>
https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
> tools/dev/gen-tags.py:80: exclude_flag = False
> A more Pythonic way would be to pull out the inner loop into a function (can
be
> global, since this script is so small) and use "return" instead of
> "break"+exclude_flag:
> 
> def Exclude(fullpath, exclude_arches):
>   for arch in exclude_arches:
>     if ("/%s/" % arch) in fullpath: return True
>   return False
> 
> And then you can write the main part (with an explicit for-loop instead of
> itertools.chain for increased simplicity) as:
> 
> with open(gtags_filename, "w") as gtags:
>   for path in paths:
>     for root, dirs, files in os.walk(path):
>       for file in files:
>         if not file.endswith(tuple(exts)): continue
>         fullpath = os.path.join(root, file)
>         if exclude(fullpath, exclude_arches): continue
>         gtags.write(fullpath + os.linesep)
> 
>
https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
> tools/dev/gen-tags.py:91: command = "ctags --fields=+l -L " + gtags_filename
> nit: just inline this into _Call() below, it's cleaner.

You're absolutely right. I've just updated the script again as you guided.
I appreciate your help!

[honggyu.kim, 2017-03-22 15:19:15]

Thanks for your suggestion.

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py
File tools/dev/gen-tags.py (right):

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:35: def __init__(self):
On 2017/03/22 12:11:09, Jakob Kummerow wrote:
> You don't need this constructor.

Done.

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:55: user_arches.append(self.ParseArg(argstring))
On 2017/03/22 12:11:09, Jakob Kummerow wrote:
> If you inline ParseArg here, you don't need the class ArgumentParser at all.
> Just make "ParseArguments(argv)" a top-level function. It would also be nice
to
> guard against typos here:
> 
> for argstring in argv:
>   if argstring in ("-h", "--help", "help":
>     PrintHelpAndExit()
>   if argstring not in ARCHES:
>     print("Invalid argument: %s" % argstring)
>     sys.exit(1)
>   user_arches.append(argstring)

Done.

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:73: gtags = open(gtags_filename, "w")
On 2017/03/22 12:11:10, Jakob Kummerow wrote:
> instead of a manual open/close pair, please use a with-block, which is simpler
> and more robust:
> 
> with open(gtags_filename, "w") as gtags:
>   # ...
>   gtags.write(...)

Done.

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:80: exclude_flag = False
On 2017/03/22 12:11:09, Jakob Kummerow wrote:
> A more Pythonic way would be to pull out the inner loop into a function (can
be
> global, since this script is so small) and use "return" instead of
> "break"+exclude_flag:
> 
> def Exclude(fullpath, exclude_arches):
>   for arch in exclude_arches:
>     if ("/%s/" % arch) in fullpath: return True
>   return False
> 
> And then you can write the main part (with an explicit for-loop instead of
> itertools.chain for increased simplicity) as:
> 
> with open(gtags_filename, "w") as gtags:
>   for path in paths:
>     for root, dirs, files in os.walk(path):
>       for file in files:
>         if not file.endswith(tuple(exts)): continue
>         fullpath = os.path.join(root, file)
>         if exclude(fullpath, exclude_arches): continue
>         gtags.write(fullpath + os.linesep)

Done.

https://codereview.chromium.org/2762903002/diff/60001/tools/dev/gen-tags.py#n...
tools/dev/gen-tags.py:91: command = "ctags --fields=+l -L " + gtags_filename
On 2017/03/22 12:11:09, Jakob Kummerow wrote:
> nit: just inline this into _Call() below, it's cleaner.

Done.

[honggyu.kim, 2017-03-22 15:21:11]

On 2017/03/22 12:24:11, jeojohn93 wrote:
> Thank you for sharing the great information with us.Generating the c tag from
> the architecture technique is help me to improve my skills and solve some
> problems in website -
> http://petrotek.ae/lubricants/industrial-lubricants/13/spindle-oils

It's just a simple script but if it's useful to you, then it's also good to me.
Thanks!

[Jakob Kummerow, 2017-03-22 15:55:22]

Description was changed from

==========
tools: Add a script to generate arch-specific ctags

It would be better to generate ctags file for specified architecture so
this CL adds a script gen-tags.py to generate architecture specific
ctags.

  Usage:
      $ tools/dev/gen-tags.py [<arch>...]

  The example usage for 'x64' is as follows:
      $ tools/dev/gen-tags.py x64

  If no <arch> is given, it generates tags file for all arches:
      $ tools/dev/gen-tags.py

R=yangguo@chromium.org,jochen@chromium.org,jkummerow@chromium.org,clemensh@ch...
==========

to

==========
tools: Add a script to generate arch-specific ctags

It would be better to generate ctags file for specified architecture so
this CL adds a script gen-tags.py to generate architecture specific
ctags.

  Usage:
      $ tools/dev/gen-tags.py [<arch>...]

  The example usage for 'x64' is as follows:
      $ tools/dev/gen-tags.py x64

  If no <arch> is given, it generates tags file for all arches:
      $ tools/dev/gen-tags.py

R=yangguo@chromium.org,jochen@chromium.org,jkummerow@chromium.org,clemensh@ch...

NOTRY=true
==========

[Jakob Kummerow, 2017-03-22 15:55:57]

Thanks for your patience, LGTM now. Let's get this landed!

[Jakob Kummerow, 2017-03-22 15:56:02]

The CQ bit was checked by jkummerow@chromium.org

[commit-bot: I haz the power, 2017-03-22 15:56:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 16:01:24]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1490198162104250,
"parent_rev": "634537c66a6e5a4637504f65369512e90111c55a", "commit_rev":
"2ff2a0c65d07bed1b8de3d77dce2e1848574e3d2"}

[commit-bot: I haz the power, 2017-03-22 16:01:31]

Description was changed from

==========
tools: Add a script to generate arch-specific ctags

It would be better to generate ctags file for specified architecture so
this CL adds a script gen-tags.py to generate architecture specific
ctags.

  Usage:
      $ tools/dev/gen-tags.py [<arch>...]

  The example usage for 'x64' is as follows:
      $ tools/dev/gen-tags.py x64

  If no <arch> is given, it generates tags file for all arches:
      $ tools/dev/gen-tags.py

R=yangguo@chromium.org,jochen@chromium.org,jkummerow@chromium.org,clemensh@ch...

NOTRY=true
==========

to

==========
tools: Add a script to generate arch-specific ctags

It would be better to generate ctags file for specified architecture so
this CL adds a script gen-tags.py to generate architecture specific
ctags.

  Usage:
      $ tools/dev/gen-tags.py [<arch>...]

  The example usage for 'x64' is as follows:
      $ tools/dev/gen-tags.py x64

  If no <arch> is given, it generates tags file for all arches:
      $ tools/dev/gen-tags.py

R=yangguo@chromium.org,jochen@chromium.org,jkummerow@chromium.org,clemensh@ch...

NOTRY=true

Review-Url: https://codereview.chromium.org/2762903002
Cr-Commit-Position: refs/heads/master@{#44032}
Committed:
https://chromium.googlesource.com/v8/v8/+/2ff2a0c65d07bed1b8de3d77dce2e184857...
==========

[commit-bot: I haz the power, 2017-03-22 16:01:32]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/2ff2a0c65d07bed1b8de3d77dce2e184857...

[honggyu.kim, 2017-05-22 07:45:58]

Patchset #6 (id:100001) has been deleted