[verible-patch-tool] color the diffs/hunks presented to the user

[fangism]

For the underlying patch-tool described here:
https://github.com/google/verible#interactive-formatting

It would be nice if the diff could be colorized for readability. Currently it is monochrome.

Example mock-up using (github markdown's default coloring):

$ verible-verilog-format-changed-lines-interactive.sh 
--- /home/fangism/local/src/lowRISC/opentitan/hw/ip/aes/rtl/aes_core.sv
+++ NEW/home/fangism/local/src/lowRISC/opentitan/hw/ip/aes/rtl/aes_core.sv
@@ -32,3 +32,3 @@
   // Signals
-  logic                 foo_bar;
+  logic foo_bar;
   logic                 ctrl_qe;
Apply this hunk? [y,n,a,d,s,q,?] n
@@ -107,3 +107,3 @@
 
-      logic   illogical;
+  logic illogical;
 
Apply this hunk? [y,n,a,d,s,q,?] n

Would also color the prompt Apply this hunk? a little differently like this (dark-themed):

Task:

• Detect when running in interactive mode, e.g. using isatty(), and only apply colors in this case.
  • Possibly detect terminal type as well.
• Make sure existing tests still work (ok that they all run non-interactively)

[fangism]

b/161930379

[DhairyaBahl]

@fangism I would like to work on this issue.

Thanks :)

[fangism]

Thank you.

[fangism]

@DhairyaBahl had any time to look into this?

[DhairyaBahl]

@fangism I am extremely sorry for delay towards the issue. I will try my best to push a PR in a day or so.

Thank you :) sorry again :)

[DhairyaBahl]

@fangism Work Update: I am trying to understand how verible works, about its various features to resolve this issue and to first understand the issue. I get that i have to change the color of the text or make it look more interactive but i am confused that how to change the colordiff if its installed by default in the system by the apt-get install commands. Kindly guide me a bit to how to approach this problem.

Thanks a lot for having patience towards me :)

[fangism]

The patch-tool is a small piece of the Verible project that is generic and only operates on patch files.

The top-level main file is https://cs.opensource.google/verible/verible/+/master:common/tools/patch_tool.cc
You only need to look downwards from here.

To build and run this:

• bazel build -c opt //common/tools:verible-patch-tool
• ./bazel-bin/common/tools/verible-patch-tool args...
• verible-patch-tool help apply-pick for usage

A simpler demo (do all of the following from the same dir):

• create two files that differ by a few lines
• diff -u file1 file2 > patch
• verible-patch-tool apply-pick < patch and follow the prompts. Say n to decline all changes, and see the interface described in the initial comment.

So let's start small, say only colorize the prompt: Apply this hunk? [y,n,a,d,s,q,?] n

That prompt is done here.

As for colorization itself, which is done by printing control characters to the terminal, see if there's a simple way to do it without introducing a library. You might research source code for other tools that colorize output. Do you they use a common library? or do their own thing?
e.g.

• https://clang.llvm.org/docs/UsersManual.html (look at color-diagnostics)

Maybe report back with a few options before coding up an implementation.

In the worst case, we may make a tiny colorize library for ourselves.

[DhairyaBahl]

@fangism I am trying to build my verible-patch-tool but it is showing following error :
ERROR: Analysis of target '//common/tools:verible-patch-tool' failed; build aborted: Unable to find flex binary

I thought this error was occuring because output_user_root contains spaces so i passed commands with different output_user_root="" commands but everytime it says unable to find flex binary

Kindly guide me further, now I am moving to the passive part i.e. understand the codebase and other things u mentioned in the previous message.

Thanks :)

[fangism]

@fangism I am trying to build my verible-patch-tool but it is showing following error :
ERROR: Analysis of target '//common/tools:verible-patch-tool' failed; build aborted: Unable to find flex binary

I thought this error was occuring because output_user_root contains spaces so i passed commands with different output_user_root="" commands but everytime it says unable to find flex binary

Kindly guide me further, now I am moving to the passive part i.e. understand the codebase and other things u mentioned in the previous message.

Thanks :)

Strange, bazel is supposed to know how to fetch/build/install flex for you automatically.
verible-patch-tool itself doesn't depend on flex, but other parts of verible do.

I wonder if this due to locally installed flex being an indirect dependency through Kythe.
See all of its dependencies here https://kythe.io/getting-started/.

Try to sudo apt-get install flex and any other missing tool errors from that list.

@ivan444 in case you have any other suggestions.

[DhairyaBahl]

Strange, bazel is supposed to know how to fetch/build/install flex for you automatically.
verible-patch-tool itself doesn't depend on flex, but other parts of verible do.
I would like to mention that i was getting flex error even in the case of patch tool only

Try to sudo apt-get install flex and any other missing tool errors from that list.

If i forgot to mention i would like to tell, I am on windows. Could that be an issue ?

[DhairyaBahl]

@fangism So do i have to install all kythe dependencies individually or there is any other option to build the patch tool.

after installing the flex it is now giving this error:
ERROR: Analysis of target '//common/tools:verible-patch-tool' failed; build aborted: empty string

Thanks:)

[fangism]

@fangism So do i have to install all kythe dependencies individually or there is any other option to build the patch tool.

after installing the flex it is now giving this error:
ERROR: Analysis of target '//common/tools:verible-patch-tool' failed; build aborted: empty string

Thanks:)

After installing flex is the flex binary in your PATH? Can you run flex --help?

Is there any other clue where "empty string" is coming from?

I unfortunately have no experience building on Windows, but at least one contribution came from someone using Windows in the past. You might have to reach out to one of the bazel users mailing lists for help.

[fangism]

According to these files:
https://cs.opensource.google/kythe/kythe/+/master:tools/build_rules/lexyacc/BUILD
https://cs.opensource.google/kythe/kythe/+/master:tools/build_rules/lexyacc/lexyacc.bzl

It's looking for /usr/bin/flex or FLEX from environment variables, and similar for /usr/bin/bison as YACC. Can you try something like FLEX=/path/to/your/installed/flex YACC=/path/to/your/bison bazel build ...? (or whatever the equivalent of that is on Windows)

[DhairyaBahl]

After installing flex is the flex binary in your PATH? Can you run flex --help?

Yes I can run flex --help it is showing other possible flags like --version

Is there any other clue where "empty string" is coming from?

I have no idea, i am working for the first time on bazel itself

I unfortunately have no experience building on Windows, but at least one contribution came from someone using Windows in the past. You might have to reach out to one of the bazel users mailing lists for help.

Okay, I guess I should join the mailing list now !!

@fangism Thanks

[DhairyaBahl]

According to these files:
https://cs.opensource.google/kythe/kythe/+/master:tools/build_rules/lexyacc/BUILD
https://cs.opensource.google/kythe/kythe/+/master:tools/build_rules/lexyacc/lexyacc.bzl

It's looking for /usr/bin/flex or FLEX from environment variables, and similar for /usr/bin/bison as YACC. Can you try something like FLEX=/path/to/your/installed/flex YACC=/path/to/your/bison bazel build ...? (or whatever the equivalent of that is on Windows)

I have set the environment variables for the system so that they can find flex. however I've also made it available for the mingw64 in its local bin directory !!

Should environment variable still be an issue ??

@fangism Thanks

[fangism]

I don't really know myself, unfortunately. The kythe project I referenced is at https://github.com/kythe/kythe -- they might have some idea about to control the locations of flex/bison.

I'm trying to think if there's a way for you to locally patch out the kythe dependency, because you don't need it for this task.
Could you try commenting out lines 143-to-end of https://cs.opensource.google/verible/verible/+/master:WORKSPACE;drc=a0d9948afb99b17f9246031434d1c4f59971ac0d;l=143
and see if bazel build -c opt //common/tools:verible-patch-tool is able to proceed?

[DhairyaBahl]

@fangism I tried and it worked, but now its showing new erro:

The target you are compiling requires Visual C++ build tools.
Bazel couldn't find a valid Visual C++ build tools installation on your machine.

Visual C++ build tools seems to be installed at C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC
But Bazel can't find the following tools:
VCVARSALL.BAT, cl.exe, link.exe, lib.exe, ml64.exe
for x64 target architecture

Please check your installation following https://docs.bazel.build/versions/master/windows.html#using

I guess, i should reinstall the Microsoft Visual studio package again or only the missing files if i can !! and should read the documentation linked by the bazel and try to fix it myself.

Thanks :)

[fangism]

Ok, that's progress, I'm glad there some documentation to guide you.

[ivan444]

We'll have to do something about those Kythe dependencies. The introduced complexity is not appropriate.

I'll address it together with reducing the c++ build requirements (lowering it from c++17 to c++11).

[DhairyaBahl]

@fangism bazel needs some tools for which i have to install latest version of C++ redistributable build tools 2019 but my pc doesn't support it. It only support till 2015 version. What should i do ?? Is there any alternative to bazel ?? or any other way to build without using bazel then kindly tell me.

Thanks

[fangism]

At this time we can only support bazel building, I'm afraid. Thanks for trying.
If you're still stuck, feel free to un-assign yourself from issues, we totally understand.

[DhairyaBahl]

@fangism I will try one more time to install this package with bazel, if I failed then I will unassign myself :)

[DhairyaBahl]

@fangism I am extremely sorry for inconvenience. I won't be able to solve this issue, because I am unable to solve the problem of bazel for the time-being. Thank you for your time and effort :)

[fangism]

@fangism I am extremely sorry for inconvenience. I won't be able to solve this issue, because I am unable to solve the problem of bazel for the time-being. Thank you for your time and effort :)

@DhairyaBahl Thanks for the update, and thanks again for trying. Some day our build support for VS will improve.