Fix 2.7.0 Migration #2444
Fix 2.7.0 Migration #2444
Conversation
rust/perspective-viewer/.gitignore
Outdated
| @@ -0,0 +1 @@ | |||
| src/ts/package.json | |||
There was a problem hiding this comment.
Don't add arbitrary files to .gitignore, tasks like twine and npm publish are not aware of these directives which is quite dangerous. If you need to create files as part of the build process, put them in /build so they are cleaned and not published - if they need to be published put them in /dist.
rust/perspective-viewer/build.js
Outdated
| @@ -80,12 +80,13 @@ async function build_all() { | |||
| execSync(`cargo bundle ${IS_DEBUG ? "" : "--release"}`, INHERIT); | |||
|
|
|||
| // JavaScript | |||
| const { default: cpy } = await cpy_mod; | |||
| await cpy("package.json", "src/ts"); | |||
There was a problem hiding this comment.
Don't copy files within the source tree (see above notes on /build and /dist), but also especially don't do this with the package.json, which breaks node's module resolution in this directory (or any package root metadata like Cargo.toml).
| } else if (options.warn) { | ||
| console.warn("Migrating from 2.6.1"); | ||
| } | ||
| old.version = parse_semver("2.7.0"); | ||
| old.version = parse_semver("2.6.1"); |
There was a problem hiding this comment.
This line is inert (because it is overwritten by assure_latest()), but if it weren't it would be wrong. If old == 2.6.1, the migration is applied (conditional on line 22 is false) but then the version field is set back to 2.6.1 again (line 27).
This specific script functionally upgrades from 2.6.1 to 2.7.0 (and presumably beyond), ergo old.version should be some version after 2.6.1 (which we wouldn't know at the time of authoring the migration).
|
@texodus I've updated this PR with some general cleanup of the migration scripts and I've replaced the copied file with an import from node_modules. I think there is still some thinking to be done about how we're going to go about migrations in the future. To-x migration scripts migrate a configuration from version From-x migration scripts migrate a configuration from version We're currently implementing the from-x migrations. The main issue with this approach, as we've discussed, is that there is no way of knowing what the next version will be. I've worked around this by simply placing the latest package version on top. In addition to fixing the latest-package version, the troubles we were having with versioning I think were caused by the mere fact that version migrations did not take the package version into account, which has been fixed in this PR. There are other problems. Because we'll be developing API changes on master, and those migrations will be happening on the same version of perspective as the previous API version, we're still going to have to deal with a form of idempotency in the migrations. That is, we'll need to make sure that the current migration script can upgrade from any version x-n to version x, and stay at version x without change. (i.e., |
This PR ensures that the version created by the migration script matches the version in the package.json. It does so by inlining the package version and bumping the version as needed.