Skip to content

Conversation

@fedacking
Copy link
Contributor

@fedacking fedacking commented Oct 27, 2025

Motivation

The MacOS default path includes a space in the path for "Application Support". If a user in MacOS runs the tooling links will fail due to the space in the path, and the '' will cause issues.

Description

  • Changes data path to be only Library, a folder that is used for theses purposes.

@fedacking fedacking requested a review from a team as a code owner October 27, 2025 21:50
@github-actions github-actions bot added the L1 Ethereum client label Oct 27, 2025
@fedacking fedacking changed the title tooling(l1): fixed data path for mac chore(l1): fixed data path for mac Oct 27, 2025
OS = $(shell uname)
ifeq ($(OS), Darwin)
DATA_PATH = $(HOME)/Library/Application\ Support
DATA_PATH = $(HOME)/Library/Application Support
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we need to add quotes around this too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this gets added to the variable in it's entirety

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason you chose this specific path? I know it's the default, but maybe for tooling we can just choose a simpler path so we don't have to do this weird logic with having different paths depending on the OS

Copy link
Contributor Author

@fedacking fedacking Oct 31, 2025

Choose a reason for hiding this comment

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

Changed the DATA_PATH only Library to simplify the tooling makefile in 6e417cc

@fedacking fedacking requested a review from MegaRedHand October 30, 2025 20:50
@github-project-automation github-project-automation bot moved this to In Review in ethrex_l1 Nov 5, 2025
@fedacking fedacking enabled auto-merge November 12, 2025 19:57
@fedacking fedacking added this pull request to the merge queue Nov 12, 2025
Merged via the queue into main with commit 906d744 Nov 12, 2025
38 checks passed
@fedacking fedacking deleted the fix-tooling-makefile branch November 12, 2025 20:39
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants