stevedlawrence commented on code in PR #232:
URL: https://github.com/apache/daffodil-vscode/pull/232#discussion_r923873964
##########
build/scripts/process_build.ts:
##########
@@ -18,39 +18,88 @@
// @ts-nocheck <-- This is needed as this file is basically a JavaScript script
// but with some TypeScript niceness baked in
const fs = require('fs')
+const path = require('path')
const execSync = require('child_process').execSync
-function prebuild() {
- fs.renameSync('LICENSE', 'tmp.LICENSE')
- fs.renameSync('NOTICE', 'tmp.NOTICE')
- fs.copyFileSync('build/bin.NOTICE', 'NOTICE')
- fs.copyFileSync('build/bin.LICENSE', 'LICENSE')
+const pkg_dir = 'dist/package'
+
+function getDir(file) {
+ return file
+ .split('/')
+ .slice(0, file.split('/').length - 1)
+ .join('/')
}
-function postbuild() {
- fs.rmSync('LICENSE')
- fs.rmSync('NOTICE')
- fs.renameSync('tmp.LICENSE', 'LICENSE')
- fs.renameSync('tmp.NOTICE', 'NOTICE')
-
- // This will make sure that if the root LICENSE and NOTICE are the same as
the build LICENSE
- // and NOTICE that they are reverted back to their original contents.
- if (
- fs.readFileSync('build/bin.LICENSE').toString() ===
- fs.readFileSync('LICENSE').toString()
- ) {
- execSync('git checkout LICENSE')
+function fileCopy(srcPath, destPath) {
+ if (fs.statSync(srcPath).isFile()) {
+ fs.copyFileSync(srcPath, destPath)
}
+}
+
+function dirCopy(file) {
+ fs.readdirSync(file).forEach((dirFile) => {
+ const filePath = path.join(file, dirFile)
+ fileCopy(filePath, path.join(pkg_dir, file, dirFile))
+ })
+}
+
+function build_package() {
+ if (fs.existsSync(pkg_dir)) {
+ fs.rmdirSync(pkg_dir, { recursive: true })
+ }
+
+ fs.mkdirSync(pkg_dir)
+
+ let fileList = fs.readFileSync('.vscodeignore').toString().split('\n')
Review Comment:
This is minor, but what are your thoughts on moving the .vscodeignore file
to another directory since it is really only used during packaging? I'm
thinking we could put .vscodeignore, bin.LICENSE, and bin.NOTICE files in
something like `build/package/`. After we've copied all the files from
.vscodeignore into `dist/package`, we could then just copy `build/package/*`
into `dist/package/`. That way adding new static files to `dist/package` is as
easy as putting them in `build/package`.
##########
build/scripts/process_build.ts:
##########
@@ -18,39 +18,81 @@
// @ts-nocheck <-- This is needed as this file is basically a JavaScript script
// but with some TypeScript niceness baked in
const fs = require('fs')
+const path = require('path')
const execSync = require('child_process').execSync
-function prebuild() {
- fs.renameSync('LICENSE', 'tmp.LICENSE')
- fs.renameSync('NOTICE', 'tmp.NOTICE')
- fs.copyFileSync('build/bin.NOTICE', 'NOTICE')
- fs.copyFileSync('build/bin.LICENSE', 'LICENSE')
+const pkg_dir = 'dist/package'
+
+function getDir(file) {
+ return file
+ .split('/')
+ .slice(0, file.split('/').length - 1)
+ .join('/')
}
-function postbuild() {
- fs.rmSync('LICENSE')
- fs.rmSync('NOTICE')
- fs.renameSync('tmp.LICENSE', 'LICENSE')
- fs.renameSync('tmp.NOTICE', 'NOTICE')
-
- // This will make sure that if the root LICENSE and NOTICE are the same as
the build LICENSE
- // and NOTICE that they are reverted back to their original contents.
- if (
- fs.readFileSync('build/bin.LICENSE').toString() ===
- fs.readFileSync('LICENSE').toString()
- ) {
- execSync('git checkout LICENSE')
+function fileCopy(srcPath, destPath) {
+ if (fs.statSync(srcPath).isFile()) {
+ fs.copyFileSync(srcPath, destPath)
}
+}
+
+function dirCopy(file) {
+ fs.readdirSync(file).forEach((dirFile) => {
+ const filePath = path.join(file, dirFile)
+ fileCopy(filePath, path.join(pkg_dir, file, dirFile))
+ })
+}
+
+function build_package() {
+ if (fs.existsSync(pkg_dir)) {
+ fs.rmdirSync(pkg_dir, { recursive: true })
+ }
+
+ fs.mkdirSync(pkg_dir)
+
+ let fileList = fs.readFileSync('.vscodeignore').toString().split('\n')
+
+ for (var i = 0; i < fileList.length; i++) {
+ var file = fileList[i].replace(/\*/gi, '').replace('!', '')
- if (
- fs.readFileSync('build/bin.NOTICE').toString() ===
- fs.readFileSync('NOTICE').toString()
- ) {
- execSync('git checkout NOTICE')
+ if (file.includes('#') || ['', '/', '\\', '\\r', '\\n'].includes(file)) {
+ continue
+ }
+
+ if (file.includes('zip')) {
+ var dir = getDir(file)
+
+ fs.readdirSync(dir).forEach((dirFile) => {
+ if (dirFile.includes('zip')) {
+ file = `${dir}/${dirFile}`
+ }
+ })
+ }
+
+ let directory = fs.statSync(file).isFile() ? getDir(file) : file
+
+ fs.mkdirSync(`${pkg_dir}/${directory}`, { recursive: true })
+
+ fs.statSync(file).isFile()
+ ? fileCopy(file, `${pkg_dir}/${file}`)
+ : dirCopy(file)
}
+
+ fileCopy('.vscodeignore', `${pkg_dir}/.vscodeignore`)
Review Comment:
Do we also need to copy bin.NOTICE and bin.LICENSE files?
##########
build/scripts/process_build.ts:
##########
@@ -18,39 +18,88 @@
// @ts-nocheck <-- This is needed as this file is basically a JavaScript script
// but with some TypeScript niceness baked in
const fs = require('fs')
+const path = require('path')
const execSync = require('child_process').execSync
-function prebuild() {
- fs.renameSync('LICENSE', 'tmp.LICENSE')
- fs.renameSync('NOTICE', 'tmp.NOTICE')
- fs.copyFileSync('build/bin.NOTICE', 'NOTICE')
- fs.copyFileSync('build/bin.LICENSE', 'LICENSE')
+const pkg_dir = 'dist/package'
+
+function getDir(file) {
+ return file
+ .split('/')
+ .slice(0, file.split('/').length - 1)
+ .join('/')
}
-function postbuild() {
- fs.rmSync('LICENSE')
- fs.rmSync('NOTICE')
- fs.renameSync('tmp.LICENSE', 'LICENSE')
- fs.renameSync('tmp.NOTICE', 'NOTICE')
-
- // This will make sure that if the root LICENSE and NOTICE are the same as
the build LICENSE
- // and NOTICE that they are reverted back to their original contents.
- if (
- fs.readFileSync('build/bin.LICENSE').toString() ===
- fs.readFileSync('LICENSE').toString()
- ) {
- execSync('git checkout LICENSE')
+function fileCopy(srcPath, destPath) {
+ if (fs.statSync(srcPath).isFile()) {
+ fs.copyFileSync(srcPath, destPath)
}
+}
+
+function dirCopy(file) {
+ fs.readdirSync(file).forEach((dirFile) => {
+ const filePath = path.join(file, dirFile)
+ fileCopy(filePath, path.join(pkg_dir, file, dirFile))
+ })
+}
+
+function build_package() {
+ if (fs.existsSync(pkg_dir)) {
+ fs.rmdirSync(pkg_dir, { recursive: true })
+ }
+
+ fs.mkdirSync(pkg_dir)
+
+ let fileList = fs.readFileSync('.vscodeignore').toString().split('\n')
+
+ for (var i = 0; i < fileList.length; i++) {
+ var file = fileList[i].replace(/\*/gi, '').replace('!', '')
- if (
- fs.readFileSync('build/bin.NOTICE').toString() ===
- fs.readFileSync('NOTICE').toString()
- ) {
- execSync('git checkout NOTICE')
+ if (
+ file.includes('#') ||
+ file == '' ||
+ file == '/' ||
+ file == '\\' ||
+ file == '\\r' ||
+ file === '\\n'
+ ) {
+ continue
+ }
+
+ if (file.includes('zip')) {
+ var dir = getDir(file)
+
+ fs.readdirSync(dir).forEach((dirFile) => {
+ if (dirFile.includes('zip')) {
+ file = `${dir}/${dirFile}`
+ }
+ })
+ }
+
+ let directory = fs.statSync(file).isFile() ? getDir(file) : file
+
+ fs.mkdirSync(`${pkg_dir}/${directory}`, { recursive: true })
+
+ fs.statSync(file).isFile()
+ ? fileCopy(file, `${pkg_dir}/${file}`)
+ : dirCopy(file)
}
+
+ fileCopy('.vscodeignore', `${pkg_dir}/.vscodeignore`)
+ execSync('yarn install', { cwd: pkg_dir })
Review Comment:
Is it possible to remove or replace yarn install some how? I think this is
needed to create the `node_modules` stuff needed to run vsce? It would be
really nice if yarn package/vsce package could just reuse the existing
`node_modules` in the root directory somehow?
If it's not possible, do we need to also copy yarn.lock into `dist/package`?
It looks like this creates a new one that isn't exactly the same as the
existing one if we don't copy?
##########
build/scripts/process_build.ts:
##########
@@ -18,39 +18,81 @@
// @ts-nocheck <-- This is needed as this file is basically a JavaScript script
// but with some TypeScript niceness baked in
const fs = require('fs')
+const path = require('path')
const execSync = require('child_process').execSync
-function prebuild() {
- fs.renameSync('LICENSE', 'tmp.LICENSE')
- fs.renameSync('NOTICE', 'tmp.NOTICE')
- fs.copyFileSync('build/bin.NOTICE', 'NOTICE')
- fs.copyFileSync('build/bin.LICENSE', 'LICENSE')
+const pkg_dir = 'dist/package'
+
+function getDir(file) {
+ return file
+ .split('/')
+ .slice(0, file.split('/').length - 1)
+ .join('/')
}
-function postbuild() {
- fs.rmSync('LICENSE')
- fs.rmSync('NOTICE')
- fs.renameSync('tmp.LICENSE', 'LICENSE')
- fs.renameSync('tmp.NOTICE', 'NOTICE')
-
- // This will make sure that if the root LICENSE and NOTICE are the same as
the build LICENSE
- // and NOTICE that they are reverted back to their original contents.
- if (
- fs.readFileSync('build/bin.LICENSE').toString() ===
- fs.readFileSync('LICENSE').toString()
- ) {
- execSync('git checkout LICENSE')
+function fileCopy(srcPath, destPath) {
+ if (fs.statSync(srcPath).isFile()) {
+ fs.copyFileSync(srcPath, destPath)
}
+}
+
+function dirCopy(file) {
+ fs.readdirSync(file).forEach((dirFile) => {
+ const filePath = path.join(file, dirFile)
+ fileCopy(filePath, path.join(pkg_dir, file, dirFile))
+ })
+}
+
+function build_package() {
+ if (fs.existsSync(pkg_dir)) {
+ fs.rmdirSync(pkg_dir, { recursive: true })
+ }
+
+ fs.mkdirSync(pkg_dir)
+
+ let fileList = fs.readFileSync('.vscodeignore').toString().split('\n')
+
+ for (var i = 0; i < fileList.length; i++) {
+ var file = fileList[i].replace(/\*/gi, '').replace('!', '')
- if (
- fs.readFileSync('build/bin.NOTICE').toString() ===
- fs.readFileSync('NOTICE').toString()
- ) {
- execSync('git checkout NOTICE')
+ if (file.includes('#') || ['', '/', '\\', '\\r', '\\n'].includes(file)) {
+ continue
+ }
+
+ if (file.includes('zip')) {
+ var dir = getDir(file)
+
+ fs.readdirSync(dir).forEach((dirFile) => {
+ if (dirFile.includes('zip')) {
+ file = `${dir}/${dirFile}`
+ }
+ })
+ }
Review Comment:
This code seems a bit fragile. For example, it removes `*`'s which are
useful since some of the lines in .vscodeignore are globs. Also, the way it
handles zip is fragile since it just copies all zips from a directory? I think
because we use globs for zip files in the .vscodeignore file?
What are you thoughts on this approach, where the key differences is 1) we
only look at lines that start with an exclamation, and 2) those lines are
treated as globs using the glob package:
```node
let lines = fs.readFileSync('.vscodeignore').toString().split('\n')
for (var i = 0; i < lines.length; i++) {
let line = lines[i]
// ignore any lines that don't start with an exclamation
if (!line.startsWith("!")) continue
// remove the exclamation
let pattern = line.substring(1)
// find all files matching the line glob and copy into dist/package/
glob(pattern, (error, files) => {
for (var j = 0; j < files.length; j++) {
let file = files[j]
// code here from below that creates directories and copies file/dir
}
})
```
This isn't tested, and I'm sure there's some node issues, but something
along those lines?
##########
build/scripts/process_build.ts:
##########
@@ -18,39 +18,88 @@
// @ts-nocheck <-- This is needed as this file is basically a JavaScript script
// but with some TypeScript niceness baked in
const fs = require('fs')
+const path = require('path')
const execSync = require('child_process').execSync
-function prebuild() {
- fs.renameSync('LICENSE', 'tmp.LICENSE')
- fs.renameSync('NOTICE', 'tmp.NOTICE')
- fs.copyFileSync('build/bin.NOTICE', 'NOTICE')
- fs.copyFileSync('build/bin.LICENSE', 'LICENSE')
+const pkg_dir = 'dist/package'
+
+function getDir(file) {
+ return file
+ .split('/')
+ .slice(0, file.split('/').length - 1)
+ .join('/')
}
-function postbuild() {
- fs.rmSync('LICENSE')
- fs.rmSync('NOTICE')
- fs.renameSync('tmp.LICENSE', 'LICENSE')
- fs.renameSync('tmp.NOTICE', 'NOTICE')
-
- // This will make sure that if the root LICENSE and NOTICE are the same as
the build LICENSE
- // and NOTICE that they are reverted back to their original contents.
- if (
- fs.readFileSync('build/bin.LICENSE').toString() ===
- fs.readFileSync('LICENSE').toString()
- ) {
- execSync('git checkout LICENSE')
+function fileCopy(srcPath, destPath) {
+ if (fs.statSync(srcPath).isFile()) {
+ fs.copyFileSync(srcPath, destPath)
}
+}
+
+function dirCopy(file) {
+ fs.readdirSync(file).forEach((dirFile) => {
+ const filePath = path.join(file, dirFile)
+ fileCopy(filePath, path.join(pkg_dir, file, dirFile))
+ })
+}
+
+function build_package() {
+ if (fs.existsSync(pkg_dir)) {
+ fs.rmdirSync(pkg_dir, { recursive: true })
+ }
+
+ fs.mkdirSync(pkg_dir)
+
+ let fileList = fs.readFileSync('.vscodeignore').toString().split('\n')
+
+ for (var i = 0; i < fileList.length; i++) {
+ var file = fileList[i].replace(/\*/gi, '').replace('!', '')
- if (
- fs.readFileSync('build/bin.NOTICE').toString() ===
- fs.readFileSync('NOTICE').toString()
- ) {
- execSync('git checkout NOTICE')
+ if (
+ file.includes('#') ||
+ file == '' ||
+ file == '/' ||
+ file == '\\' ||
+ file == '\\r' ||
+ file === '\\n'
+ ) {
+ continue
+ }
+
+ if (file.includes('zip')) {
+ var dir = getDir(file)
+
+ fs.readdirSync(dir).forEach((dirFile) => {
+ if (dirFile.includes('zip')) {
+ file = `${dir}/${dirFile}`
+ }
+ })
+ }
+
+ let directory = fs.statSync(file).isFile() ? getDir(file) : file
+
+ fs.mkdirSync(`${pkg_dir}/${directory}`, { recursive: true })
+
+ fs.statSync(file).isFile()
+ ? fileCopy(file, `${pkg_dir}/${file}`)
+ : dirCopy(file)
}
+
+ fileCopy('.vscodeignore', `${pkg_dir}/.vscodeignore`)
+ execSync('yarn install', { cwd: pkg_dir })
+ execSync('yarn package', { cwd: pkg_dir })
Review Comment:
yarn package just calls vsce, thoughts on just directly calling vsce
ourselves here and removing the "yarn package script? Is there ever a case
where we would want to run "yarn package" without all thise new copy stuff?
vsce package also has a `--out` parameter which could be used instead of
having to call get_vsix? Then `yarn build` just becomes this:
```
"build": "node -e
\"require('./build/scripts/process_build.ts').build_package()\"",
```
and it simplifies the package.json a bit?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
