Powershell addition #1766

shaehn · 2017-09-08T14:48:51Z

This is the initial attempt at adding PowerShell analyzer to OpenGrok.

Changed HistoryGuru::addRepository so that it will recognize .opengrok_skip_history files in folders prior to scanning the folder/directory.

After looking at the code there was a simple concatenation of a directory string with the data from the command line. A better technique would be to use the Paths interface from java.nio.file.Paths which will deal appropriately with file separation characters when they are missing or not.

Added code to indicate parent stream of user's Accurev workspace. This will be seen on OpenGrok home page.

Removed 1st for-loop which was not doing much of anything. Only directories are considered to be looked at as being repositories, or holders of repository info.

This is the initial introduction of the PowerShell script analyzer for OpenGrok.

Hmm... what appears to compile satisfactorily on my machine had issues on Travis.

shaehn · 2017-09-08T20:15:32Z

What in blazes is going on here? I looked at the Travis details and it is complaining about errors in documentation over which I have no control. What am I doing wrong? I need some help/direction here or it is just going to be stuck in a broken state. (You can tell this is the first time I am going through this experience).

OK, apparently greater-than signs are illegal characters in comments.
So now the problem has to do with failing tests.
I see that the ctags being used on Travis isn't the same one I have been using from Univeral Ctags. So there are messages like: ctags: Unknown option: --kinddef-Posh
which is something that I have added.

I haven't a clue where to go from here.
Help Please.

Removed greater-than sign from comments

tarzanek · 2017-09-09T06:29:48Z

build.xml

@@ -246,6 +246,8 @@ Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved.
        <run-jflex dir="${gen.analysis.dir}/javascript" name="JavaScriptXref"/>
        <run-jflex dir="${gen.analysis.dir}/lua" name="LuaSymbolTokenizer"/>
        <run-jflex dir="${gen.analysis.dir}/lua" name="LuaXref"/>
+        <run-jflex dir="${gen.analysis.dir}/posh" name="PoshSymbolTokenizer"/>


I wouldn't mind if the analyzer would be called
PowerShell....
this way it is a bit confusing to see what is it for, I hope it's not too posh ;)

I can do that. I just was using the vernacular of the PowerShell community.

tarzanek · 2017-09-09T06:31:43Z

nbproject/project.properties

@@ -94,7 +94,7 @@ javadoc.encoding=${source.encoding}
 build.test.classes.dir=${build.dir}/test/classes
 jar.index=${jnlp.enabled}
 file.reference.jrcs.jar=lib/jrcs.jar
-javac.compilerargs=-Xlint\:unchecked
+javac.compilerargs=-Xlint:-fallthrough


please keep both options ( http://wiki.netbeans.org/Java_Hints#Standard_Javac_warnings )

Hmm.... I didn't realize that this got pushed up too. I thought it was just my local compilation. So are you asking to put both unchecked and -fallthrough together as part of the compilation. Or should I just revert to the unchecked option?

Is there a way to assign compile options to specific files like some C/C++ compilers do? For example, I would really like the -fallthrough option only be applied to those files that are generated by JFlex.

hmm, maybe it can be done, if you want we can move this to a subsequent push request to expedite this one

tarzanek · 2017-09-09T06:34:04Z

src/org/opensolaris/opengrok/analysis/Ctags.java

-                                    addTag(defs, seenSymbols, lnum, name, "argument",
+                        //TODO this algorithm assumes that data types occur to
+                        //     the left of the argument name, so it will not
+                        //     work for languages like rust, kotlin, etc. which


ok, this is not great,
if the old approach worked for kotlin, then I'd like to keep it
if it didn't and this is an improvement, then I am OK

btw. this code is for parsing the ctags output and this format is given, so we don't need to assume anything or make it work as reverse engineering, format is here: http://docs.ctags.io/en/latest/news.html#reference-tags

The old approach would not work for any language that has its data type to the right of the named argument. It was assuming 'C' style syntax. . It searched backward for that intervening space between datatype and argName. It also did not work for typeless languages. The algorithm I put into place will work for type less languages and datatypes to the left of the argName. To get the parsing to work for languages like kotlin we need to get a ctag attribute in the language that tells us its data type description is to the right side of the argument.

PS. We do have to parse this because this is the signature string which is all the stuff between the function's parenthesis. Another issue can be raised to deal with this one.

ok, so let's make this pass on full tests with universal ctags - @vladak - can you run this as a clone of a job on helius pointing to @shaehn s branch/fork ?
This way we will get a latest build with latest java and (if Vlada will update universal ctags) latest ctags and we will see if your changes break something
Unfortunately I don't have access to helius anymore and I didn't manage to build its copy outside of Oracle yet.

I was finally able to get the build to go green. I had to update the CtagsParserTest.java because the new parsing section I changed found more symbols. Some of the examples in the tests actually failed the old parsing method.

Currently, this is not going to fail kotlin programs because there is no signature being gathered by the parser in Ctags.java (at least that is what I think will happen. What I am not sure about is how the 'C' language gets a signature field declared. When I run the universal ctags on a simple .c file, there is no signature field generated. Yet looking at the set of tags when debugging OpenGrok one appears out of thin air, so to speak. Can you explain that to me?)

tarzanek · 2017-09-09T06:39:05Z

src/org/opensolaris/opengrok/analysis/posh/PoshAnalyzer.java

+ */
+
+/*
+ * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved.


just 2017 please

OK, I fixed all the copyright dates in the files. I have not pushed them up yet because that is just going to generate another failed build.

don't worry about failed builds, this is why CI is in place, to see builds fail ;)
(or fix)

tarzanek · 2017-09-09T06:39:19Z

src/org/opensolaris/opengrok/analysis/posh/PoshAnalyzerFactory.java

+ */
+
+/*
+ * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved.


tarzanek · 2017-09-09T06:39:29Z

src/org/opensolaris/opengrok/analysis/posh/PoshSymbolTokenizer.lex

+ */
+
+/*
+ * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved.


tarzanek · 2017-09-09T06:39:45Z

src/org/opensolaris/opengrok/analysis/posh/PoshXref.lex

+ */
+
+/*
+ * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved.


tarzanek · 2017-09-09T06:41:20Z

src/org/opensolaris/opengrok/history/AccuRevHistoryParser.java

@@ -92,13 +92,6 @@ History parse(File file, Repository repos) throws HistoryException {
                Executor executor = repository.getHistoryLogExecutor(file);
                executor.exec(true, this);

-                /*


I don't really like mixing various things in one pull request ...
but then I do it as well (d'oh!) being too lazy to open a new branch, so I guess we both need to sit down and read and start using the git branching as in https://github.com/OpenGrok/OpenGrok/wiki/Developer-intro
(thnx for the perfect writeup @vladak ! )

but I did make a new branch, so I must not be doing something else correctly.

I think this might fix itself once the other pullrequest is merged

tarzanek · 2017-09-09T06:42:31Z

src/org/opensolaris/opengrok/history/HistoryGuru.java

        for (File file : files) {
-            Repository repository = null;


yep, mixing two things, very hard to review and retest

but it looks like all accurev changes come from #1721

Yes, those changes are from that issue. So I must not be doing the branching correctly. To start, I first made a fork from the main OpenGrok github repository. I then made the AccuRev changes and pushed them up to my repository, then made a pull request. I then made a branch from my repository to deal with the new PowerShell code. So I guess its going to have the Accurev changes in it.

How else would I do the branching to keep things separate? Note, I am using the web interface, not the git command line interface and I am using the GitHub Desktop UI.
...
Hmm... after contemplating what the GitHub Desktop UI, I think the problem is that I needed to create a branch after I did the first fork from the GitHub master before I made modifications. That would have separated out the AccuRev changes from the Powershell changes. So does this mean problem is going to remain until my 1721 gets merged into the OpenGrok master? (if so, this puts me in a spot where I cannot create any more branches until that happens. That is, if you don't want to keep seeing the AccuRev changes for each branch.)

:) you can always create clean branches, but you need to go to git cli, git UIs are a bit oversimplistic and sometimes hide the fantastic possibilities that one can do with git ...

tarzanek · 2017-09-09T06:44:14Z

src/org/opensolaris/opengrok/web/Util.java

@@ -590,7 +590,7 @@ public static void readableLine(int num, Writer out, Annotation annotation,
                out.write("search\" href=\"" + env.getUrlPrefix());
                out.write("defs=&amp;refs=&amp;path=");
                out.write(project);
-                out.write("&amp;hist=" + URIEncode(r));
+                out.write("&amp;hist=&quot;" + URIEncode(r) + "&quot;");


won't this break other SCMs ? can you test to index a sample git repo and try if history links work properly?

This should not break other SCMs. All that is happening is that quotation marks are being placed around the revision number in the search form. In AccuRev the revision is of the form number/number and the '/' is a special character in the search system. Without the quotes, the user would get hollered at by OpenGrok. I think the other SCM's use just a simple number so normally this problem would not show up. Here is a sample of the href line:

href="/source/s?defs=&refs=&path=&project=navigation&hist="1673%2F3"&type="
Here the %2F is the repesentation of '/'

How would I perform the test you are suggesting? I am using netbeans 8.2 and all the documentation I find seems to be of older releases. The user interface has changed a lot and does not match the examples I am seeing.

the test would be to index a git repo besides accurev and check the links in same places as accurev (but for other history of course)

OK, I ran this on a mecurial repository which gives a simple revision number with no ill effects.
As you can see by the enclosed picture. I selected the 'S' in the annotated line below:

and it produced the following page
(the history revision number 4116 is surrounded with quotes).

tarzanek · 2017-09-09T06:46:17Z

@tulinkry I think we might have a race condition in your auth plugin tests - can you have a look at stacks in this travis: https://travis-ci.org/OpenGrok/OpenGrok/builds/273440700?utm_source=github_status&utm_medium=notification ?

tarzanek · 2017-09-11T19:02:18Z

src/org/opensolaris/opengrok/analysis/Ctags.java

+            // PowerShell
+            command.add("--langdef=Posh");
+            command.add("--langmap=Posh:+.ps1,Posh:+.psm1");
+            command.add("--kinddef-Posh=f,function,functions");


so we will restrict kinddef only when universal ctags are detected (see other ifs around this area)
for exuberant ctags the semantic info will be simply missing, alt. you might try to rewrite (if possible) to old approach (which is just a loger line from the quick look I had at http://docs.ctags.io/en/latest/news.html?highlight=kinddef#defining-a-kind )

and I am also wondering if this restriction change will get the travis build working ... (even though if any tests will depend on this info they might fail, but then I didn't see any tests added ... yet ;) )

it actually looks like this is the problem and not the NPEs from auth plugins ...
so I think once you surround this block of code with
if (!env.isUniversalCtags()) {
travis will start working ...

Needed to keep tags unknown to Excuberant ctags out of execution.

New signature parsing algorithm picks up more symbols than previous one.

tarzanek · 2017-09-13T09:07:24Z

src/org/opensolaris/opengrok/analysis/Ctags.java

@@ -237,6 +238,28 @@ private void initialize() throws IOException {
            command.add("--regex-pascal=/^(uses|interface|implementation)$/\\1/s,Section/");
            command.add("--regex-pascal=/^unit[[:space:]]+([a-zA-Z0-9_<>, ]+)[;(]/\\1/u,unit/");

+            // PowerShell


this will be a bit hard to merge once we drop exuberant ctags (I was kinda expecting an if which will decide what to do for exctags and what for uctags), but it's an option
and congratulations to working travis! :)

tarzanek · 2017-09-13T09:10:15Z

thank you, these fixes look really good
I spoke to @vladak and he should be back soon (expect a reply in cca week, so stay tuned please) so we can approve the accurev part
I am fully OK with this pull req, so it's just a matter of merging now and I hope that within 10 days most this will be merged (I might ask you for resolving any conflicts once accurev #1721 is merged)

shaehn · 2017-09-13T14:21:16Z

Yes, I will help with the merge if it doesn't go well. (I am crossing my fingers that normal work won't prevent me from doing it. This was done during a lull in local operations). You never answered my previous question above, so I will repeat it here.

Currently, this is not going to fail kotlin programs because there is no signature being gathered by the parser in Ctags.java (at least that is what I think will happen. What I am not sure about is how the 'C' language gets a signature field declared. When I run the universal ctags on a simple .c file, there is no signature field generated. Yet looking at the set of tags when debugging OpenGrok one appears out of thin air, so to speak. Can you explain that to me?)

The reason I ask is that I would like to come up with a fix for that signature parsing section that will handle the various styles of function signatures.

On another topic, I have renamed the folder from posh to powershell as previously suggested. I did not rename the files inside though and left them as Posh... Would this be adequate enough to let anyone else know that the analyzer is for powershell scripts? This means I have another push to do to change the contents on Github.

tarzanek · 2017-09-14T09:59:00Z

sorry, I might have missed the Q
C signatures are definitely taken from ctags,
opengrok logs usually print the command line for ctags (I don't have my test script handy) and one can just take it and run manually (without the daemon param) and the output "tag" file will contain signatures ...
(I will try to get you my sample test script for ctags, but this might not happen sooner than in 4-5 days, I need to restore my old files)

on the renaming - if you can refactor and rename all within this pull request it would be good, if not then we shall rename it later

tarzanek · 2017-09-14T12:06:45Z

got the options (just extracted this one file, so you can use it), which should produce the c signatures (didn't test):

--fields=-anf+iKnS
--c-kinds=+l
--file-scope=yes
-u
--C++-kinds=+l

you can create a file of it and pass it on to universal ctags

tarzanek · 2017-09-18T09:50:01Z

hi @shaehn can you please resolve conflict ?
I think then we can merge this :)

shaehn · 2017-09-18T15:10:46Z

Ok, but I think you will have to explain to me how to go about doing the merge. When I poked the "Resolve conflicts" button, it showed the problem areas, but I could not figure out how to do the resolution from that page. Reading other documentation has not helped much either, because my experience here does not seem to match the documentation. I am going to need some guidance here.

Once the merge is successful and the code is pulled into the main baseline, can I destroy the current branches that I have created so that I can start anew with proper branching from 'main'?

vladak · 2017-09-18T15:16:11Z

The syncing with upstream is described on https://help.github.com/articles/syncing-a-fork/

Yep, once the pull req is merged to OpenGrok/OpenGrok, you can delete your branch.

vladak · 2017-09-18T15:18:59Z

generic info about conflict resolution is here: https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/

basically you edit the file(s) with conflict, change the code to what you think is best outcome from both conflicting sides and erase the delimiters, and once done with the file, git add the file and continue the merge.

shaehn · 2017-09-18T16:15:26Z

Ok, I looked at the mentioned documentation, but my repository is not in the state that is shown there. This is what I have when I enter 'git status' on the command line.

Clearly, I have to do some other git command to get the repository into the conflict state, but I don't know what it is.

Wait a minute. Is syncing-a-fork how I get my repository into the conflict state?

vladak · 2017-09-18T17:29:49Z

Yes, you need to sync with something which is in conflict in order to get into conflict resolution.

You have uncommitted 2 files with changes under src/. Probably you wish to commit these first, assuming you have not started the merge yet.

Also, the nbproject files can be stashed away (see git stash) while you perform the merge etc.

vladak · 2017-09-18T18:14:02Z

You need to read both links - the first one will explain how to sync (which will eventually lead to having conflict in you local repo) and the second one how to resolve the conflicts.

shaehn · 2017-09-18T19:18:12Z

I am getting closer, but not there yet. This is what I tried.

Can you give me explicit command to do the sync? I apparently just don't know the name that this stuff is hiding under. Thanks. (I guess I am kind of surprised there is no explicit set of instructions to do this for OpenGrok. The generic documentation is too generic, without some explicit examples.)

vladak · 2017-09-18T19:38:21Z

fetch just downloads repository metadata but does not really do anything else. You need to follow one of these guides:

It's either git pull or git merge that triggers the merging process.

Also, you can experiment a bit and stage pull requests against your own fork.

vladak · 2017-09-19T08:34:48Z

There are also some pretty nice videos out there that explain the parts of the process in detail, e.g.:

…wershellAddition

tarzanek · 2017-09-19T14:25:33Z

merging, let's give this a whirl

shaehn · 2017-10-06T03:20:06Z

You might want to add a note to your rc-15 release in that it includes powershell analyzer so you and everyone else knows when it got included.

vladak · 2017-10-06T07:10:44Z

Sure, added.

shaehn added 6 commits August 4, 2017 12:24

Have .opengrok_skip_history seen by AccuRev

c7d2640

Changed HistoryGuru::addRepository so that it will recognize .opengrok_skip_history files in folders prior to scanning the folder/directory.

Improvements to AccuRev Repository

055ca4c

Added code to indicate parent stream of user's Accurev workspace. This will be seen on OpenGrok home page.

Incorporated suggested changes

ba21c3b

Removed 1st for-loop which was not doing much of anything. Only directories are considered to be looked at as being repositories, or holders of repository info.

Adding Powershell analyzer oracle#1753

14119b8

This is the initial introduction of the PowerShell script analyzer for OpenGrok.

fixes build break on Travis

a0f4a5b

Hmm... what appears to compile satisfactorily on my machine had issues on Travis.

Trying to fix build attempt number 2

d277c45

Removed greater-than sign from comments

tarzanek reviewed Sep 9, 2017

View reviewed changes

tarzanek reviewed Sep 11, 2017

View reviewed changes

shaehn added 5 commits September 12, 2017 11:01

Attempt number 3 to get past build break

4efb8c4

attempt number 4 build break correction

899fe9a

Needed to keep tags unknown to Excuberant ctags out of execution.

Attempt 5 at fixing build break

dbb92cc

Fixed failing Ctags test

46c2c12

New signature parsing algorithm picks up more symbols than previous one.

Prevent crash when no tags found

61daf02

tarzanek reviewed Sep 13, 2017

View reviewed changes

tarzanek self-assigned this Sep 13, 2017

tarzanek added this to the 1.1 milestone Sep 13, 2017

tarzanek assigned shaehn Sep 13, 2017

shaehn added 2 commits September 14, 2017 11:20

Renamed Posh... filenames to Powershell...

4b125a6

Undid fix for different issue

0961984

Merge branch 'master' of https://github.com/OpenGrok/OpenGrok into po…

78bc0f3

…wershellAddition

tarzanek merged commit e7aa298 into oracle:master Sep 19, 2017

shaehn deleted the powershellAddition branch September 19, 2017 14:40

brunoborges unassigned tarzanek Dec 18, 2017

Powershell addition #1766

Powershell addition #1766

Conversation

shaehn commented Sep 8, 2017

shaehn commented Sep 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaehn Sep 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarzanek Sep 9, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarzanek Sep 9, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaehn Sep 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaehn Sep 12, 2017 • edited

Choose a reason for hiding this comment

tarzanek commented Sep 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarzanek commented Sep 13, 2017

shaehn commented Sep 13, 2017 • edited

tarzanek commented Sep 14, 2017

tarzanek commented Sep 14, 2017 • edited

tarzanek commented Sep 18, 2017

shaehn commented Sep 18, 2017

vladak commented Sep 18, 2017

vladak commented Sep 18, 2017

shaehn commented Sep 18, 2017 • edited

vladak commented Sep 18, 2017 • edited

vladak commented Sep 18, 2017

shaehn commented Sep 18, 2017 • edited

vladak commented Sep 18, 2017

vladak commented Sep 19, 2017 • edited

tarzanek commented Sep 19, 2017

shaehn commented Oct 6, 2017

vladak commented Oct 6, 2017

shaehn commented Sep 8, 2017 •

edited

shaehn Sep 12, 2017 •

edited

tarzanek Sep 9, 2017 •

edited

tarzanek Sep 9, 2017 •

edited

shaehn Sep 11, 2017 •

edited

shaehn Sep 12, 2017 •

edited

shaehn commented Sep 13, 2017 •

edited

tarzanek commented Sep 14, 2017 •

edited

shaehn commented Sep 18, 2017 •

edited

vladak commented Sep 18, 2017 •

edited

shaehn commented Sep 18, 2017 •

edited

vladak commented Sep 19, 2017 •

edited