Skip to content

Switch default socket to /var/run/nfd/nfd.sock on Mac#147

Merged
zjkmxy merged 3 commits intonamed-data:mainfrom
brad-lowe:brad/macos
May 2, 2025
Merged

Switch default socket to /var/run/nfd/nfd.sock on Mac#147
zjkmxy merged 3 commits intonamed-data:mainfrom
brad-lowe:brad/macos

Conversation

@brad-lowe
Copy link
Contributor

#144

Checks operating system before choosing socket file.
If on Mac, use /var/run/nfd/nfd.sock
Otherwise, use /run/nfd/nfd.sock

Fixes an issue where Mac gives a permission denied error when trying to create a file in the /run folder.

@brad-lowe brad-lowe marked this pull request as ready for review May 2, 2025 17:02
Copy link
Member

@zjkmxy zjkmxy left a comment

Choose a reason for hiding this comment

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

LGTM

# Whether to enable Unix stream transports
enabled: true
# Location of the socket file
## If running on Mac, use /var/run/nfd/nfd.sock
Copy link
Member

Choose a reason for hiding this comment

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

nit: Maybe add another line explaining on Windows it is ${TEMP}\\nfd\\nfd.sock

enabled: true
# Location of the socket file
## If running on Mac, use /var/run/nfd/nfd.sock
socket_path: /run/nfd/nfd.sock
Copy link
Member

Choose a reason for hiding this comment

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

maybe this setting should be commented out in the sample so that it falls back to the default which works out of the box on all platforms?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This makes sense, thanks! I'll add a comment like @zjkmxy mentioned above as well in case they want to manually set it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commented out the line, and explained how the path is set on Windows vs Mac

@zjkmxy zjkmxy merged commit ff446e2 into named-data:main May 2, 2025
9 checks passed
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.

3 participants