-
Notifications
You must be signed in to change notification settings - Fork 442
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
elf: search for host libraries within search paths #2909
Conversation
A regression was introduced with commit 76e8a3f which considered elf files on the host system to be valid. Additionally, since the host-found libraries were absolute paths, an os.path.join suffixing the search paths snapcraft cared about were ignored. A unit test which considers this scenario was added. LP: #1860766 Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
Codecov Report
@@ Coverage Diff @@
## master #2909 +/- ##
=========================================
+ Coverage 87.9% 87.9% +<.01%
=========================================
Files 229 229
Lines 16517 16517
Branches 2548 2549 +1
=========================================
+ Hits 14519 14520 +1
+ Misses 1444 1443 -1
Partials 554 554
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
snapcraft/internal/elf.py
Outdated
if self._is_valid_elf(self.soname_path): | ||
valid_search_paths = [p for p in self.search_paths if os.path.exists(p)] | ||
in_search_paths = any( | ||
[self.soname_path.startswith(p) for p in valid_search_paths] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that it matters here, but I think in this would be a good case for a generator (lose the list brackets):
any(self.soname_path.startswith(p) for p in valid_search_paths)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a fan of generators in general
A regression was introduced with commit 76e8a3f
which considered elf files on the host system to be valid. Additionally, since
the host-found libraries were absolute paths, an os.path.join suffixing the
search paths snapcraft cared about were ignored.
A unit test which considers this scenario was added.
LP: #1860766
Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com
./runtests.sh static
?./runtests.sh tests/unit
?