search

broccolijs / broccoli-funnel

MIT License
66 stars 37 forks source link

Ensure `lookupDestinationPath` will handle relative path #123

Closed SparshithNR closed 4 years ago

SparshithNR commented 4 years ago
  1. Let lookupDestinationPath handle the destDir and getDestinationPath
  2. added test case Addressing #122
rwjblue commented 4 years ago

Changes seem good, can you look into failing windows CI? 

1) broccoli-funnel
       without filtering options
         can properly handle the output path being a broken symlink:
     Error: ENOENT: no such file or directory, scandir 'C:/Users/appveyor/AppData/Local/Temp/1/broccoli-1992c5HOEsyNqA9w/out-1-funnel/'
      at Object.readdirSync (fs.js:790:3)
      at _walkSync (node_modules\broccoli-test-helper\node_modules\walk-sync\index.js:74:18)
      at Function.entries (node_modules\broccoli-test-helper\node_modules\walk-sync\index.js:50:10)
      at Object.readEntries (node_modules\broccoli-test-helper\dist\util.js:96:48)
      at TreeDiff.diff (node_modules\broccoli-test-helper\dist\tree_diff.js:11:32)
      at builder.build.then (node_modules\broccoli-test-helper\dist\output.js:16:27)
SparshithNR commented 4 years ago

@rwjblue It is because of nodejs bug. https://github.com/nodejs/node/issues/30538 https://github.com/broccolijs/broccoli-funnel/issues/119

rwjblue commented 4 years ago

I see. We should still have a passing CI in this repo before moving forward. Why don't we work around the issue?

rwjblue commented 4 years ago

@SparshithNR - I'd really like to get this fixed, can you take a look at making it at least pass?

SparshithNR commented 4 years ago

Sorry, @rwjblue for not looking into it. I will make sure I will put some time to get some workaround if possible and update my progress EOD.

SparshithNR commented 4 years ago

@rwjblue Fixed the windows test with a workaround. #124

rwjblue commented 4 years ago

Sorry looks like it is still failing

rwjblue commented 4 years ago

Merged the work around #124, can you rebase?

SparshithNR commented 4 years ago

Rebased and updated the PR.