Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Embed catapult as submodule #1614

Open
dayo09 opened this issue Aug 8, 2023 · 9 comments
Open

Embed catapult as submodule #1614

dayo09 opened this issue Aug 8, 2023 · 9 comments

Comments

@dayo09
Copy link
Contributor

dayo09 commented Aug 8, 2023

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 #1613 (comment)

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 #1613 (comment)

@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 #1613 (comment)

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 8, 2023

@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.

@lemmaa
Copy link
Member

lemmaa commented Aug 8, 2023

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 8, 2023

@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
Copy link
Contributor Author

dayo09 commented Aug 8, 2023

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.

@lemmaa
Copy link
Member

lemmaa commented Aug 8, 2023

#1614 (comment)

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

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.)

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 9, 2023

@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 #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. (#1612 seems to be missing it.)

Sure :-D thanks for the point!

@lemmaa
Copy link
Member

lemmaa commented Aug 9, 2023

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.

@dayo09
Copy link
Contributor Author

dayo09 commented Aug 11, 2023

@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 Add catapult-trace2html dependency #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?

@lemmaa
Copy link
Member

lemmaa commented Aug 16, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants