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.

Sign up for GitHub

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

Jump to bottom

Fix detection of gcc-c++ and other package names. #374

Merged
merged 6 commits into from
Dec 1, 2021
Merged

Fix detection of gcc-c++ and other package names. #374

merged 6 commits into from
Dec 1, 2021

Conversation

abadger
Copy link
Member

@abadger abadger commented Nov 25, 2021 •
edited by bocekm
Loading

The regular expression that was being used to parse package names out of
yum output was missing gcc-c++, NetworkManager, and other package names.
Using a simpler regex and a function that parses the package names from
the rest of the NVR should fix that.

This fixes a user visible problem where packages like gcc-c++ can cause
convert2rhel to fail to downgrade packages. This is because gcc-c++
would be missed by the regex and thus we'd be unable to figure out that
we need to downgrade gcc-c++ when at the same time as we downgrade gcc.

Resolves OAMG-6136.

@abadger
Fix detection of gcc-c++ and other package names.
573ee89 
The regular expression that was being used to parse package names out of
yum output was missing gcc-c++, NetworkManager, and other package names.
Using a simpler regex and a function that parses the package names from
the rest of the NVR should fix that.

This fixes a user visible problem where packages like gcc-c++ can cause
convert2rhel to fail to downgrade packages.  This is because gcc-c++
would be missed by the regex and thus we'd be unable to figure out that
we need to downgrade gcc-c++ when downgrading gcc.
@abadger abadger changed the title Fix detection of gcc-c++ and other package names. [WIP] Fix detection of gcc-c++ and other package names. Nov 25, 2021
@abadger abadger force-pushed the fix-c++-package-detection branch from 521333a to 7881948 Compare November 25, 2021 16:15
@abadger abadger changed the title [WIP] Fix detection of gcc-c++ and other package names. Fix detection of gcc-c++ and other package names. Nov 25, 2021
@abadger
Copy link
Member Author

abadger commented Nov 25, 2021

Updated to use @SpyTec 's regex and added some simple unittests for the new function. Probably should add to the unittests for get_problematic_pkgs() as well so that package names like gcc-c++ and NetworkManager are tested there.

@bocekm
Copy link
Member

bocekm commented Nov 26, 2021

The unit test can test with:

gcc-c++
NetworkManager
ImageMagick-c++
devtoolset-1.1-libstdc++-devel
389-ds-base

@abadger
Work on unittests for get_problematic_pkgs and find_pkg_names
691633d 
* test_find_pkg_names: Add a few more package names that represent
  cornercases ("." in package name, starts with a digit, has version
  numbers in the name, has uppercase characters, has an epoch.

* test_get_problematic_pkgs: Move this out of the unittest.TestCase
  so that we can use pytest's parametrize functionality with it.

  * Rework the test to use parametrize.  Parametrize will run a separate
    test for each set of assertions, that way we will (1) be able to
    pinpoint which data caused the problem and (2) will test all of the
    permutations of calling get_problematic_pkgs instead of failing on
    the first problem and not knowing whether the other datasets passed.

  * Found that the Requires dataset should be causing a test failure.
    Break that into a separate test function that tests a subset of
    functionality for now with comments on what is going wrong.
    Opened Issue oamg#378 to address that in the future.
@abadger
Copy link
Member Author

abadger commented Nov 29, 2021

Added those packages to the unittest for find_pkg_name(). Restructured the unittest for get_problematic_pkgs() and will construct some output that uses those pkg names to add to that test.

@abadger
Copy link
Member Author

abadger commented Nov 29, 2021

/packit build

bocekm and others added 2 commits November 30, 2021 15:29
@bocekm
Cover by a integration test
732dc56
@abadger
Unittest cases for get_problematic_pkgs()
d24e424 
* Add pkg names with dots, dashes, starting with numbers, plus signs and
  uppercase to our tests of get_problematic_pkgs().  All of these
  filenames now are properly parsed for Error tests (which is what this
  PR focuses on fixing) but do not work when specified in a Requires.
  That should be addressed when
  oamg#378 is fixed.
* Added the java-1.8.0-openjdk package to the unusual package names to
  check (version number with dots embedded inside of the package name)
@abadger
Copy link
Member Author

abadger commented Nov 30, 2021

Okay, unless CI finds some new problems, I'm done with this PR now. The latest commit just adds to the unittests. One interesting note is that all of the package names @bocekm mentioned here: #374 (comment) now succeed when parsed from the Error: Package: string but they all fail when parsed as part of Requires:. (Fixing that will be tracked by #378)

bocekm
bocekm approved these changes Nov 30, 2021
View reviewed changes
Copy link
Member

@bocekm bocekm left a comment

Choose a reason for hiding this comment

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

Tested manually on OL7 with gcc-c++ installed:
Found packages causing dependency errors: set([u'nss-softokn', u'gcc', u'gcc-c++', u'libstdc++-devel'])

convert2rhel/unit_tests/pkghandler_test.py Show resolved Hide resolved
@bocekm
Copy link
Member

bocekm commented Dec 1, 2021

@kokesak verified the change as well.

@bocekm bocekm merged commit 7ee307f into oamg:main Dec 1, 2021
@abadger abadger deleted the fix-c++-package-detection branch December 2, 2021 19:27
@Venefilyn Venefilyn mentioned this pull request Dec 13, 2021
Merged
bocekm added a commit to mportman12/convert2rhel that referenced this pull request Feb 22, 2022
@abadger @bocekm
Fix detection of gcc-c++ and other package names. (oamg#374)
502c679 
Fix detection of gcc-c++ and other package names.

The regular expression that was being used to parse package names out of
yum output was missing gcc-c++, NetworkManager, and other package names.
Using a simpler regex and a function that parses the package names from
the rest of the NVR should fix that.

This fixes a user visible problem where packages like gcc-c++ can cause
convert2rhel to fail to downgrade packages.  This is because gcc-c++
would be missed by the regex and thus we'd be unable to figure out that
we need to downgrade gcc-c++ when downgrading gcc.

* test_find_pkg_names: Add a few more package names that represent
  cornercases ("." in package name, starts with a digit, has version
  numbers in the name, has uppercase characters, has an epoch).

* Found that the Requires dataset should be causing a test failure.
  Break that into a separate test function that tests a subset of
  functionality for now with comments on what is going wrong.
  Opened Issue oamg#378 to address that in the future.

* Add pkg names with dots, dashes, starting with numbers, plus signs and
  uppercase to our tests of get_problematic_pkgs().  All of these
  filenames now are properly parsed for Error tests (which is what this
  PR focuses on fixing) but do not work when specified in a Requires.
  That should be addressed when
  oamg#378 is fixed.

* Added the java-1.8.0-openjdk package to the unusual package names to
  check (version number with dots embedded inside of the package name)

Co-authored-by: Eric Gustavsson <[email protected]>
Co-authored-by: Michal Bocek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bocekm bocekm bocekm approved these changes

@Venefilyn Venefilyn Venefilyn left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@abadger @bocekm @Venefilyn