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

Update to mkdirp ≥ 1 #142

Open
guimard opened this issue Oct 21, 2020 · 4 comments
Open

Update to mkdirp ≥ 1 #142

guimard opened this issue Oct 21, 2020 · 4 comments

Comments

@guimard
Copy link

guimard commented Oct 21, 2020

Hi,

it looks like this quick-and-dirty patch is enough to switch to mkdirp ≥ 1 :

--- a/lib/millstone.js
+++ b/lib/millstone.js
@@ -214,8 +214,14 @@
             }
         }
         var dir_path = path.dirname(options.filepath);
-        mkdirp(dir_path, 0755, function(err) {
-            if (err && err.code !== 'EEXIST') {
+        mkdirp(dir_path, 0755).then(() => {
+                download(url, options, function(err, filepath) {
+                    if (err) return callback(err);
+                    callback(null, filepath);
+                });
+        })
+        .catch((err) => {
+            if ( err.code !== 'EEXIST') {
                 if (env == 'development') console.error('[millstone] could not create directory: ' + dir_path);
                 callback(err);
             } else {
@@ -533,8 +539,9 @@
         benchmark = options.benchmark;

     Step(function setup() {
-        if (nosymlink) mkdirp(base, 0755, this);
-        else mkdirp(path.join(base, 'layers'), 0755, this);
+        var me = this;
+        if (nosymlink) mkdirp(base, 0755).then(() => {me()}).catch((e)=>{me(e)});
+        else mkdirp(path.join(base, 'layers'), 0755).then(() => {me()}).catch((e)=>{me(e)});
     }, function style_externals(err) {
         if (err && err.code !== 'EEXIST') throw err;
         if (benchmark) console.time("[millstone][benchmark] Resolving style (mss) externals");
--- a/package.json
+++ b/package.json
@@ -27,7 +27,7 @@
         "zipfile": "~0.5.5",
         "sqlite3": "4.1.0",
         "mime": "~1.2.11",
-        "mkdirp": "~0.5.0",
+        "mkdirp": "^1.0.3",
         "optimist": "~0.6.1"
     },
     "devDependencies": {
@csytsma
Copy link
Member

csytsma commented Oct 21, 2020

@guimard Can you help define how this would be tested, to confirm the patch doesn't break anything? Or provide your own test results? Then we can easily merge your patch.

@guimard
Copy link
Author

guimard commented Oct 22, 2020

Here is the result:

$ npm run test

> millstone@0.6.18 test
> mocha -R spec --timeout 10000



  ✓ correctly handles re-downloading a zip that is invalid in its cached state (988ms)
  ✓ correctly handles invalid json
  ✓ correctly handles missing shapefile at relative path
  ✓ correctly handles missing shapefile at absolute path
  ✓ correctly handles images with no extension (53ms)
  ✓ correctly handles mac os x zipped archives with the lame __MACOSX/ subfolder (721ms)
  ✓ correctly localizes remote image/svg files (603ms)
  ✓ correctly localizes zipped json (321ms)
  ✓ correctly handles a zipfile containing multiple shapefiles without corrupting data (957ms)
  ✓ correctly handles files without symlinking
  ✓ correctly handles a zipfile containing multiple shapefiles without corrupting data
  ✓ correctly detects content-disposition from kml
  ✓ correctly detects content-disposition from google docs csv
  ✓ correctly detects content-disposition from geoserver
  ✓ correctly detects content-disposition from cartodb
  ✓ correctly detects content-type bin
  ✓ correctly detects datacouch csv content-type
  ✓ correctly detects geonode content-types
  ✓ correctly caches remote files (1386ms)
  ✓ correctly handles datasources with uppercase extensions
  ✓ correctly prefers (for back compatibility) shapefiles over .txt files in zip archive
  isRelative
    ✓ detects C:\ as an absolute path on Windows
    ✓ detects C:\ as relative path on non-Windows
    ✓ detects paths starting with \ as absolute on Windows
    ✓ detects paths starting with \ as relative on non-Windows
    ✓ detects paths starting with / as absolute on non-Windows
    ✓ detects paths starting with / as absolute on Windows

  util
    ✓ copies all files from shapefiles (and no extras)
    ✓ copies single files correctly


  29 passing (5s)

guimard added a commit to guimard/millstone that referenced this issue Oct 22, 2020
@guimard
Copy link
Author

guimard commented Oct 22, 2020

@csytsma : I updated my patch using your master branch into a PR (#143)

@guimard
Copy link
Author

guimard commented Oct 22, 2020

and published this in Debian/experimental

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

No branches or pull requests

2 participants