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

chore(common): package version maintenance adjustments #3872

Closed
jahorton opened this issue Nov 17, 2020 · 8 comments
Closed

chore(common): package version maintenance adjustments #3872

jahorton opened this issue Nov 17, 2020 · 8 comments

Comments

@jahorton
Copy link
Contributor

As noted during work on #3836, the kmlmc build needed for Developer's unit tests (during a Windows/Developer build) is triggering a version change within our node-based packages that cause later sub-builds (which rely on the original version value) to fail.

From the TC build logs:

[14:39:20]
[Step 3/6] > @keymanapp/models-templates@14.0.178 build C:\BuildAgent\work\7ac43416c45637e9\keyman\common\models\templates
[14:39:20]
[Step 3/6] > tsc
[14:39:20]
[Step 3/6] 
[14:39:24]
[Step 3/6] lerna success Bootstrapped 13 packages
[14:39:25]
[Step 3/6] v14.0.178-alpha
[14:39:26]
[Step 3/6] 
[14:39:26]
[Step 3/6] > @keymanapp/lexical-model-compiler@14.0.178-alpha build C:\BuildAgent\work\7ac43416c45637e9\keyman\developer\js
[14:39:26]
[Step 3/6] > tsc
[14:39:26]
[Step 3/6] 
[14:39:32]
[Step 3/6] Typescript compilation successful.
[14:39:33]
[Step 3/6] 
[14:39:33]
[Step 3/6] > @keymanapp/lexical-model-compiler@14.0.178-alpha test C:\BuildAgent\work\7ac43416c45637e9\keyman\developer\js
[14:39:33]
[Step 3/6] > mocha

I've traced that section of the logs to this part of the build script:

if (( install_dependencies )) ; then
verify_npm_setup
fi
if [ -n "$publish_version" ]; then
set_npm_version "$publish_version" || fail "Setting version failed."
fi
build || fail "Compilation failed."
echo "Typescript compilation successful."
if (( run_tests )); then
npm test || fail "Tests failed"
fi

So, it appears that Developer's build is setting $publish_version, so when running the unit-testing Makefile it's incidentally altering the package.json version entry that causes the break during later sub-builds.

Originally posted by @jahorton in #3836 (comment)

As noted during team discussions, we should (ideally) rectify this detail by having the committed version of our packages always using tagged versions, instead of having the tier / environment tags only applied during builds. Applying the tier automatically shouldn't pose any issue, though there likely would be a bit of work required to ensure local development builds aren't adversely affected or dual-tagged (like -alpha-local)... unless, of course, we're fine with potential dual-tagging.

@mcdurdin
Copy link
Member

Disabling step 9 "Publish Developer NPM modules" of Keyman Desktop/Developer CI build (-publish-to-npm) until we get this resolved.

@mcdurdin
Copy link
Member

@jahorton wrote:

I'm just guessing here, but since we're bothering to rebuild @keymanapp/developers /lexical-model-compiler from scratch here, we could probably just git restore * at the repo root at the start of the step.
I'm pretty sure the build script is configured to automatically update the version number during that specific build when run within our CI processes. If this assumption holds, that should be 'enough'.

@mcdurdin
Copy link
Member

I've added a temporary patch to step 9 of the CI build script but we should presumably revisit this to clean that up.

@mcdurdin mcdurdin modified the milestones: P10S20, B14S8 Nov 27, 2020
@jahorton
Copy link
Contributor Author

jahorton commented Mar 3, 2021

I advise cross-referencing with #4291, which made some changes that should simplify whatever the needed approach is. At the very least, the original underlying issue (different versions among the npm packages during Developer builds) is no more.

@mcdurdin
Copy link
Member

mcdurdin commented Mar 9, 2021

Yeah, at this point I've got no idea what we need to do here. Things seem to be working smoothly in general, so I'm hesitant to barge in and make changes!

@jahorton
Copy link
Contributor Author

jahorton commented Mar 9, 2021

Yeah, at this point I've got no idea what we need to do here. Things seem to be working smoothly in general, so I'm hesitant to barge in and make changes!

Disabling step 9 "Publish Developer NPM modules" of Keyman Desktop/Developer CI build (-publish-to-npm) until we get this resolved.

I've added a temporary patch to step 9 of the CI build script but we should presumably revisit this to clean that up.

I'd want to revisit these points and run a test build - can we re-enable and/or restore the original version of the corresponding step in our build scripts without issue?

@mcdurdin mcdurdin modified the milestones: B14S8, A15S3 Mar 16, 2021
@mcdurdin mcdurdin modified the milestones: A15S3, A15S5 Mar 25, 2021
@mcdurdin mcdurdin modified the milestones: A15S5, A15S6 May 31, 2021
@mcdurdin mcdurdin modified the milestones: A15S6, A15S7 Jun 14, 2021
@mcdurdin mcdurdin modified the milestones: A15S7, A15S8 Jun 28, 2021
@mcdurdin mcdurdin modified the milestones: A15S8, A15S9 Jul 12, 2021
@mcdurdin mcdurdin modified the milestones: A15S9, Future Jul 22, 2021
@mcdurdin
Copy link
Member

See also #6074 for other possible interactions

@darcywong00 darcywong00 modified the milestones: Future, B15S3 Feb 24, 2022
@darcywong00
Copy link
Contributor

Package versions should be fixed by now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants