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

added UAST diff algorithm#309

Closed
quinor wants to merge 12 commits into
bblfsh:masterfrom
quinor:master
Closed

added UAST diff algorithm#309
quinor wants to merge 12 commits into
bblfsh:masterfrom
quinor:master

Conversation

@quinor
Copy link
Copy Markdown

@quinor quinor commented Sep 21, 2018

still WIP, diffing works, applying diff not implemented yet

Comment thread uast/diff/changelist.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/apply.go Outdated
Comment thread uast/diff/apply.go Outdated
Comment thread uast/diff/changelist.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
keys := make(map[string]bool)
iterate := func(keyset nodes.Object) {
for key := range keyset {
if in := keys[key]; !in {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment still applies

Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/diff.go Outdated
@dennwc
Copy link
Copy Markdown
Member

dennwc commented Sep 24, 2018

Also, it would be nice to have some tests for diff generation as well as for appling the diff.

@quinor
Copy link
Copy Markdown
Author

quinor commented Oct 27, 2018

How do we add tests in the repository? I've got some internal ones.

@dennwc
Copy link
Copy Markdown
Member

dennwc commented Oct 30, 2018

@quinor You can add a test function to a new file ./uast/diff/diff_test.go. CI will call it automatically.

@quinor
Copy link
Copy Markdown
Author

quinor commented Oct 30, 2018

@dennwc how to proceed if the test(s) need serialized data or external resources to work? My current testing procedure is to launch a client that runs the diff on two files and tries to apply it afterwards comparing the result with source, additionally having bblfshd docker running for the source file parsing.

@dennwc
Copy link
Copy Markdown
Member

dennwc commented Oct 30, 2018

@quinor Feel free to add a fixtures or testdata folder and add few YAML UAST files to it. Tests can read and diff those files. No need to run the whole client+bblfshd chain.

To get those YAMLs you can run the bblfsh-cli included in the latest Go client. It can connect to bblfshd, parse files and emit the YAML/binary formats.

@quinor
Copy link
Copy Markdown
Author

quinor commented Oct 30, 2018

Is there any documentation/examples for the go+CI we are using?

@dennwc
Copy link
Copy Markdown
Member

dennwc commented Oct 30, 2018

@quinor Don't think about CI right now, you can use go test ./... to test it locally. The same command will be executed by CI.

@quinor
Copy link
Copy Markdown
Author

quinor commented Oct 30, 2018

Did not realize there is an unified "go test" framework. That simplifies things alot :)

Comment thread uast/diff/apply.go Outdated
Comment thread uast/diff/apply.go Outdated
Comment thread uast/diff/changelist.go
@quinor quinor force-pushed the master branch 2 times, most recently from b2681a8 to c65e770 Compare November 10, 2018 16:01
Comment thread uast/diff/apply.go Outdated
Comment thread uast/diff/apply.go Outdated
Comment thread uast/diff/apply.go
Comment thread uast/diff/apply.go Outdated
Comment thread uast/diff/changelist_test.go Outdated
Comment thread uast/diff/diff.go Outdated
Comment thread uast/diff/testdata/create_testdata.py Outdated
Comment thread uast/diff/testdata/create_testdata.py Outdated
@@ -0,0 +1,36 @@
#!/usr/bin/env python3
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The script is pretty simple. Can be written in Go as well (it's the only Python file here).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I only threw it in so that one may try to regenerate/generate new test cases. Also, see the answers below.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still think it makes sense to rewrite it in Go. All the SDK is in Go. I see no reason to require Python installation for such a simple script.

@@ -0,0 +1,148 @@
720_1
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe there is a formal definition for this? For example "source size is less than N bytes/lines" or "the UAST size is less than N bytes/nodes"?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not sure how I'd chosen these. If there was such definition, how to proceed?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The script can check the file size before writing it to the destination.

By the way, if the script was in Go, you can use the Babelfish client directly and count nodes / check the size of the source or the UAST.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The reason I wrote it in Python is I'm not really comfortable with file-processing scripts in Go. Python seemed like a better alternative to bash (that's what it would be written in otherwise).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Also, Python exists on every modern Linux distro so it's still quite portable (see the alternative: bash)

Comment thread uast/diff/testdata/create_testdata.py Outdated
@quinor quinor force-pushed the master branch 2 times, most recently from 025d8bb to 8c1149e Compare December 12, 2018 10:32
@dennwc
Copy link
Copy Markdown
Member

dennwc commented Mar 6, 2019

Needs a rebase on top of a current master at least.

Also make sure to run go fmt ./...

Wojciech Jabłoński added 9 commits March 24, 2019 16:09
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Wojciech Jabłoński added 2 commits March 24, 2019 16:51
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
Signed-off-by: Wojciech Jabłoński <wj359634@students.mimuw.edu.pl>
@dennwc dennwc mentioned this pull request Apr 11, 2019
@dennwc
Copy link
Copy Markdown
Member

dennwc commented Apr 11, 2019

Have to close this PR - cannot rebase, because it's made from the master branch on the fork. See #396.

@dennwc dennwc closed this Apr 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants