phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
2 stars 5 forks source link

Build failure: `TypeError: Cannot read properties of undefined (reading 'requirejsNamespace')` #331

Closed chrisklus closed 1 year ago

chrisklus commented 1 year ago

This build originated from a translation submission for build-an-atom 1.6.

Build server log:

Jun 22 22:58:32 phet-server2.int.colorado.edu build-server[124816]:

debug: Loading JSON from ../release-branches/build-an-atom-1.6/shred/package.json
error: BUILD ABORTED! TypeError: Cannot read properties of undefined (reading 'requirejsNamespace')
error: TypeError: Cannot read properties of undefined (reading 'requirejsNamespace')
    at module.exports (/data/share/phet/build-server/perennial/js/build-server/getRepoStringMap.js:31:47)
    at async getFullStringMap (/data/share/phet/build-server/perennial/js/build-server/getFullStringMap.js:25:30)
    at async parseScreenNamesFromSimulation (/data/share/phet/build-server/perennial/js/build-server/parseScreenNames.js:18:21)
    at async module.exports (/data/share/phet/build-server/perennial/js/build-server/createTranslationsXML.js:57:23)
    at async runTask (/data/share/phet/build-server/perennial/js/build-server/taskWorker.js:254:13)
error: Build failure: Error: Build aborted, TypeError: Cannot read properties of undefined (reading 'requirejsNamespace'). Sim = build-an-atom Version = 1.6.26 Brands = phet Locales = se Shas = {"comment":"# build-an-atom 1.6.26 Thu Jun 22 2023 01:17:54 GMT-0600 (Mountain Daylight Time)"

build-server/getRepoStringMap.js:31:

const packageJSON = await loadJSON( `${checkoutDir}/${repo}/package.json` );
const requirejsNamespace = packageJSON.phet.requirejsNamespace;

The problem is happening because the build server is trying to get the requirejsNamespace name of shred, but the shred checked out in build-an-atom-1.6 has a package.json with no requirejsNamespace property. The problem is new because until commits from yesterday in https://github.com/phetsims/perennial/issues/330, this requirejsNamespace request from dependencies was using master branch checkouts, and shred's package.json does have that property.

I'm not sure if this problem is unique to shred or if there are other repos that were relying on the master branch of the dependency. We know that the dependencies of phet-io-test-sim 2.12 and area-builder 1.1 do not have the problem.

I'll check in with @jonathanolson and @mattpen tomorrow about the way to proceed. If it's best to use the master branch checkout, then we can just change the checkoutDir for getting the namespace to the new directory of master branch checkouts and clone/pull the dependency repo.

mattpen commented 1 year ago

All other translations last night were successful: gravity-and-orbits, diffusion, proportion-playground, and bending-light. None of these sims depend on shred though. Looks like shred got missed in the batch maintenance release for https://github.com/phetsims/chipper/issues/1367 perhaps??

mattpen commented 1 year ago

Seems like we need to do another batch release, following a process like this:

for each releaseBranch:
  for each repo in releaseBranch/{{simRepo}}/dependencies.json:
    if {{repo}}/*_strings.json exists
      if {{repo}}/package.json does not contain phet.requirejsNamespace
         add a phet object and an namespace to {{repo}}/package.json

We can probably identify these automatically, but I'm not sure how to determine what the value for the requirejsNamespace should be.

jonathanolson commented 1 year ago

Hmm, it's been convention for a long time that we have requirejsNamespace (that last maintenance release didn't add it). Looks like the commit landed in https://github.com/phetsims/shred/commit/22b91284b89bc9d2b0d674b04d3764037d27b199, but potentially that doesn't cover all release branches?

mattpen commented 1 year ago

@jonathanolson - build-an-atom is using phetsims/shred@fc19ce5e9cb58ccd057c8644795ed70d0745a178, which does not have a phet object at all in its package.json.

jonathanolson commented 1 year ago

Ran the following:

const fs = require( 'fs' );

fs.readdirSync( '../release-branches' ).forEach( releaseBranchDir => {
  fs.readdirSync( `../release-branches/${releaseBranchDir}` ).forEach( repo => {
    if ( fs.existsSync( `../release-branches/${releaseBranchDir}/${repo}/${repo}-strings_en.json` ) ) {
      const packageJSON = JSON.parse( fs.readFileSync( `../release-branches/${releaseBranchDir}/${repo}/package.json`, 'utf-8' ) );
      if ( !packageJSON.phet || !packageJSON.phet.requirejsNamespace ) {
        console.log( `${releaseBranchDir}/${repo}` );
      }
    }
  } );
} );

Resulted in:

build-an-atom-1.5-phetio/shred
build-an-atom-1.6/shred
mattpen commented 1 year ago

@jonathanolson deployed a patch for shred for build-an-atom (1.6 and 1.5-phetio). The production deploy rebuilt the failed translations (se and ee) successfully.

@chrisklus and I retriggered translations for se and ee so that the translator credits are properly added.

This issue is fixed, closing.