Skip to content

Commit

Permalink
Proposed fix for the analyze step
Browse files Browse the repository at this point in the history
This commit avoids the hardcoded use of the ./dist directory in the diff
anaysis step, instead using the directory specified via a new
--working_dir flag. Without this, the ./dist directory is always used
(if it exists, otherwise failure).

The "dist/dist/" repeat has also been removed because it should no
longer be required.

This commit could have used the `distDir` variable instead, but a new
"working_dir" flag has been introduced instead. A generic name has been
chosen to enable its use for things besides diff analysis later if
needed. It's possible that several files in "dist" could be moved there
later if they're not needed in the distribution, but this has not been
attempted here.

I feel a new directory is reasonable because the files associated with
the previous release don't naturally fit with "downloads" (i.e. files
from OSM) or "dist" (the new distribution). In Android in the future, we
may commit dist + download for change history, but committing the
previous release file seems redundant as it also lives in dist/ under a
previous commit.

The flags documentation has been adjusted to try to make the
distinctions between directories clearer.
  • Loading branch information
nfuller committed Jun 15, 2021
1 parent 4e05cb6 commit e0ed8f1
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 22 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,5 @@ $RECYCLE.BIN/
# Project
# =========================
downloads
dist
working
dist
46 changes: 25 additions & 21 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,17 @@ var expectedZoneOverlaps = require('./expectedZoneOverlaps.json')

const argv = yargs
.option('downloads_dir', {
description: 'Set the download location',
description: 'Set the download location for features from OpenStreetMap',
default: './downloads',
type: 'string'
})
.option('working_dir', {
description: 'Set the working files location for temporary / intermediate files',
default: './working',
type: 'string'
})
.option('dist_dir', {
description: 'Set the dist location',
description: 'Set the dist location, for the generated release files',
default: './dist',
type: 'string'
})
Expand Down Expand Up @@ -65,6 +70,7 @@ const argv = yargs
// Resolve the arguments with paths so relative paths become absolute.
const downloadsDir = path.resolve(argv.downloads_dir)
const distDir = path.resolve(argv.dist_dir)
const workingDir = path.resolve(argv.working_dir)

// allow building of only a specified zones
let includedZones = []
Expand Down Expand Up @@ -744,7 +750,7 @@ var downloadLastRelease = function (cb) {
data = JSON.parse(data)
// determine last release version name and download link
const lastReleaseName = data.name
lastReleaseJSONfile = `./dist/${lastReleaseName}.json`
lastReleaseJSONfile = `${workingDir}/${lastReleaseName}.json`
let lastReleaseDownloadUrl
for (var i = 0; i < data.assets.length; i++) {
if (data.assets[i].browser_download_url.indexOf('timezones-with-oceans.geojson') > -1) {
Expand Down Expand Up @@ -774,24 +780,18 @@ var downloadLastRelease = function (cb) {
file.close((err) => {
if (err) return cb(err)
// unzip file
console.log('unzipping latest release')
console.log(`unzipping latest release from ${lastReleaseJSONfile}.zip`)
exec(
`unzip -o ${lastReleaseJSONfile} -d dist`,
`unzip -o ${lastReleaseJSONfile} -d ${workingDir}`,
err => {
if (err) { return cb(err) }
console.log('unzipped file')
console.log('moving unzipped file')
// might need to change this after changes to how files are
// zipped after 2020a
fs.copyFile(
path.join(
'dist',
'dist',
'combined-with-oceans.json'
),
lastReleaseJSONfile,
cb
)

const srcFile = path.join(workingDir, 'combined-with-oceans.json')
console.log(`unzipped file: ${srcFile}`)

const destFile = lastReleaseJSONfile
console.log(`Renaming ${srcFile} to ${destFile}`)
fs.rename(srcFile, destFile, cb)
}
)
})
Expand Down Expand Up @@ -892,11 +892,15 @@ var analyzeChangesFromLastRelease = function (cb) {

const autoScript = {
makeDownloadsDir: function (cb) {
overallProgress.beginTask('Creating downloads dir')
overallProgress.beginTask(`Creating downloads dir (${downloadsDir})`)
safeMkdir(downloadsDir, cb)
},
makeWorkingDir: function (cb) {
overallProgress.beginTask(`Creating working dir (${workingDir})`)
safeMkdir(workingDir, cb)
},
makeDistDir: function (cb) {
overallProgress.beginTask('Creating dist dir')
overallProgress.beginTask(`Creating dist dir (${distDir})`)
safeMkdir(distDir, cb)
},
getOsmBoundaries: ['makeDownloadsDir', function (results, cb) {
Expand Down Expand Up @@ -925,7 +929,7 @@ const autoScript = {
exec('zip -j ' + distDir + '/input-data.zip ' + downloadsDir +
'/* timezones.json osmBoundarySources.json expectedZoneOverlaps.json', cb)
}],
downloadLastRelease: ['makeDistDir', function (results, cb) {
downloadLastRelease: ['makeWorkingDir', function (results, cb) {
if (argv.skip_analyze_diffs) {
overallProgress.beginTask('WARNING: Skipping download of last release for analysis!')
cb()
Expand Down

0 comments on commit e0ed8f1

Please sign in to comment.