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

update highlight.js to 9.15.6 #6658

Merged
merged 3 commits into from
Apr 16, 2019
Merged

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Apr 16, 2019

Not sure if this fixes any open issues (it does not address #6057), but I think it's time we update it.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Apr 16, 2019
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 16, 2019
@zeripath
Copy link
Contributor

I would say that I'm not sure how you've built this. (I remember that this was described somewhere and I do trust you not to have put a horrible hack in). however we could do with a good way to compare this to ensure that this is the highlight.pack.js that we should be using.

@techknowlogick
Copy link
Member

@zeripath here is how to build if you want to verify https://highlightjs.readthedocs.io/en/latest/building-testing.html

@silverwind
Copy link
Member Author

silverwind commented Apr 16, 2019

Built using

git clone https://github.com/highlightjs/highlight.js
cd highlight.js
npm i
node tools/build.js
sha256sum build/highlight.pack.js

Should give you SHA256 4b5c3fb194da019662639d15f62574a6f060db510fc6646ffc431fa116dcf086.

The build contains all languages, so the script went from ~450kB to ~670kB.

@zeripath
Copy link
Contributor

zeripath commented Apr 16, 2019

@techknowlogick I've got that but I'm not able to build exactly the same pack - presumably there are a different number of languages in that.

Ah ok from the git clone! Thanks @silverwind !

@techknowlogick
Copy link
Member

I can verify per above commands

@silverwind
Copy link
Member Author

Built on Node.js v11.13.0, but I doubt it matters.

@zeripath
Copy link
Contributor

Thanks @silverwind !

@techknowlogick
Copy link
Member

Hmm.. I'm getting a different sha. I am using nvm so I was able to target the same version of node in use. Let me try a few things.

@zeripath
Copy link
Contributor

zeripath commented Apr 16, 2019

I get a different sha256sum too mine is b251f1a22f401cd82e5e96e1f9247104635bf5e38dc39ef03271a363e9638b82

Wonder why its happening? (❯ node -v [21:58:09]
v10.15.3
)

@silverwind
Copy link
Member Author

Actually the built version is not 9.15.6 but current git master. Not sure if it's worth to revert to the release tag, it seems to work and master contains a few unreleased fixes that might be interesting to have.

@techknowlogick
Copy link
Member

I'm able to match the same sha as @zeripath on latest master commit. For the tagged 9.15.6 version I get 470d9a9fe34579cb4f638bf61aefc9b0d76afbbc90d0888facaa8f9d8e3df914

@silverwind
Copy link
Member Author

silverwind commented Apr 16, 2019

Built on another machine with node 11.14.0, still got 4b5c3fb194da019662639d15f62574a6f060db510fc6646ffc431fa116dcf086 for master.

@techknowlogick
Copy link
Member

@silverwind perhaps we are on different commits for master (in case you cloned/forked it earlier). I'm on commit 2b46620c as master.

@codecov-io
Copy link

codecov-io commented Apr 16, 2019

Codecov Report

Merging #6658 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6658      +/-   ##
==========================================
+ Coverage   40.54%   40.55%   +<.01%     
==========================================
  Files         406      406              
  Lines       54501    54501              
==========================================
+ Hits        22099    22102       +3     
+ Misses      29366    29362       -4     
- Partials     3036     3037       +1
Impacted Files Coverage Δ
models/repo_list.go 66.84% <0%> (-1.06%) ⬇️
modules/log/event.go 65.98% <0%> (+1.52%) ⬆️
models/unit.go 14.28% <0%> (+14.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 38b4c23...85dc75a. Read the comment docs.

@silverwind
Copy link
Member Author

Nope, also on 2b46620c. Just tried in docker and now I get the same SHA as @zeripath, weird:

$ docker run -it --rm node:11 bash
Unable to find image 'node:11' locally
11: Pulling from library/node
e79bb959ec00: Pull complete
d4b7902036fe: Pull complete
1b2a72d4e030: Pull complete
d54db43011fd: Pull complete
69d473365bb3: Pull complete
6e2490ee2dc8: Pull complete
727ad86f629c: Pull complete
23840d79cb44: Pull complete
Digest: sha256:a23014909228ec00666b0ddf5f28638a9fac8f844a972bed831838f6d1c73641
Status: Downloaded newer image for node:11
root@680b3b35f64a:/# git clone https://github.com/highlightjs/highlight.js
Cloning into 'highlight.js'...
remote: Enumerating objects: 5, done.
remote: Counting objects: 100% (5/5), done.
remote: Compressing objects: 100% (5/5), done.
remote: Total 31244 (delta 0), reused 4 (delta 0), pack-reused 31239
Receiving objects: 100% (31244/31244), 8.75 MiB | 8.17 MiB/s, done.
Resolving deltas: 100% (20279/20279), done.
root@680b3b35f64a:/# cd highlight.js
root@680b3b35f64a:/highlight.js# npm i
added 444 packages from 832 contributors and audited 3498 packages in 7.972s
found 4 vulnerabilities (1 low, 1 moderate, 2 high)
  run `npm audit fix` to fix them, or `npm audit` for details
root@680b3b35f64a:/highlight.js# node tools/build.js
Starting build.
Copying documentation.
Writing documentation.
Generating demo.
Building highlight.js pack file.
Compressing highlight.js pack file.
Writing highlight.js pack file.
Finished build.
root@680b3b35f64a:/highlight.js# sha256sum build/highlight.pack.js
b251f1a22f401cd82e5e96e1f9247104635bf5e38dc39ef03271a363e9638b82  build/highlight.pack.js

@zeripath
Copy link
Contributor

Do you have any development versions installed using npm install ? That could be it.

@zeripath
Copy link
Contributor

Prob best to push the reproducible pack up.

@silverwind
Copy link
Member Author

silverwind commented Apr 16, 2019

Can't explain why but the differences are in the minfied variable names:

@@ -16,11 +16,11 @@
 000000f0  65 66 69 6e 65 28 5b 5d  2c 66 75 6e 63 74 69 6f  |efine([],functio|
 00000100  6e 28 29 7b 72 65 74 75  72 6e 20 74 2e 68 6c 6a  |n(){return t.hlj|
 00000110  73 7d 29 29 7d 28 66 75  6e 63 74 69 6f 6e 28 69  |s}))}(function(i|
-00000120  29 7b 76 61 72 20 64 3d  5b 5d 2c 6c 3d 4f 62 6a  |){var d=[],l=Obj|
-00000130  65 63 74 2e 6b 65 79 73  2c 62 3d 7b 7d 2c 73 3d  |ect.keys,b={},s=|
+00000120  29 7b 76 61 72 20 70 3d  5b 5d 2c 6c 3d 4f 62 6a  |){var p=[],l=Obj|
+00000130  65 63 74 2e 6b 65 79 73  2c 62 3d 7b 7d 2c 6d 3d  |ect.keys,b={},m=|
 00000140  7b 7d 2c 74 3d 2f 5e 28  6e 6f 2d 3f 68 69 67 68  |{},t=/^(no-?high|
 00000150  6c 69 67 68 74 7c 70 6c  61 69 6e 7c 74 65 78 74  |light|plain|text|
-00000160  29 24 2f 69 2c 5f 3d 2f  5c 62 6c 61 6e 67 28 3f  |)$/i,_=/\blang(?|
+00000160  29 24 2f 69 2c 75 3d 2f  5c 62 6c 61 6e 67 28 3f  |)$/i,u=/\blang(?|
 00000170  3a 75 61 67 65 29 3f 2d  28 5b 5c 77 2d 5d 2b 29  |:uage)?-([\w-]+)|
 00000180  5c 62 2f 69 2c 72 3d 2f  28 28 5e 28 3c 5b 5e 3e  |\b/i,r=/((^(<[^>|
 00000190  5d 2b 3e 7c 5c 74 7c 29  2b 7c 28 3f 3a 5c 6e 29  |]+>|\t|)+|(?:\n)|

Seems their build process is non-deterministic, but I guess it's no big deal. I'll push the version built in docker.

@zeripath
Copy link
Contributor

No problems. Thanks again @silverwind.

@silverwind
Copy link
Member Author

Pushed the one with b251f1a22f401cd82e5e96e1f9247104635bf5e38dc39ef03271a363e9638b82 and updated version to commit hash.

@techknowlogick techknowlogick added this to the 1.9.0 milestone Apr 16, 2019
@techknowlogick techknowlogick added the topic/ui Change the appearance of the Gitea UI label Apr 16, 2019
@techknowlogick techknowlogick merged commit 867ad49 into go-gitea:master Apr 16, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants