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

Revamp ldd API #2802

Merged
merged 7 commits into from
Nov 7, 2023
Merged

Revamp ldd API #2802

merged 7 commits into from
Nov 7, 2023

Conversation

hugelgupf
Copy link
Member

New:

  • ldd.List(paths ..string) -> lists dependency files (interps and dynamic libraries), as named in the given binary.
  • ldd.FList(paths ...string) -> like List, but also lists all files pointed to through symlinks in the dependency files.

Neither of them return the original file name in paths anymore.

Removing:

  • ldd.List (equivalent is FList, but FList does not list the original file name anymore)
  • ldd.Ldd (there is nothing returning FileInfo anymore, because nothing was using it)

Signed-off-by: Chris Koch <chrisko@google.com>
Signed-off-by: Chris Koch <chrisko@google.com>
Signed-off-by: Chris Koch <chrisko@google.com>
Removes the function that returns FileInfo, remaining is the API that
just returns paths.

Provides separate functions for just listing the dependency files, just
the way they are in the binary, as well as a function for listing the
dependency files and all the files they point to through symlinks.

Signed-off-by: Chris Koch <chrisko@google.com>
Signed-off-by: Chris Koch <chrisko@google.com>
Copy link

codecov bot commented Nov 5, 2023

Codecov Report

Attention: 31 lines in your changes are missing coverage. Please review.

Files Coverage Δ
cmds/exp/pox/pox.go 55.03% <50.00%> (-10.98%) ⬇️
pkg/uroot/uroot.go 64.36% <0.00%> (+0.73%) ⬆️
pkg/ldd/ldd_unix.go 56.00% <34.88%> (-22.65%) ⬇️

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!

@rminnich
Copy link
Member

rminnich commented Nov 5, 2023

it all looks fine but was there some driving reason or is this part of making things better.

Also I did stumble on this today:
r.minnich@2840nrd-rminnic tamago-example % otool -L /bin/date
/bin/date:
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.0.0)

This is on osx. I wonder if we should use it when runtime.GOOS=darwin and our target is darwin?

Copy link
Member

@rminnich rminnich left a comment

Choose a reason for hiding this comment

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

Is the behaviour of the lddfiles command still the same? Printing the file name was useful in scripts.

@hugelgupf
Copy link
Member Author

Is the behaviour of the lddfiles command still the same? Printing the file name was useful in scripts.

I can add that back.

it all looks fine but was there some driving reason or is this part of making things better.

Yes, I couldn't get a list of just the un-followed symlink files out of the current API for warptools/ldshim#2

Signed-off-by: Chris Koch <chrisko@google.com>
@hugelgupf
Copy link
Member Author

PTAL

@rminnich
Copy link
Member

rminnich commented Nov 6, 2023

if you can add it back, that would be nice. A common usage pattern is something like
lddfiles /bin/this /bin/that | cpio -o
and it's critical that /bin/that and /bin/that be included. That should probably be a test ...

@hugelgupf
Copy link
Member Author

hugelgupf commented Nov 6, 2023 via email

@hugelgupf
Copy link
Member Author

Ping?

@rminnich rminnich added automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author. labels Nov 7, 2023
@hugelgupf hugelgupf merged commit a9b6c66 into u-root:main Nov 7, 2023
22 of 24 checks passed
@hugelgupf hugelgupf deleted the lddnew branch November 7, 2023 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Applying this label auto-merges the PR when ready Awaiting author Waiting for new changes or feedback for author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants