Enable Github depend bot to scan for Javascript vulnerability

[antonysigma]

Add a dummy javascript library dependencies file "package.json" and the book-keeping file "yarn.lock" to monitor the IRVisualizer tool for outdated dependencies. Activate Github's Dependbot notification for any security vulnerabilities and virus.

Upgrade the JQuery library to v3.7.0 .

cc'ed @maaz139 and @mcourteaux .

[mcourteaux]

The security check seems useful, but the extra build dependency on yarn seems very unpleasant. Is this mandatory, or only to update the yarn.lock file? What's the purpose of that file anyway? Doesn't Github check the dependencies through the package.json?

[antonysigma]

Is this mandatory, or only to update the yarn.lock file? What's the purpose of that file anyway? Doesn't Github check the dependencies through the package.json?

Thank you for the feedback. Okay. Removing the yarn.lock file.

[mcourteaux]

@steven-johnson This PR was made concurrently with mine (#7755) and unintentionally reverts my changes (due to some refactoring and code moving from one file to another it seems). Please don't merge, until this is addressed.

[steven-johnson]

Thanks for the heads-up :-)

[mcourteaux]

@steven-johnson @antonysigma I am very sorry for the confusion. I saw some code in the file that I was confident that I deleted (the whole part about init_tooltips). But it seemed I just forgot to delete that code. I changed the tooltips to be CSS-only, which warrants deleting some javascript, which I forgot it seems. So not outdated! Apologies!

[steven-johnson]

So, can we clarify: is this ready to review and/or land?

[mcourteaux]

I haven't tested this PR yet. I could try that tomorrow, it's late in Belgium right now. :)

[mcourteaux]

Okay gonna test now!

[mcourteaux]

Okay, I can confirm it works. I have no clue if this will actually make GitHub keep an eye on this, but at least it doesn't break anything as far as I can tell.

[mcourteaux]

LGMT Minor comment: see below.

[steven-johnson]

LGTM pending green buildbots

[mcourteaux]

@antonysigma Why is there a symlink from the template.js to the main.js? If you just rename the file, Git will keep track of the commit history as it will treat it as a rename. Using a symlink breaks it.

Also, I believe symlinks will break this on Windows??

Also, maybe the indent can be undone in main.js?

[antonysigma]

Why is there a symlink from the template.js to the main.js? Also, I believe symlinks will break this on Windows??

It was an attempt to rename all templates files from *.template.html to template/*.[js|css|html]. Agreed it is a scope creep. Okay. I changed it back. The PR should be now simplier.

Also, maybe the indent can be undone in main.js?

Not sure if auto-formatting will contaminate the git blame in the commit history. Deferring to @steven-johnson .

If you wish, I can add a dummy clang-format style config file to this PR, risking a scope creep. Clang-format is capable of auto-intenting ECMAScript up to ES6.

[mcourteaux]

@antonysigma I liked the .js extension you made. So putting the <script> tag around it and having HTML feels inferior to what you originally had. You can just rename this file from *.html to *.js and Git should understand.

Just to complete: right now, you also have a nested <script> tag, because it's once in the HTML file, and once in the C++ generator code.

[mcourteaux]

So, to be clear what I mean: if you rename this file to be StmtToViz_javascript.template.js, and remove the <script> tag, I think it'd be ideal.

[antonysigma]

I am afraid it will upset both the Makefile and CMake toolchain, and I don't know CMake enough to make the changes. I added your request to the Todo list in the Readme. Currently, my primary goal is to activate the Dependabot.

May I rely on you to rename the template files without upsetting Makefile and CMake? Run git grep html_template and you will know what I mean. Also, the bin2c manual is a nice reference; I guess the core-Halide developers reverse engineered bin2c to binary2cpp for Windows users.

[mcourteaux]

Sure! I see. Let's keep it simple right now and keep the .html extension.

[mcourteaux]

@antonysigma I see what you meant with this:

It was an attempt to rename all templates files from *.template.html to template/*.[js|css|html]. Agreed it is a scope creep. Okay. I changed it back. The PR should be now simplier.

We can turn this into a *.js file, if we add a build rule that does the binary2cpp for the JS file here:

Halide/Makefile

Lines 1190 to 1191 in acc9413

	$(BUILD_DIR)/html_template.%.cpp: $(BIN_DIR)/binary2cpp $(SRC_DIR)/irvisualizer/%.template.html
	./$(BIN_DIR)/binary2cpp halide_html_template_$* < $(SRC_DIR)/irvisualizer/$*.template.html > $@

and here:

Halide/src/CMakeLists.txt

Lines 362 to 399 in acc9413

	set(HTML_TEMPLATE_FILES
	StmtToViz_dependencies
	StmtToViz_javascript
	StmtToViz_stylesheet
	)
	##
	# Build and import the runtime.
	##
	add_subdirectory(runtime)
	##
	# Build the template files via binary2cpp.
	##
	add_library(Halide_c_templates OBJECT)
	foreach (f IN LISTS C_TEMPLATE_FILES)
	set(SRC "$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/${f}.template.cpp>")
	set(DST "c_template.${f}.template.cpp")
	add_custom_command(OUTPUT "${DST}"
	COMMAND binary2cpp "halide_c_template_${f}" < "${SRC}" > "${DST}"
	DEPENDS "${SRC}" binary2cpp
	VERBATIM)
	target_sources(Halide_c_templates PRIVATE ${DST})
	endforeach ()
	foreach (f IN LISTS HTML_TEMPLATE_FILES)
	set(SRC "$<SHELL_PATH:${CMAKE_CURRENT_SOURCE_DIR}/irvisualizer/${f}.template.html>")
	set(DST "html_template.${f}.template.cpp")
	add_custom_command(OUTPUT "${DST}"
	COMMAND binary2cpp "halide_html_template_${f}" < "${SRC}" > "${DST}"
	DEPENDS "${SRC}" binary2cpp
	VERBATIM)
	target_sources(Halide_c_templates PRIVATE ${DST})
	endforeach ()

But maybe that's a little too elaborate just to change a file extension...

[mcourteaux]

To conclude this lenghty nitpicking discussion, let's keep the <script> tag right now, to make syntax highlighting work, because the html extension throws off the highlighters without the script-tag.

[mcourteaux]

@steven-johnson LGTM! I think this can land. 😄 🎉

[antonysigma]

let's keep the <script> tag right now, to make syntax highlighting work, because the html extension throws off the highlighters without the script-tag.

Done. Kindly asking @alexreinking to debug the CI build failure. I rebased the changes on top of v14.0.0-534-gacc941308, not sure what went wrong.

[mcourteaux]

@antonysigma Thanks for taking the time here! I hope I wasn't too annoying 😝 Cool to see how the PR size went down over time. 😄

[steven-johnson]

This looks good, except we really must have a CMake-based build script for this as well; as you know, CMake is the reference build implementation for Halide, and we expect everything in it to be buildable via CMake. (Make support is provided on a best-effort basis by the contributors who care about it, but not all Halide features are support via Make, e.g. the Python bindings.)

[antonysigma]

Hi @steven-johnson , acknowledged the need to integrate all build process into Makefile and CMake. Since this PR's primary goal is to activate Github Dependabot, not to build anything, may I remove the Makefile to get this PR accepted?

The roadmap to trans-compile javascripts into stmt_html with Make/CMake integration is mentioned in the Readme.

[steven-johnson]

Let me see if I can put together a solution so we can land this

[steven-johnson]

OK, looking at this further, I don't think we need to bother with CMake -- TBH, the Makefile is so trivial even it could basically be moved into the README (but it's ok to leave it in).

That said, what I think does need to be added to the README is what tools are assumed to be installed on your system -- e.g. yarn, which may be present on all JavaScript expert's systems, but definitely wasn't present on mine (nor was Node, for that matter). Also eslint? etc etc

[steven-johnson]

ok, after doing brew install node yarn eslint esbuild on my Mac laptop, doing make lint results in:

$ make lint
yarn run lint
yarn run v1.22.19
$ eslint StmtToViz_javascript.template.html

/Users/srj/GitHub/Halide/src/irvisualizer/StmtToViz_javascript.template.html
  1:1  error  Parsing error: Unexpected token <

✖ 1 problem (1 error, 0 warnings)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
make: *** [lint] Error 1

[antonysigma]

ok, after doing brew install node yarn eslint esbuild on my Mac laptop, doing make lint results in:

eslint StmtToViz_javascript.template.html

/Users/srj/GitHub/Halide/src/irvisualizer/StmtToViz_javascript.template.html
  1:1  error  Parsing error: Unexpected token <

Tagging @mcourteaux for the package.json roadmap discussion.

Steven has a question about the "redundant" <script> tag failure from the eslint output. Eslint is the javascript static analyzer. He suggests we remove the Makefile, do you agree? The eslint path/to/template/StmtToViz_main.js idea with the *.js extension certainly can wait. I recalled you also have another PR #7793 to land asap, right? The Makefile and eslint idea can wait.

cc'ed @steven-johnson .

[mcourteaux]

Tagging @mcourteaux for the package.json roadmap discussion.

Steven has a question about the "redundant" <script> tag failure from the eslint output. Eslint is the javascript static analyzer. He suggests we remove the Makefile, do you agree? The eslint path/to/template/StmtToViz_main.js idea with the *.js extension certainly can wait. I recalled you also have another PR #7793 to land asap, right? The Makefile and eslint idea can wait.

@antonysigma @steven-johnson Right now, I still haven't run any of eslint, yarn, node or esbuild. My only requirement was that we do not need any of these tools to actually build Halide and the generators. Relying on any of these tools for in actual production use, would be a major turn-off for me. I was assuming these tools are nice-to-haves when actually working on the code of this HTML/JS stuff to get some more assistance, comfort of development, and static analysis reports.

So, to answer the question "agree to remove the Makefile?" Yes! The Makefile right now does not build anything it seems, but just describes the tools you could use to get development assistance, if I interpret the situation correctly. To be honest, I don't see the benefit of a make watch over a yarn run watch (besides the automatic triggering of yarn install). This indeed should go in the README and probably in human readable format, instead of Makefile format. The benefit of a Makefile here seems minimal.

What does seem useful is the static analysis (as most of us are used to compile-time validation of your code, instead of crashing at runtime), which could be automatically validated by the buildbots. But overall, this seems like a separate feature from the original idea of simply listing the dependencies to have an automatic scan from Dependabot.

On overall roadmap (potentially several bite-sized PRs) could be:

1. Enable depandabot now by adding package.json (basically, the core bit of this PR).
2. Rename the template files to have their proper extensions, e.g., *.hmtl, *.js, *.css and make sure all the Makefile and CMake build rules are up to date. Meanwhile, refactor the <script> tag to be generated by the C++ code.
3. Accommodate an individual developer to run the JS/HTML specific linting tools, should they wish. This becomes more straightforward with the proper filetypes in place as per (2).
4. Integrate linting as an OPTIONAL step in the build process in CMake and Makefile, and enable this in the buildbots.
5. Once linting is an optional step in the build procedure (and the webtech tools are available on the build system), we can check out if we can produce these amalgamated offline builds of the HTML file which has the dependencies baked in.

Right now, this PR seems like it's grabbing bits of 1 and 3 without first solving 2. In the long term, I think reaching step 4 seems beneficial, as now PRs are mostly eyeballed and quickly tested to see if stuff still functions, but tools like this will probably improve the review process.

[mcourteaux]

To add to this, I'm willing to fix step 2, if this PR get trimmed to cover step 1. If we agree to proceed this way, then please do keep your work around because this yarn and eslint stuff seems useful, and you seem to know how it works (I don't... To be honest, I'm not sure what yarn is for, as I haven't needed it and everything did work. I do understand what the linting does tho 😄 ), so you or I could use it as reference material to proceed with step 3 of my proposed roadmap, once we have reached step 2.

[antonysigma]

Great! Deal! @steven-johnson I moved the Makefile contents to README.md. The PR is ready to land.

[steven-johnson]

At this point I will defer to @mcourteaux for approval.

[mcourteaux]

I was already getting annoyed by the way this JS stuff works. Was looking to get yarn to test the suggested commands in the README. Yarn seems to be a package manager, which you use to install the dependencies. Turns out, to install yarn you need corepack (some sort of module/package manager), that comes with npm, which is a package manager. 🤯 What? So I need package manager (apt) to install a package manager (npm), to install an alternative experimental package manager corepack, to install another package manager yarn, to download three small scripts which are hosted online? All of this to replace a couple of <script src=""> tags?

If this is the price we pay to get the generated HTML to be functioning offline, I'm not sure if this is the right path to take. To be honest, I'd much rather replace the jQuery, and bootstrap stuff with some simple Javascript and CSS. For the assembly syntax highlighting, there are two lightweight options:

• it will then only work if you have internet, or the file is cached by your browser.
• we just include this really small file in the repository (it's literally 2kB: https://cdn.jsdelivr.net/gh/speed-highlight/core/dist/index.js), which is possible, as the project is licensed CC0.

This seems like a reasonable compromise for a repository like Halide, as we'd stay out of the JS madhouse.

Any thoughts @steven-johnson @antonysigma? I hope I'm not being difficult, but I really feel like this stuff does not belong in a repo like Halide.

[steven-johnson]

So I need package manager (apt) to install a package manager (npm), to install an alternative experimental package manager corepack, to install another package manager yarn, to download three small scripts which are hosted online?

It's package managers all the way down

If this is the price we pay to get the generated HTML to be functioning offline, I'm not sure if this is the right path to take.

I wish I could express useful thoughts about this, but my knowledge of "modern JS/HTML packaging" is, uh, ~20 years out of date -- I don't know what the current expectations are. (I do agree that needing to install multiple JS-specific tools seems like a surprising and suboptimal burden.)

we just include this really small file in the repository (it's literally 2kB: https://cdn.jsdelivr.net/gh/speed-highlight/core/dist/index.js), which is possible, as the project is licensed CC0.

I this that CC0 would OK to include -- based on my reading of things it's considered "unencumbered", so as long as we add the right info to our LICENSE.txt we should be ok. That said: adding @abadams @jrk to see if they have opinions/concerns about this.

[abadams]

My javascript knowledge is also rudimentary at best, but I'll say I'm strongly in favor of minimizing the number of dependencies, and CC0 looks fine to me.

[antonysigma]

I can sense a subtle inclination to revert PR #7056 and #7421, so that stmt_html will remain as a syntax-highlighted, printable program listing. If that's the case, you all have my support, and let me retract the PR.

Hi @mcourteaux looks like we are not getting Dependabot with this PR. Please feel free to submit your own PR. The CVE concerns about the jQuery 1.x needs to be resolved. You are also welcome to take the bits and pieces of this PR to get your work done. Examples are: .eslint.js to flush out bugs in the Javascript; .clang-format to auto-format the javascript code.

[antonysigma]

Hi @abadams and @steven-johnson , I realized the PR to activate Github Dependabot, meant to reduce your review time, and to build confidence on the Single Page Web Application (SPA), managed to destroy both. Sorry to diverting all the attention from other more critical PRs.

@mcourteaux 's concerns about the SPA and the lack of governance is legitimate. No matter I like it or not, with PRs #7056 and #7421, Halide's developers inadvertently embraced SPA built into the IR Visualizer design. stmt_html is no longer a pretty-printed program listing with syntax highlighting, but now becomes an SPA because it has hot path analysis, IR-to-assembly code seeking functions, as well as data movement + compute cost estimates in real time.

SPA is notorious for its lack of governance, and accelerated code rot. I roughly estimated 1.5 year decay half life of a typical SPA module. Once we embrace the SPA architecture, we will either need automation (e.g. Github Dependabot) or manual labor to keep it under control. Or, like @mcourteaux suggested, the Halide team can always revert stmt_html back to the static program listing output.

Similar projects encountered the "foreign/alien" SPA technology from the volunteers, and merged the PRs. But, they quickly grow wary of added complexity and extra maintenance. Each project eventually comes up with their own unique solutions. Feel free to reach out to them for advice. Examples are:

• Jupyter Notebook: [Discuss] The future of the classic notebook interface jupyter/notebook#6210 (comment)

• Upgrade jQuery to 3.5.x for Notebook 5.x jupyter/notebook#6469

• Open hardware testing framework https://github.com/google/openhtf/tree/master/openhtf/output/web_gui

• web_gui build is broken google/openhtf#1039

[mcourteaux]

@antonysigma Thank you for the very insightful comment about SPAs.

I will attempt to do a major cleanup as I try to tackle #7519. Ideally, I'd like to kick out:

• Bootstrap & bootstrap icons (there really are no icons: it's just + and -)
• jQuery
• VizIR
• Treeflex
• Most of the JavaScript (without losing the functionality).

This should bring back the StmtHTML to a simple and clean page. I want to stress that I will keep the following aspects/features:

• collapsible subtrees (like for/if/func, etc...)
• cost model (although I wonder if there is any reason to express the memory in bits rather than bytes? are the DSPs out there where it doesn't make sense to use the word byte?)
• the split panels between Stmt and Assembly/PTX
• jump to assembly buttons

Additionally, I figured out that the split panels are very slow because of how the overflow of the content of the panels in the horizontal direction is handled. I believe I will be able to finally speed this aspect up as well.

[mcourteaux]

I can sense a subtle inclination to revert PR #7056 and #7421, so that stmt_html will remain as a syntax-highlighted, printable program listing. If that's the case, you all have my support, and let me retract the PR.

I am very surprised to see #7056, and in particular the branch halide:darya-ver/ir-viz. This PR (which went not into halide:main) is way more involved than what currently sits in main. This branch is 64 commits ahead of main, and @maaz139 seemed to be working on this. Now, I am not sure what the goal is, or what the aim is for this branch.

Can @maaz139 elaborate on the use of the VizIR HTML? As you are maintaining this, I assume you use it and have a use case for it? Was the goal to merge this into main eventually? Seeing this makes me feel uncomfortable saying I want to throw it all out 😬. I am legitimately confused, as I contributed stuff such as #7713 #7793 #7755, which totally diverges away from the work you have in halide:darya-ver/ir-viz.

[mcourteaux]

I believe @maaz139 might be on holidays or something. In the meanwhile, @steven-johnson do you know anything about the work on the halide:darya-ver/ir-viz branch? (Read last comment for more details.)

[steven-johnson]

No, I don't know the current status.

[abadams]

The commits in that branch also seem to be part of #7421, which is merged, so I wouldn't worry about it:

[mcourteaux]

The commits in that branch also seem to be part of #7421, which is merged, so I wouldn't worry about it:

The branch was merged in the past, but has since gotten 64 additional commits, which were not merged.

[abadams]

But looking at those commits, they have the same descriptions as the commits in #7421, so I think they're duplicates or something.

[maaz139]

Apologies for the slow response. I am currently not working on the visualizer although I am happy to help review / implement important changes. I'm not sure what happened here but the commits on #7421 have already been merged. I will close that branch.

[mcourteaux]

Apologies for the slow response. I am currently not working on the visualizer although I am happy to help review / implement important changes. I'm not sure what happened here but the commits on #7421 have already been merged. I will close that branch.

Okay! Cool. I'll give it a shot! It seems that @darya-ver has done a lot of work that went beyond the PR. You can see a demo of what she did here: https://darya-ver.github.io/files/project_files/visualize_halide_example_blur3x3.html and reflects the screenshot in #7056. So I'm not sure what's up with that.