-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Modernize and run eslint on tools/lz4-compress.js. NFC #15846
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,157 +1,60 @@ | ||
| #!/usr/bin/env node | ||
| // Copyright 2015 The Emscripten Authors. All rights reserved. | ||
| // Emscripten is available under two separate licenses, the MIT license and the | ||
| // University of Illinois/NCSA Open Source License. Both these licenses can be | ||
| // found in the LICENSE file. | ||
|
|
||
| // *** Environment setup code *** | ||
| var arguments_ = []; | ||
| var debug = false; | ||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| var ENVIRONMENT_IS_NODE = typeof process === 'object'; | ||
| var ENVIRONMENT_IS_WEB = typeof window === 'object'; | ||
| var ENVIRONMENT_IS_WORKER = typeof importScripts === 'function'; | ||
| var ENVIRONMENT_IS_SHELL = !ENVIRONMENT_IS_WEB && !ENVIRONMENT_IS_NODE && !ENVIRONMENT_IS_WORKER; | ||
| const arguments_ = process['argv'].slice(2); | ||
| const debug = false; | ||
|
|
||
| if (ENVIRONMENT_IS_NODE) { | ||
| // Expose functionality in the same simple way that the shells work | ||
| // Note that we pollute the global namespace here, otherwise we break in node | ||
| print = function(x) { | ||
| process['stdout'].write(x + '\n'); | ||
| }; | ||
| printErr = function(x) { | ||
| process['stderr'].write(x + '\n'); | ||
| }; | ||
|
|
||
| var nodeFS = require('fs'); | ||
| var nodePath = require('path'); | ||
|
|
||
| if (!nodeFS.existsSync) { | ||
| nodeFS.existsSync = function(path) { | ||
| try { | ||
| return !!nodeFS.readFileSync(path); | ||
| } catch(e) { | ||
| return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| function find(filename) { | ||
| var prefixes = [nodePath.join(__dirname, '..', 'src'), process.cwd()]; | ||
| for (var i = 0; i < prefixes.length; ++i) { | ||
| var combined = nodePath.join(prefixes[i], filename); | ||
| if (nodeFS.existsSync(combined)) { | ||
| return combined; | ||
| } | ||
| } | ||
| return filename; | ||
| } | ||
|
|
||
| read = function(filename, binary) { | ||
| filename = nodePath['normalize'](filename); | ||
| var ret = nodeFS['readFileSync'](filename); | ||
| // The path is absolute if the normalized version is the same as the resolved. | ||
| if (!ret && filename != nodePath['resolve'](filename)) { | ||
| filename = path.join(__dirname, '..', 'src', filename); | ||
| ret = nodeFS['readFileSync'](filename); | ||
| } | ||
|
Comment on lines
-52
to
-56
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this part not necessary anymore?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, this was just boilerplate copied from other scripts. Its not needed there. |
||
| if (ret && !binary) ret = ret.toString(); | ||
| return ret; | ||
| }; | ||
|
|
||
| readBinary = function(filename) { return read(filename, true) }; | ||
|
|
||
| load = function(f) { | ||
| globalEval(read(f)); | ||
| }; | ||
|
|
||
| arguments_ = process['argv'].slice(2); | ||
|
|
||
| } else if (ENVIRONMENT_IS_SHELL) { | ||
| // Polyfill over SpiderMonkey/V8 differences | ||
| if (!this['read']) { | ||
| this['read'] = function(f) { snarf(f) }; | ||
| } | ||
|
|
||
| if (typeof scriptArgs != 'undefined') { | ||
| arguments_ = scriptArgs; | ||
| } else if (typeof arguments != 'undefined') { | ||
| arguments_ = arguments; | ||
| } | ||
|
|
||
| } else if (ENVIRONMENT_IS_WEB) { | ||
| this['print'] = printErr = function(x) { | ||
| console.log(x); | ||
| }; | ||
|
|
||
| this['read'] = function(url) { | ||
| var xhr = new XMLHttpRequest(); | ||
| xhr.open('GET', url, false); | ||
| xhr.send(null); | ||
| return xhr.responseText; | ||
| }; | ||
|
|
||
| if (this['arguments']) { | ||
| arguments_ = arguments; | ||
| } | ||
| } else if (ENVIRONMENT_IS_WORKER) { | ||
| // We can do very little here... | ||
|
|
||
| this['load'] = importScripts; | ||
|
|
||
| } else { | ||
| throw 'Unknown runtime environment. Where are we?'; | ||
| function print(x) { | ||
| process['stdout'].write(x + '\n'); | ||
| } | ||
|
|
||
| function globalEval(x) { | ||
| eval.call(null, x); | ||
| function printErr(x) { | ||
| process['stderr'].write(x + '\n'); | ||
| } | ||
|
|
||
| if (typeof load === 'undefined' && typeof read != 'undefined') { | ||
| this['load'] = function(f) { | ||
| globalEval(read(f)); | ||
| }; | ||
| function read(filename, binary) { | ||
| filename = path['normalize'](filename); | ||
| let ret = fs['readFileSync'](filename); | ||
| if (ret && !binary) ret = ret.toString(); | ||
| return ret; | ||
| } | ||
|
|
||
| if (typeof printErr === 'undefined') { | ||
| this['printErr'] = function(){}; | ||
| function readBinary(filename) { | ||
| return read(filename, true); | ||
| } | ||
|
|
||
| if (typeof print === 'undefined') { | ||
| this['print'] = printErr; | ||
| function globalEval(x) { | ||
| eval.call(null, x); | ||
| } | ||
|
|
||
| assert = function(x, message) { | ||
| if (!x) throw 'assertion failed: ' + message + ' : ' + new Error().stack; | ||
| function load(f) { | ||
| globalEval(read(f)); | ||
| } | ||
|
|
||
| if (!Math['imul'] || Math['imul'](0xffffffff, 5) !== -5) Math['imul'] = function imul(a, b) { | ||
| var ah = a >>> 16; | ||
| var al = a & 0xffff; | ||
| var bh = b >>> 16; | ||
| var bl = b & 0xffff; | ||
| return (al*bl + ((ah*bl + al*bh) << 16))|0; | ||
| assert = function(x, message) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other functions in this file have
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think its because this function needs to be added to the global namespace so that the script that is Its a strange quirk of node and if you just say I have followup PR that will remove the use of eval completely .. but for now this smaller cleanup still relies on eval and the global shared namespace. |
||
| if (!x) throw new Errror(message); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like one too many |
||
| }; | ||
|
|
||
| // Redirect console.log message from MiniLZ4 to stderr since stdout is | ||
| // where we return the decompressed data. | ||
| console.log = printErr; | ||
|
|
||
| // *** Environment setup code *** | ||
|
|
||
| var lz4 = arguments_[0]; | ||
| var input = arguments_[1]; | ||
| var output = arguments_[2]; | ||
| const lz4 = arguments_[0]; | ||
| const input = arguments_[1]; | ||
| const output = arguments_[2]; | ||
|
|
||
| load(lz4); | ||
|
|
||
| var data = readBinary(input); | ||
| if (!(data instanceof ArrayBuffer)) { | ||
| printErr('converting to ArrayBuffer'); | ||
| data = new Uint8Array(data).buffer; | ||
| } | ||
|
|
||
| var start = Date.now(); | ||
| var compressedData = MiniLZ4.compressPackage(data); | ||
| nodeFS['writeFileSync'](output, Buffer.from(compressedData['data'])); | ||
| const data = new Uint8Array(readBinary(input)).buffer; | ||
| const start = Date.now(); | ||
| const compressedData = MiniLZ4.compressPackage(data); | ||
| fs.writeFileSync(output, Buffer.from(compressedData['data'])); | ||
| compressedData['data'] = null; | ||
| printErr('compressed in ' + (Date.now() - start) + ' ms'); | ||
| print(JSON.stringify(compressedData)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not necessary anymore? It seems we use these variables in other js files..?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this script is only ever run under node these days.. and its standalone. This was just copyied and pasted from another script but not needed here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we guarantee this only runs on node? I'm not very familiar with this script's usage, but searching
lz4-compressin the codebase only turns uptools/file_packager.py, which looks it can run elsewhere too... Also do we not run this on d8 as well?Anyway, if that's the case, does it give a readable error message when someone tries to run this on other environments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a completely internal tool just like
src/compiler.jsandtools/preprocessor.js. These tool may once have worked in non-node environments but today we assume they all run under node only. The code here is completely internal to emscripten and there is exactly one call site.All of these internal JS scripts require node these days and there is no reason anyone should try to run them under d8 (at least not that I know of).
If a d8 user did try to run this script and saw
require() is not a functionis should be very obvious that they are trying to run a node script outside of node.