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

fix: analytics storage_path in $HOME/.ipfs-desktop #1900

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

milahu
Copy link
Contributor

@milahu milahu commented Sep 4, 2021

make it work with a read-only node_modules

@ipfs ipfs deleted a comment from welcome bot Sep 6, 2021
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, this will remove noisy error (example below) printed to the cli when using .tar.xz package (and most likely others):

$ ./ipfs-desktop
2021-09-07T19:24:25.963Z info: [meta] logs can be found on /home/lidel/.config/IPFS Desktop
Error: ENOTDIR: not a directory, mkdir '/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/node_modules/countly-sdk-nodejs/data'
    at Object.mkdirSync (fs.js:987:3)
    at Object.Countly.init (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/node_modules/countly-sdk-nodejs/lib/countly.js:157:24)
    at module.exports (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/src/analytics.js:6:11)
    at run (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/src/index.js:68:11)
Error: ENOENT, node_modules/countly-sdk-nodejs/data/__cly_id.json not found in /home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar
    at createError (electron/js2c/asar_bundle.js:5:1289)
    at Object.t.<computed> [as open] (electron/js2c/asar_bundle.js:5:2560)
    at Object.writeFile (fs.js:1446:6)
    at writeFile (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/node_modules/countly-sdk-nodejs/lib/countly.js:1540:12)
    at storeSet (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/node_modules/countly-sdk-nodejs/lib/countly.js:1570:13)
    at Object.Countly.init (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/node_modules/countly-sdk-nodejs/lib/countly.js:173:21)
    at module.exports (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/src/analytics.js:6:11)
    at run (/home/lidel/tmp/ipfs-desktop-0.16.3-rc1/ipfs-desktop-0.16.3-linux-x64/resources/app.asar/src/index.js:68:11) {
  code: 'ENOENT',
  errno: -2
}

Before we merge this, small asks inline.

src/analytics.js Outdated Show resolved Hide resolved
src/analytics.js Outdated Show resolved Hide resolved
@lidel lidel marked this pull request as draft October 1, 2021 21:05
@lidel lidel added need/author-input Needs input from the original author and removed need/author-input Needs input from the original author labels Oct 1, 2021
@lidel lidel self-assigned this Oct 1, 2021
userData The directory for storing your app's configuration files, which
by default it is the appData directory appended with your app's name.
https://github.com/electron/electron/blob/main/docs/api/app.md#appgetpathname
@lidel lidel force-pushed the fix-analytics-storage-path branch from ee5fa81 to c6dd22b Compare October 1, 2021 21:27
@lidel lidel marked this pull request as ready for review October 1, 2021 21:27
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased and switched to userData

This means analytics data will land in dirs as described in https://github.com/electron/electron/blob/main/docs/api/app.md#appgetpathname

@lidel lidel merged commit cb611d2 into ipfs:main Oct 1, 2021
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

Successfully merging this pull request may close these issues.

2 participants