ONE-vscode icon indicating copy to clipboard operation
ONE-vscode copied to clipboard

Embed catapult as submodule

Open dayo09 opened this issue 1 year ago • 9 comments

What?

Let's embed catapult as a submodule.

Beforehand, let's double check its license, usage, and deployment strategy.

Comment history

Moved from [third_party] Add catapult as submodule

@dayo09 , have you checked the license for the cataplut project? If you want to introduce an external module, it is better to create a separate issue for it, check and organize all matters including the license, and proceed with PR only after taking appropriate measures.

Let's create an issue first!

Originally posted by @lemmaa in https://github.com/Samsung/ONE-vscode/issues/1613#issuecomment-1668934482

When I checked the license of catapult module, this project is BSD-3-Clause license even though the LICENSE file in the catapult-project is Chromium. SOSHub said that we can use this project under the license compliance.

https://opensource.sec.samsung.net/view/search?searchword=Y2F0YXB1bHQ

/cc @lemmaa

Originally posted by @jyoungyun in https://github.com/Samsung/ONE-vscode/issues/1613#issuecomment-1668941245

@jyoungyun , my other concern is when deploying. Actually, this tool should be distributed and used. What kind of package structure will it have then?

Originally posted by @lemmaa in https://github.com/Samsung/ONE-vscode/issues/1613#issuecomment-1668948429

dayo09 avatar Aug 08 '23 05:08 dayo09

@lemmaa As @jyoungyun said, I checked that the license is okay.

About deployment, yes it must include the whole repository for runtime service.

I think another possible approach is to make the 3000+ lines of trace2html of catapult as an npm package and I put it to the npm registry.

dayo09 avatar Aug 08 '23 06:08 dayo09

@dayo09 how about https://www.npmjs.com/package/traceviewer or https://www.npmjs.com/package/trace2html-cli ?

lemmaa avatar Aug 08 '23 06:08 lemmaa

@lemmaa Thanks for the suggestion! But it didn't work for me.

Beforehand, I tried traceviewer and it's missing some directories (py_vulcanize) that trace2html uses. Relacted comment

Now I tried trace2html-cli and got the similar problem.

./node_modules/trace2html-cli/catapult/tracing/bin/trace2html ./res/samples/traces/sample.timeline.json 
Traceback (most recent call last):
  File "/home/dayo/git/ONE-vscode/./node_modules/trace2html-cli/catapult/tracing/bin/trace2html", line 13, in <module>
    from tracing_build import trace2html
ModuleNotFoundError: No module named 'tracing_build'

catapult/tracing/tracing_build directory exists in an original project, but it is not included in this npm distribute for some reason.

dayo09 avatar Aug 08 '23 09:08 dayo09

I stripped the source code to include the files required to run trace2html. (I am not sure that it's perfectly stripped, but it has sized-down).

You can see the stripped version here : #1612.

It's removed some build/infra/test files and third party tools. I may distribute this version of catapult with npm registry.

dayo09 avatar Aug 08 '23 09:08 dayo09

https://github.com/Samsung/ONE-vscode/issues/1614#issuecomment-1669238696

Yes, it seems that the last publish of both projects was outdated 7 years ago. It wasn't a good choice. :(

Anyway, as long as we follow LICENSE well, I don't think it matters either way.

Therefore, the next thing to consider is the convenience of distribution and installation. In my rough measurements, this package was around 5MB~6MB. I expect #1612 the one you stripped off is probably smaller. Depending on which method you choose, consider the following.

1. Installing with npm

  • When installing/uninstalling an extension, is it natural for it to be installed/uninstalled as an npm package?
  • Wouldn't it be burdensome to publish and manage this as an npm package?
  • Are there any difficulties in testing?
  • ...

2. Included in the extension package as a submodule

  • When packaging, how can we filter out the necessary parts like #1612 from the entire source?
  • Will there be any difficulties in testing?
  • ...

In both cases, the catapult project's top-level LICENSE file(https://github.com/catapult-project/catapult/blob/main/LICENSE) will need to be distributed together. (#1612 seems to be missing it.)

lemmaa avatar Aug 08 '23 10:08 lemmaa

@lemmaa

0. Project size

  • before strip : 1.2G
du -h --max-depth=1 .                                                        1 ↵  9437  12:46:19
60K     ./infra
5.9M    ./dashboard
160K    ./catapult_build
12K     ./bin
228K    ./trace_processor
7.5M    ./telemetry
836K    ./systrace
1.6M    ./devil
664K    ./netlog_viewer
223M    ./tracing
169M    ./dist
5.3M    ./experimental
16K     ./hooks
441M    ./.git
330M    ./third_party
128K    ./skia_bridge
5.8M    ./common
1.2M    ./docs
4.0K    ./build
200K    ./web_page_replay_go
140K    ./perf_issue_service
112K    ./.cipd
256K    ./dependency_manager
  • after strip : 29M
 du -h --max-depth=1 .                             1 ↵  9447  12:49:45
13M     ./tracing
16M     ./third_party
104K    ./common

1. Installing with npm

When installing/uninstalling an extension, is it natural for it to be installed/uninstalled as an npm package?

As ONE-vscode's dependencies are managed with npm, it's easy to use current test / deployment stages. It can be added as 'dependencies' (not dev-dependencies) slot so that it is used by extension users.

Wouldn't it be burdensome to publish and manage this as an npm package?

It seems once it's published, there is no addtional / regular distribution required to maintain the package. (I mean, no expiration date exists in npm registry)

And publishing npm registry looks so simple, you need to only configure npm package.json to a minimal level and hit 'npm login '& 'npm publish'

However, catapult-project/catapult git repository is actively maintained, so we may need to update it for a new features someday. I will leave a bash script so that we can do it easily someday.

Are there any difficulties in testing?

I plan to test it inside our extension by simply running the trace2html and checking if the output comes. It can be done by our current mocha testing infra.

2. Included in the extension package as a submodule

When packaging, how can we filter out the necessary parts like https://github.com/Samsung/ONE-vscode/pull/1612 from the entire source? Will there be any difficulties in testing?

There is no way I found to use a submodule in fragment. I guess in many part npm distribution is much reasonable. We are now managing all of our dependencies with npm, so there will be much advantage in maintenance.

In both cases, the catapult project's top-level LICENSE file(https://github.com/catapult-project/catapult/blob/main/LICENSE) will need to be distributed together. (https://github.com/Samsung/ONE-vscode/pull/1612 seems to be missing it.)

Sure :-D thanks for the point!

dayo09 avatar Aug 09 '23 03:08 dayo09

FYI, I'm guessing https://github.com/catapult-project/catapult/blob/main/tracing/trace_viewer.gni is the full list of files required by trace_viewer.

lemmaa avatar Aug 09 '23 04:08 lemmaa

@lemmaa @jyoungyun

I have published catapult-trace2html project, which is a stripped catapult project version only for trace2html tool.

image

Git repository

https://github.com/dayo09/catapult-trace2html

  • There are git tags in sync with npm publish.
  • I added the project discription into README.md and attached the original README.md below.

NPM Registry

https://www.npmjs.com/package/catapult-trace2html?activeTab=readme

  • I used my personal gmail.
  • I checked that it works well with the DRAFT #1616
  • LICENSE The npm package's license is set to BSD-3-Clause (itentical to the original catapult project)
  • AUTHORS, OWNERS files are remained.
    • [ ] Q. Should I remove 'OWNERS' as it's not directly owned by the origianal authors?

dayo09 avatar Aug 11 '23 08:08 dayo09

AUTHORS, OWNERS files are remained.

  • [ ] Q. Should I remove 'OWNERS' as it's not directly owned by the origianal authors?

According to the third term of the BSD-3 Clause license. Maybe. I guess. Anyway, why not contact the open source group?

Also, the current distribution method may not be a problem temporarily, but I do not know if it will be appropriate in the long term. I hope you consider how we can distribute it, not as individuals, but from a project perspective, without taking on any special guarantees or responsibilities.

lemmaa avatar Aug 16 '23 07:08 lemmaa