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

Fixed missing deb packages #1270

Closed
wants to merge 2 commits into from
Closed

Fixed missing deb packages #1270

wants to merge 2 commits into from

Conversation

Shivanshu10
Copy link

Hey,
I tried fixing #1269 issue, missing deb packages and build it for latest ubuntu release.
Please review my commit and provide me with proper guidance cause this is my first contribution to Open Source.
Thanks

changed libffi6 to libffi7
changed ubuntu to latest
@ivg
Copy link
Member

ivg commented Feb 16, 2021

Cool, thanks for starting this work! The only problem with this approach is that we will drop support for the older distributions. We are trying our best to support all LTS distributions of Ubuntu, mainly because they are widely used in cloud services. We can see that libffi6 is provided by trusty, xenial, and bionic, whilst libffi7 is only provided by focal. So we have a three to one bargain that will break more than fix. And, as far as I can tell, there are indeed no backports of libffi7 to older distributions, neither compat builds of libffi6 on the newer distributions.

With that said, we can try to make both worlds happy. We can keep the main release action on bionic (18.04) but add an additional action that will run on focal (latest, aka 20.04) and add extra assets to the main release. Thus, at the end, we will have two sets of binary packages - built on bionic, they will work on trusty, xenial, bionic, and the assets built on focal. The latter will be distinguished from the former by having an extra build identifier +libffi7, e.g., bap_2.3.0+libffi7.deb.

Now to the technical stuff, here is the plan. We will keep the existing release.yml action intact and let it do its work on Saturdays. We will add a new release-focal.yml action that will run on Sundays and use focal. Just copy the release.yml and make the corresponding changes. Next, we need some changes in release.sh, first we need an extra (optional) parameter, so we add an extra line after the BAP_VERSION line,

FFI_VERSION=${2:-6}

that means take the second parameter to the release.sh as the value of FFI_VERSION and if it is not set than set it to 6.

Now we need to use it in the DEPENDS section of the DEBIAN/control file, e.g.,

libffi$FFI_VERSION,

After this changes are implemented we can call release.sh in our new release-focal.yml. First of all, we need to set the version to ${{ env.VERSION }}+libffi7, to prevent our prebuilt deb packages from being overridden. Next, we need to remove the Create new prerelease action and instead use the "upload release asset" action from here. There are a few caveats with this action as it is pretty much low-level. The first caveat is that we need to get the upload url. The next caveat is that we need to upload each file separately as this action doesn't support wildcards,

    - name: Get upload URL
      id: geturl
      run:   |
         upload_url=$(curl -sL https://api.github.com/repos/${{github.repository}}/releases/latest?access_token=${{ secrets.GITHUB_TOKEN }} | jq -r '.upload_url')
         echo ::set-output name=upload_url::$upload_url
    - name: Upload Release Asset XXX
      uses: actions/upload-release-asset@v1
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
      with:
        upload_url: ${{ steps.geturl.outputs.upload_url }}
        asset_path: bap/XXXX_${{ env.VERSION }}+libffi7.deb
        asset_name: XXXX_${{ env.VERSION }}+libffi7.deb
        asset_content_type: application/vnd.debian.binary-package

and we need to repeat the Upload Release Asset XXX for each asset XXX that we want to upload, i.e., three times for each deb package, bap, libbap, and libbap-dev.

@Shivanshu10
Copy link
Author

I will try my best to fix this thing, thanks alot for guiding me

@Shivanshu10
Copy link
Author

Hey,
I shouldn't use this line

asset_name: XXXX_${{ env.VERSION }}+libffi7.deb

cause inside release.sh,

BAP_VERSION=$1
FFI_VERSION=${2:-6}

and this line

for pkg in bap libbap libbap-dev; do
    deb=$pkg\_$BAP_VERSION

packages name is generated by BAP_VERSION not using FFI_VERSION so when looking for asset_name,

bap_${{ env.VERSION }}+libffi7.deb

search may fail cause.
Please correct me if I am wrong

@ivg
Copy link
Member

ivg commented Feb 16, 2021

Hey,
I shouldn't use this line

asset_name: XXXX_${{ env.VERSION }}+libffi7.deb

cause inside release.sh,

BAP_VERSION=$1
FFI_VERSION=${2:-6}

and this line

for pkg in bap libbap libbap-dev; do
    deb=$pkg\_$BAP_VERSION

packages name is generated by BAP_VERSION not using FFI_VERSION so when looking for asset_name,

bap_${{ env.VERSION }}+libffi7.deb

search may fail cause.
Please correct me if I am wrong

Sure, the original plan was to pass the new version name to the release.sh script, e.g.,

 run: ./tools/release.sh ${{ env.VERSION }}+libffi7 7

a little bit ugly but this should generate the correct artifacts. An alternative approach, would be to keep the version as it is, but to use asset_name: XXXX_${{ env.VERSION }}+libffi7.deb (which is, as far as I understand, is the name of the in the upload folder) but use the original name in the souce path, e.g.,

asset_path: bap/XXXX_${{ env.VERSION }}.deb

I think the latter solution (with different source and destination names, specified via asset_path and asset_name, correspondingly) looks a bit nicer. But the final decision is up to you.

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.

2 participants