Skip to content

DLPX-73653 Unpin unzip package following fix for DLPX-73555#225

Merged
smeda-delphix merged 1 commit intomasterfrom
DLPX-73653
Feb 4, 2022
Merged

DLPX-73653 Unpin unzip package following fix for DLPX-73555#225
smeda-delphix merged 1 commit intomasterfrom
DLPX-73653

Conversation

@smeda-delphix
Copy link
Copy Markdown
Contributor

@smeda-delphix smeda-delphix commented Feb 3, 2022

removed the pinned unzip package as the latest one fixes DLPX-73555.

Manual verification:
Installed latest version of unzip 6.0-25ubuntu1 and verified by executing -
unzip -t /export/home/delphix/dlpx-app-gate/appliance/bin/jars/com.delphix.cli/cli/1.0.0/cli-1.0.0.zip
(previously failing command that led to pinning down the version)

ab-pre-push - http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/721/

Comment thread packages/misc-debs/config.sh Outdated
Copy link
Copy Markdown
Contributor

@prakashsurya prakashsurya left a comment

Choose a reason for hiding this comment

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

I think you also need to update packages/virtualization/config.sh .. i.e.

 20 PACKAGE_DEPENDENCIES="adoptopenjdk crypt-blowfish host-jdks misc-debs"          

and

 30 >       >       "$DEPDIR"/misc-debs/unzip_6.0-21ubuntu1_amd64.deb               

and perhaps remove the --allow-downgrades option in lib/common.sh:

 476 >       >       #                                                               
 477 >       >       # We use the "--allow-downgrades" for the case of a             
 478 >       >       # package needing to install the "unzip" debian package         
 479 >       >       # that we build via the "misc-debs" linux-pkg package.          
 480 >       >       # The "misc-debs" based "unzip" package may have an             
 481 >       >       # older version than what's already installed on the            
 482 >       >       # system, so we need to "--allow-downgrades" in order to        
 483 >       >       # install that specific package; further, that specific         
 484 >       >       # package is required to build the "virtualization"             
 485 >       >       # debian packages.                                              
 486 >       >       #                                                               
 487 >       >       sudo env DEBIAN_FRONTEND=noninteractive apt-get install \       
 488 >       >       >       -y --allow-downgrades "$@" && return  

(we could probably leave it too.. but the comment should at least be removed/altered)

@prakashsurya
Copy link
Copy Markdown
Contributor

This also brings up another potential issue.. we have the appliance/packaging/build-dependencies file in the app-gate.. that's supposed to list the packages needed to build the virtualization package.. i.e. we use it like this:

 23 >       logmust read_list "$WORKDIR/repo/appliance/packaging/build-dependencies"
 24 >       logmust install_pkgs "${_RET_LIST[@]}"  

but, I don't see the unzip package listed in that file.. so, I'm wondering if we remove unzip from being installed via:

 30 >       >       "$DEPDIR"/misc-debs/unzip_6.0-21ubuntu1_amd64.deb               

if, then.. we won't install it at all, during the build of the virtualization package.. leading to that package build to fail.. e.g. due to unzip not getting installed at all, during the build..

@prakashsurya
Copy link
Copy Markdown
Contributor

so, I think we need to at least build the virtualization package, with this linux-pkg change.. e.g. run ab-pre-push with the appropriate options, to ensure it uses this change as it's linux-pkg clone.. and then, potentially, we may have a flag day, where we need to add unzip to the app-gate's build-dependencies file; not sure if this will be needed or not, the ab-pre-push run should let us know..

@prakashsurya
Copy link
Copy Markdown
Contributor

you should be able to test this via this Jenkins job: http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/build?delay=0sec

set LINUX_PKG_GIT_URL and LINUX_PKG_GIT_BRANCH to your git repository information.. e.g. https://github.com/smeda-delphix/linux-pkg.git and DLPX-73653.. and set ADDITIONAL_PACKAGES_TO_BUILD to virtualization

@smeda-delphix
Copy link
Copy Markdown
Contributor Author

Started - http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/722/

@prakashsurya
Copy link
Copy Markdown
Contributor

ok.. so in that run I see:

15:31:34  Running: sudo env DEBIAN_FRONTEND=noninteractive  apt-get install -y --allow-downgrades debhelper devscripts equivs shellcheck jq
...
15:31:34  The following additional packages will be installed:
...
15:31:34    python3-unidiff python3-xdg t1utils unzip wdiff

so it does look like we'll get unzip installed via some transitive dependency.. so no flag day.. but it'll be good to add unzip to the build-dependencies file as a follow up to this change..

Copy link
Copy Markdown
Contributor

@prakashsurya prakashsurya left a comment

Choose a reason for hiding this comment

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

LGTM

@smeda-delphix
Copy link
Copy Markdown
Contributor Author

yes, we get unzip by default. We had issue with the default version and had to pin a specific version. So, removing this should allow it to go back to how it was before. I removed the --allow-downgrade option as well now. Started a new run. Sorry, I missed that comment before.
http://selfservice.jenkins.delphix.com/job/appliance-build-orchestrator-pre-push/724/

@smeda-delphix
Copy link
Copy Markdown
Contributor Author

Going ahead with the merge. The only dx-test failure seen in the run was an unrelated issue mentioned in #engineering channel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants