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

handle realpath in makerootfs.sh sanely #3274

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

deitch
Copy link
Contributor

@deitch deitch commented Jun 9, 2023

We use realpath in makerootfs.sh to find the absolute path to various elements. Unfortunately, that has two shortcomings:

  1. realpath is not available on all systems, mainly some Mac
  2. even where realpath is available, we run it against a filename that may not yet exist. Some systems will resolve it, but others will error out.

This updates makerootfs.sh to handle those cases sanely. Instead of realpath the target file, we realpath the parent directory.

If realpath does not exist, we fall back to cd $parent && pwd -P, which gives the same result.

@deitch deitch requested review from eriknordmark and rvs as code owners June 9, 2023 06:05
@deitch deitch force-pushed the realpath-handling branch 2 times, most recently from bb126d6 to f7660df Compare June 9, 2023 06:25
@andrewd-zededa
Copy link
Contributor

I removed my symlink workaround and tried the build, works great. Thanks

@eriknordmark
Copy link
Contributor

If realpath does not exist, we fall back to cd $parent && pwd -P, which gives the same result.

Why can't we always do this if it gives the same result?

tools/makerootfs.sh Outdated Show resolved Hide resolved
@deitch deitch force-pushed the realpath-handling branch from f7660df to 6def244 Compare June 9, 2023 15:42
@deitch
Copy link
Contributor Author

deitch commented Jun 9, 2023

Why can't we always do this if it gives the same result?

No idea. I guess we could. I will try it and update.

Signed-off-by: Avi Deitcher <avi@deitcher.net>
@deitch deitch force-pushed the realpath-handling branch from 6def244 to 83407c4 Compare June 9, 2023 15:44
@deitch
Copy link
Contributor Author

deitch commented Jun 9, 2023

OK, updated

@andrewd-zededa
Copy link
Contributor

Latest changes are still a working build on my machine.

Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@zedi-pramodh
Copy link

If realpath is removed then we do not need the commit. You might as well revert that commit.

commit bbce625
Author: Pramodh Pallapothu pramodh@zededa.com
Date: Tue Jun 6 12:40:58 2023 -0700

Add coreutils package as prereq for MAC

Signed-off-by: Pramodh Pallapothu <pramodh@zededa.com>

@deitch
Copy link
Contributor Author

deitch commented Jun 10, 2023

That commit isn't in here.

@zedi-pramodh
Copy link

I know I did that. But since you are removing realpath you might as remove the prerequisites of coreutils for the MAC.

@deitch
Copy link
Contributor Author

deitch commented Jun 11, 2023

No problem @zedi-pramodh ; done.

This reverts commit bbce625.

Signed-off-by: Avi Deitcher <avi@deitcher.net>
@deitch deitch force-pushed the realpath-handling branch from c9b573a to 524f452 Compare June 11, 2023 08:26
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

Run eden

@eriknordmark eriknordmark merged commit 13fba79 into lf-edge:master Jun 12, 2023
@deitch deitch deleted the realpath-handling branch June 13, 2023 07:30
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.

4 participants