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

Add build workflow #30644

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Add build workflow #30644

wants to merge 9 commits into from

Conversation

Lesmiscore
Copy link
Contributor

@Lesmiscore Lesmiscore commented Feb 17, 2022

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

(Forgive me for not checking some of them)

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

yt-dlp build workflow and devscripts/update-version.py for latter.

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

The goal for this PR is to give this repository a build workflow, enough to create a release except signatures (aka .sig).
It also reproduces how the youtube-dl.exe was built.

The files are reduced version to create a daily releases, so I need some fixes.

Big thanks for yt-dlp build workflow which this PR is based on. (forgive me for being lazy while ESA is in session) ESA ended.

TODO

Known Notes:

  • Has to set a secret with key PUSH_VERSION_COMMIT. Value can be "anything".
  • To fully reproduce Releases as it were, you need the private key to sign binaries. That's not what I can have. Either dstftw or remitamine absolutely has it.

Closes #31067.

@Lesmiscore

This comment was marked as off-topic.

@Lesmiscore Lesmiscore marked this pull request as ready for review February 20, 2022 09:22
@gamer191
Copy link

gamer191 commented Jun 9, 2022

Has this PR been completely forgotten about?

@Lesmiscore
Copy link
Contributor Author

maybe

ping @dirkf

@dirkf dirkf mentioned this pull request Jun 20, 2022
11 tasks
@dirkf
Copy link
Contributor

dirkf commented Jun 20, 2022

No. Great work by everyone, and thanks, and apologies for taking so long to follow this up.

To do:

  • signing key
  • complete testing in my dev repo
  • add bundled build for Pythonless MacOS and any other Pythonly challenged Unixes, following yt-dlp

@pukkandan
Copy link
Contributor

add bundled build for Pythonless MacOS and any other Pythonly challenged Unixes, following yt-dlp

You'll need to use pyinstaller for this and not py2exe

@Lesmiscore
Copy link
Contributor Author

Lesmiscore commented Jun 20, 2022

This is kinda PoC anyways...

@gamer191
Copy link

gamer191 commented Jun 21, 2022

add bundled build for Pythonless MacOS and any other Pythonly challenged Unixes, following yt-dlp

IMO that's unnecessary, because afaik all those devices will be compatible with yt-dlp (at least the macbooks, I'm not sure about the other unixes).

signing key

and speaking of the signing key, I wonder if @dirkf would be willing to share the signing key with @pukkandan, so that yt-dlp can also be signed (and hence won't be flagged by any antiviruses, although that's not a major issue nowadays)

@Lesmiscore
Copy link
Contributor Author

Lesmiscore commented Jun 21, 2022

the signing key with @pukkandan, so that yt-dlp can also be signed

The signing part implies .sig files like there

For a signing keys for EXE files (which you meant) must NEVER be done as you're asking to share private keys (possibly) containing dirkf's personal info!

edit: youtube-dl.exe from version 2021.12.17 isn't signed, for a PE file

MrRawes

This comment was marked as outdated.

@rautamiekka

This comment was marked as outdated.

@gamer191 gamer191 mentioned this pull request Jul 15, 2022
3 tasks
@afterdelight

This comment was marked as duplicate.

@dirkf

This comment was marked as outdated.

- name: Get Changelog
id: get_changelog
run: |
changelog=$(cat ChangeLog | grep -oPz '(?s)(?<=version ${{ steps.bump_version.outputs.ytdl_version }}\n{2}).+?(?=\n{2,3}version)') || true
Copy link

@MrRawes MrRawes Jul 18, 2022

Choose a reason for hiding this comment

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

you should change this

Suggested change
changelog=$(cat ChangeLog | grep -oPz '(?s)(?<=version ${{ steps.bump_version.outputs.ytdl_version }}\n{2}).+?(?=\n{2,3}version)') || true
changelog=$(grep -oPz '(?s)(?<=version ${{ steps.bump_version.outputs.ytdl_version }}\n{2}).+?(?=\n{2,3}version)' ChangeLog) || true

Copy link
Contributor

Choose a reason for hiding this comment

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

The advantage of cat file | whatever is that it puts the file at the start of the command, so decoupling it from the pipe being applied to it. < file whatever should have the same benefit without any feline intervention, but isn't widely used.

@Vangelis66
Copy link

Vangelis66 commented Feb 20, 2023

... Sorry for barging in 😉, I not being a python coder 😊, but,

  1. Does this PR aim to replicate "in a way" the automated youtube-dl releases to be found here?
  2. I'm asking because those binaries (youtube-dl.exe), as much as they resemble the "older" official releases by the previous team of maintainers, they still are compiled without native Crypto support, which, though not a deal breaker in itself, necessitates the downloading of a certain type of streams (mainly HLSe) via another dependency, FFmpeg...
  3. I have verified that the latest, as it stands, official release of 2021.12.17 has been built with the pycrypto-2.6.1 module:

ytdl

so my hope/wish/suggestion 😉 is any future binary release (based on this PR) by the new management also contain "native" Crypto support...

PS: The abandoned pycrypto module is sufficient for yt-dl purposes, however PyPI doesn't hold any wheels for it; the module contains C++ extensions, thus "installing it" from source requires compilation via a supported C++ compiler, compatible with the CPython version it's about to be used with...
Another option is to use the pycryptodome (not -x) module, for which PyPI wheels are available; but the end binary filesize will increase this way...

Kind regards 😃

@pukkandan
Copy link
Contributor

Another option is to use the pycryptodome (not -x) module, for which PyPI wheels are available; but the end binary filesize will increase this way...

As far as I remember, py2exe doesn't work well with it. Hence why yt-dlp_min doesn't use it

@dirkf
Copy link
Contributor

dirkf commented Feb 20, 2023

While ffmpeg isn't a hard requirement, so much yt-dl[p] functionality depends on it that I can't imagine not having it, except in some niche application of yt-dl.

Also, it solves the problem for the old Pythons that yt-dl is trying to support but which are not supported by pycryptodome. A Unix self-extracting build will use an installed Python and so find any pre-installed optional modules but I don't think that's true in the Windows version?

@Vangelis66
Copy link

FWIW, there have been cases in the past where I wanted to fetch geo-fenced media content served over HLSe (AES-128 encrypted), and the only geo-location circumvention means available to me was a SOCKS5 proxy (or a SSH tunnel).

youtube-dl does carry native support for SOCKS proxies via the proxy switch:

--proxy "socks5://proxyhost:proxyport"

so it passes this down to [hlsnative] which, when crypto is available, can successfully fetch HLSe over SOCKS...
FFmpeg, to my knowledge so far, still doesn't support the SOCKS proxy scheme, only HTTP[S]...

PS: It wasn't obvious to me back then, but, in the absence of crypto support, I could have used proxy-chaining (e.g. "The Web"<=> SOCKS5 proxy <=> Privoxy[HTTP] => FFmpeg), but why make things so complicated for the average, layman, user 😉?

but I don't think that's true in the Windows version?

Nope... The standalone Windows binaries built either by py2exe/pyinstaller come with the python interpreter itself and required extra python modules embedded:

[debug] yt-dlp version 2023.02.14 [8b37c58f8] (win_x86_exe)
[debug] Python 3.7.16 (CPython x86 32bit) - Windows-Vista-6.0.6003-SP2 (OpenSSL 1.1.1t  7 Feb 2023)
[debug] exe versions: ffmpeg n5.2-dev-2245-N-109649-gab8cde6 (setts), ffprobe n5.2-dev-2245-N-109649-gab8cde6, phantomjs 2.1.1, rtmpdump 2.4
[debug] Optional libraries: Cryptodome-3.17, brotli-1.0.9, certifi-2022.12.07, mutagen-1.46.0, sqlite3-2.6.0, websockets-10.4

i.e. no type of "installation" is required at all...

@nicolaasjan
Copy link

PS: The abandoned pycrypto module is sufficient for yt-dl purposes,

Do you think that's wise?
Buffer Overflow in pycrypto .

https://nvd.nist.gov/vuln/detail/CVE-2013-7459

Base Score: 9.8 CRITICAL

@dirkf dirkf linked an issue Feb 24, 2023 that may be closed by this pull request
3 tasks
@center0
Copy link

center0 commented Feb 25, 2023

The official response is to "Switch to pycryptodome", as a quick fix.

pycrypto/pycrypto#290 (comment)
pycryptotodome is incompatible with arm64 as of writing this comment as per below issue.

But it seems like a good quick fix to release something.

A better fix would be to move to the cryptography library, but i am not aware of what this would imply.
"most of the python ecosystem has moved on to using cryptography,
https://github.com/pyca/cryptography"

tencentyun/cos-python-sdk-v5#231

#30644

I would go with the better fix. But that's just me.
Wdyt?

@dirkf
Copy link
Contributor

dirkf commented Feb 25, 2023

yt-dl can't use cryptography because of missing legacy platform support.

It can use the common API of Pycrypto and Pycryptodome, relying on the user having installed the latter if suitable, or the former, or bundling the former otherwise. Though I'm not sure that Pycrypto will help ARM64 since it expects Py<=3.3. If there is a known security issue in Pycrypto it should be possible to fix it, though someone (maybe a Bellingcat-type open source investigator) who is really concerned about such issues should be running yt-dl in a sandbox.

@nicolaasjan
Copy link

If there is a known security issue in Pycrypto it should be possible to fix it

Debian still maintains pycrypto and have fixed the CVE-2013-7459 vulnerability (#849495).
See also the changelog.

python-crypto (2.6.1-7) unstable; urgency=high

  [ Salvatore Bonaccorso ]
  * Throw exception when IV is used with ECB or CTR (CVE-2013-7459)
    (Closes: #849495)

Someone should be able to build from source, I guess?
Source package available here.
python-crypto_2.6.1.orig.tar.gz

@pukkandan
Copy link
Contributor

Imho, this is being unnecessarily delayed and side tracked. Getting any release out, even one without deps or even unsigned will cover most people's needs and should be the priority imo. Automation, signing, pycryptodome, standalone unix binaries are all useful to have, but is not essential to a release and can be added later incrementally.

@Vangelis66
Copy link

Vangelis66 commented Mar 1, 2023

Do you think that's wise?
Buffer Overflow in pycrypto .

https://nvd.nist.gov/vuln/detail/CVE-2013-7459

Well, indeed, both pycrypto tags v2.6.1 and v2.7a1 (the second was cut from master on 20131021 but never made it to PyPI) are vulnerable to that CVE...

Be that as it may, in the linked article it's stated that CVE has been patched in the pycrypto master branch:
pycrypto/pycrypto#176 => pycrypto/pycrypto@8dbe0dc

Additionally, the pycrypto GH repo has seen in 2022 some minor activity, after a long period of inertia, but has now been made a Public archive... pycrypto/pycrypto@8dbe0dc is the very last code snapshot (with CVE-2013-7459 patched 👍 ).

pycrypto suffers from another publicly disclosed CVE, CVE-2018-6594, and that hasn't been patched in pycrypto master, but a fix is public here; doesn't seem to affect the purposes for which the module is used under youtube-dl ...

The "official" Win-binaries produced by the former team of devs seem to contain only a subset of the full pycrypto package, namely ONLY AES-related stuff:

Crypto 
  -- Cipher
     -- __init__.pyo
     -- _AES.pyd
     -- AES.pyo
     -- blockalgo.pyo
  -- Util
     -- __init__.pyo
     -- py3compat.pyo
  -- __init__.pyo

which is understandable, because "we" only need AES-128 decrypting support; if this stuff is still vulnerable to the mentioned CVEs, I'm not sure...

however PyPI doesn't hold any wheels for it;

For those interested and willing to trust third-party compiled wheels, GitHub holds:

py3.5 wheels of v2.6.1:

https://github.com/sfbahr/PyCrypto-Wheels/raw/master/pycrypto-2.6.1-cp35-none-win32.whl
https://github.com/hex-in/pycrypto-2.6.1/raw/master/dist/pycrypto-2.6.1-cp35-cp35m-win32.whl

py3.6 wheels of v2.6.1/v2.7a1:

https://github.com/hex-in/pycrypto-2.6.1/raw/master/dist/pycrypto-2.6.1-cp36-cp36m-win32.whl
https://github.com/hex-in/pycrypto/raw/master/dist/pycrypto-2.7a1-cp36-cp36m-win32.whl

py3.7 wheels of v2.6.1:

https://github.com/hex-in/pycrypto-2.6.1/raw/master/dist/pycrypto-2.6.1-cp37-cp37m-win32.whl

py3.8 wheels of v2.6.1:

https://github.com/hex-in/pycrypto-2.6.1/raw/master/dist/pycrypto-2.6.1-cp38-cp38-win32.whl

... and py3.4 wheels for v2.6.1 were available on www.voidspace.org.uk, but now that host has shut down; web archive link below:

https://web.archive.org/web/20200627032153/http://www.voidspace.org.uk/python/pycrypto-2.6.1/pycrypto-2.6.1-cp34-none-win32.whl

For the reasons I described in my previous post, I'd prefer youtube-dl.exe binaries, when other pending issues get resolved, to be Crypto enabled...

@nicolaasjan: You routinely compile your own, py3.4-based, Windows builds, with py2exe; can you please confirm that substituting pycrypto for pycryptodome-v3.9.9 results in non-functional builds (if the compilation succeeds at all, that is...) ?
Thanks in advance 😄 ...

@nicolaasjan

This comment was marked as off-topic.

@Vangelis66
Copy link

Pip couldn't find (pycryptodome) 3.9.9

Why not? It's certainly there, in wheel format for py3.4 x86:

https://files.pythonhosted.org/packages/3f/64/c096ccf34ed329c4a17cce65d38aa8b598e5f1e9275f53520cf3e68bfd53/pycryptodome-3.9.9-cp34-cp34m-win32.whl

especially since I specifically requested its creation and the module's author indulged 👍 ...

Legrandin/pycryptodome#228 (?)

Well, this comment 😉 from there describes an identical situation...
This appears related, which, in turn, leads to this workaround; does it change things at all?

BTW, and I think it might be an interesting read for the current maintainer, there's a related discussion between the two main former devs (dstftw and remitamine) on the topic of crypto libs and youtube-dl, as comments on 1bba88e 😉 ...

@nicolaasjan

This comment was marked as off-topic.

Copy link
Contributor

@ReenigneArcher ReenigneArcher left a comment

Choose a reason for hiding this comment

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

Some suggestions for fixing outdated/depreciated parts of this PR.

Hoping to give some momentum to this PR.

env:
PUSH_VERSION_COMMIT: ${{ secrets.PUSH_VERSION_COMMIT }}
if: "env.PUSH_VERSION_COMMIT == ''"
run: echo ::set-output name=version_suffix::$(date -u +"%H%M%S")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
run: echo ::set-output name=version_suffix::$(date -u +"%H%M%S")
run: echo "version_suffix=$(date -u +"%H%M%S")" >> $GITHUB_OUTPUT

set-output is depreciated

make issuetemplates
- name: Push to release
id: push_release
run: echo ::set-output name=head_sha::$(git rev-parse HEAD)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
run: echo ::set-output name=head_sha::$(git rev-parse HEAD)
run: echo "head_sha=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT

set-output is depreciated

Comment on lines +51 to +62
- name: Get SHA2-256SUMS for youtube-dl
id: sha256_bin
run: echo "::set-output name=sha256_bin::$(sha256sum youtube-dl | awk '{print $1}')"
- name: Get SHA2-256SUMS for youtube-dl.tar.gz
id: sha256_tar
run: echo "::set-output name=sha256_tar::$(sha256sum youtube-dl.tar.gz | awk '{print $1}')"
- name: Get SHA2-512SUMS for youtube-dl
id: sha512_bin
run: echo "::set-output name=sha512_bin::$(sha512sum youtube-dl | awk '{print $1}')"
- name: Get SHA2-512SUMS for youtube-dl.tar.gz
id: sha512_tar
run: echo "::set-output name=sha512_tar::$(sha512sum youtube-dl.tar.gz | awk '{print $1}')"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Get SHA2-256SUMS for youtube-dl
id: sha256_bin
run: echo "::set-output name=sha256_bin::$(sha256sum youtube-dl | awk '{print $1}')"
- name: Get SHA2-256SUMS for youtube-dl.tar.gz
id: sha256_tar
run: echo "::set-output name=sha256_tar::$(sha256sum youtube-dl.tar.gz | awk '{print $1}')"
- name: Get SHA2-512SUMS for youtube-dl
id: sha512_bin
run: echo "::set-output name=sha512_bin::$(sha512sum youtube-dl | awk '{print $1}')"
- name: Get SHA2-512SUMS for youtube-dl.tar.gz
id: sha512_tar
run: echo "::set-output name=sha512_tar::$(sha512sum youtube-dl.tar.gz | awk '{print $1}')"
- name: Get SHA2-256SUMS for youtube-dl
id: sha256_bin
run: echo "sha256_bin=$(sha256sum youtube-dl | awk '{print $1}')" >> $GITHUB_OUTPUT
- name: Get SHA2-256SUMS for youtube-dl.tar.gz
id: sha256_tar
run: echo "sha256_tar=$(sha256sum youtube-dl.tar.gz | awk '{print $1}')" >> $GITHUB_OUTPUT
- name: Get SHA2-512SUMS for youtube-dl
id: sha512_bin
run: echo "sha512_bin=$(sha512sum youtube-dl | awk '{print $1}')" >> $GITHUB_OUTPUT
- name: Get SHA2-512SUMS for youtube-dl.tar.gz
id: sha512_tar
run: echo "sha512_tar=$(sha512sum youtube-dl.tar.gz | awk '{print $1}')" >> $GITHUB_OUTPUT

set-output is depreciated

Comment on lines +64 to +94
- name: Create Release
id: create_release
uses: actions/create-release@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
tag_name: ${{ steps.bump_version.outputs.ytdl_version }}
release_name: youtube-dl ${{ steps.bump_version.outputs.ytdl_version }}
commitish: ${{ steps.push_release.outputs.head_sha }}
body: ${{ env.changelog }}
draft: false
prerelease: false
- name: Upload youtube-dl Unix binary
id: upload-release-asset
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ steps.create_release.outputs.upload_url }}
asset_path: ./youtube-dl
asset_name: youtube-dl
asset_content_type: application/octet-stream
- name: Upload Source tar
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ steps.create_release.outputs.upload_url }}
asset_path: ./youtube-dl.tar.gz
asset_name: youtube-dl-${{ steps.bump_version.outputs.ytdl_version }}.tar.gz
asset_content_type: application/gzip
Copy link
Contributor

Choose a reason for hiding this comment

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

This action is archived.

Suggested change
- name: Create Release
id: create_release
uses: actions/create-release@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
tag_name: ${{ steps.bump_version.outputs.ytdl_version }}
release_name: youtube-dl ${{ steps.bump_version.outputs.ytdl_version }}
commitish: ${{ steps.push_release.outputs.head_sha }}
body: ${{ env.changelog }}
draft: false
prerelease: false
- name: Upload youtube-dl Unix binary
id: upload-release-asset
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ steps.create_release.outputs.upload_url }}
asset_path: ./youtube-dl
asset_name: youtube-dl
asset_content_type: application/octet-stream
- name: Upload Source tar
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ steps.create_release.outputs.upload_url }}
asset_path: ./youtube-dl.tar.gz
asset_name: youtube-dl-${{ steps.bump_version.outputs.ytdl_version }}.tar.gz
asset_content_type: application/gzip
- name: Create Release
id: create_release
uses: ncipollo/release-action@v1
with:
tag: ${{ steps.bump_version.outputs.ytdl_version }}
name: youtube-dl ${{ steps.bump_version.outputs.ytdl_version }}
commit: ${{ steps.push_release.outputs.head_sha }}
body: ${{ env.changelog }}
draft: false
prerelease: false
artifacts: |
./youtube-dl,./youtube-dl.tar.gz

For other inputs see the readme: https://github.com/ncipollo/release-action

Comment on lines +126 to +135
- name: Upload youtube-dl.exe Windows binary
id: upload-release-windows
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ needs.build_unix.outputs.upload_url }}
asset_path: ./youtube-dl.exe
asset_name: youtube-dl.exe
asset_content_type: application/vnd.microsoft.portable-executable
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Upload youtube-dl.exe Windows binary
id: upload-release-windows
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
upload_url: ${{ needs.build_unix.outputs.upload_url }}
asset_path: ./youtube-dl.exe
asset_name: youtube-dl.exe
asset_content_type: application/vnd.microsoft.portable-executable
- name: Update Release
id: update-release-windows
uses: ncipollo/release-action@v1
with:
allowUpdates: true
tag: ${{ needs.build_unix.outputs.ytdl_version }}
name: youtube-dl ${{ needs.build_unix.outputs.ytdl_version }}
commit: ${{ needs.build_unix.outputs.head_sha }}
body: ${{ env.changelog }}
draft: false
prerelease: false
artifacts: |
./youtube-dl.exe

Comment on lines +136 to +141
- name: Get SHA2-256SUMS for youtube-dl.exe
id: sha256_win
run: echo "::set-output name=sha256_win::$((Get-FileHash youtube-dl.exe -Algorithm SHA256).Hash.ToLower())"
- name: Get SHA2-512SUMS for youtube-dl.exe
id: sha512_win
run: echo "::set-output name=sha512_win::$((Get-FileHash youtube-dl.exe -Algorithm SHA512).Hash.ToLower())"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Get SHA2-256SUMS for youtube-dl.exe
id: sha256_win
run: echo "::set-output name=sha256_win::$((Get-FileHash youtube-dl.exe -Algorithm SHA256).Hash.ToLower())"
- name: Get SHA2-512SUMS for youtube-dl.exe
id: sha512_win
run: echo "::set-output name=sha512_win::$((Get-FileHash youtube-dl.exe -Algorithm SHA512).Hash.ToLower())"
- name: Get SHA2-256SUMS for youtube-dl.exe
id: sha256_win
run: echo "sha256_win=$((Get-FileHash youtube-dl.exe -Algorithm SHA256).Hash.ToLower())" >> $GITHUB_OUTPUT
- name: Get SHA2-512SUMS for youtube-dl.exe
id: sha512_win
run: echo "sha512_win=$((Get-FileHash youtube-dl.exe -Algorithm SHA512).Hash.ToLower())" >> $GITHUB_OUTPUT

set-output is depreciated

Comment on lines +8 to +15
outputs:
version_suffix: ${{ steps.version_suffix.outputs.version_suffix }}
ytdl_version: ${{ steps.bump_version.outputs.ytdl_version }}
upload_url: ${{ steps.create_release.outputs.upload_url }}
sha256_bin: ${{ steps.sha256_bin.outputs.sha256_bin }}
sha512_bin: ${{ steps.sha512_bin.outputs.sha512_bin }}
sha256_tar: ${{ steps.sha256_tar.outputs.sha256_tar }}
sha512_tar: ${{ steps.sha512_tar.outputs.sha512_tar }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
outputs:
version_suffix: ${{ steps.version_suffix.outputs.version_suffix }}
ytdl_version: ${{ steps.bump_version.outputs.ytdl_version }}
upload_url: ${{ steps.create_release.outputs.upload_url }}
sha256_bin: ${{ steps.sha256_bin.outputs.sha256_bin }}
sha512_bin: ${{ steps.sha512_bin.outputs.sha512_bin }}
sha256_tar: ${{ steps.sha256_tar.outputs.sha256_tar }}
sha512_tar: ${{ steps.sha512_tar.outputs.sha512_tar }}
outputs:
version_suffix: ${{ steps.version_suffix.outputs.version_suffix }}
ytdl_version: ${{ steps.bump_version.outputs.ytdl_version }}
upload_url: ${{ steps.create_release.outputs.upload_url }}
sha256_bin: ${{ steps.sha256_bin.outputs.sha256_bin }}
sha512_bin: ${{ steps.sha512_bin.outputs.sha512_bin }}
sha256_tar: ${{ steps.sha256_tar.outputs.sha256_tar }}
sha512_tar: ${{ steps.sha512_tar.outputs.sha512_tar }}
head_sha: ${{ steps.push_release.outputs.head_sha }}

@Lesmiscore
Copy link
Contributor Author

🤷 seems like dirkf is working privately so

@Vangelis66
Copy link

Vangelis66 commented Apr 29, 2023

@Lesmiscore

Apologies for contacting you here/this way, but 😉 ...
... You appear to be the person behind the unofficial "nightly/daily" youtube-dl automated builds published below:

https://github.com/ytdl-patched/youtube-dl/tags

and produced via GHA; most sadly, the Rebase on Upstream workflow runs have been constantly failing for the past 23 (!) consecutive days:

https://github.com/ytdl-patched/youtube-dl/actions/workflows/rebase-on-upstream.yml

thus even the latest tag offered (2023.04.28.334) is built on stale source code from Mar 14th 2023 😭 ...

As I'm sure you're aware by now, your kind service is currently the source for updated yt-dl EXEs for many Windows users, but in master at least 17 commits "happened" after Mar 14th:

6fece0a...HEAD

among which the latest "fix" for Google/YT's throttling issue (which, thankfully, doesn't appear to still be an "issue" with latest yt-player 0c487f05 😜 ) ... I'm not asking for myself, mind you 😉 , I can build my own EXEs locally, but the "audience" you are targeting with your builds probably can't...

Huge thanks ❤️ in advance for an eventual resolution/fix...

Best regards 😄

@dirkf
Copy link
Contributor

dirkf commented Apr 29, 2023

I suppose this arises from the following chain of unpleasantness:

  • GH withdraws Ubuntu 18.04 runners
  • yt-dl test workflow is changed to skip test sets that needed it
  • nightly workflow fails because GH needs a special "password" when modifying workflow.

But now seems to be running, many thanks.

@Vangelis66
Copy link

Vangelis66 commented Apr 29, 2023

Thanks Lesmiscore for your immediate action on the reported issue 🥇 ... Latest "nightly" tag as of this writing, 2023.04.29.1919, is indeed built on 211cbfd 👍 !

However, to futureproof the service, it's MHO the relevant .yml files should be modified to integrate some "safety" checks, so that when:
a) a Rebase on Upstream workfow fails, the ensuing Build workflow shouldn't be run; it'd be a useless consumption of GHA resources, as the resulting artifacts would be identical, code-wise, to the ones produced in the immediately previous (successful) Build WF run; in addition, the repo owner should be somehow alerted about the Rebase on Upstream WF failure...
b) a Rebase on Upstream workfow succeeds, but the ensuing Build workflow fails (has indeed happened in the past); again, the repo owner should be made aware of such a failed scenario...

As I'm totally clueless myself about GHA, this is purely academic on my part, but I'm confident savvy people monitoring this PR would come up with applicable solutions (hopefully 😜 ) ...

dirkf: The https://github.com/ytdl-patched/youtube-dl repo doesn't (by design) have its issue tracker enabled; as the administrator of this repo, what would be the recommended place/way one should report a future "breakage" of the unofficial "daily" youtube-dl builds service?

@dirkf
Copy link
Contributor

dirkf commented Apr 29, 2023

For now, post in the yt-dl tracker.

@Vangelis66 Vangelis66 mentioned this pull request Apr 29, 2023
3 tasks
@Vangelis66
Copy link

... one should report a future "breakage" of the unofficial "daily" youtube-dl builds service?

Today's "daily" youtube-dl build, v2023.05.26.810, is still built on d1c6c5c from May 11th, whereas master is currently on 2389c7c from May 23rd 😞 ; that's because the relevant Rebase on Upstream workflow run failed; excerpt from the corresponding log:

+ git rebase upstream/master
Auto-merging youtube_dl/YoutubeDL.py
CONFLICT (content): Merge conflict in youtube_dl/YoutubeDL.py
Rebasing (1/1)
error: could not apply 3d8d94e6b... Automated Daily Builds
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 3d8d94e6b... Automated Daily Builds
Error: Process completed with exit code 1.

Regards.

@dirkf
Copy link
Contributor

dirkf commented May 26, 2023

1f7c6f8 changed some code that the downstream build is trying to modify. Let's see if b8a86dc is any better.

... No, try again.

@nicolaasjan
Copy link

Looks like the builds succeed again after manually triggered.

@Vangelis66

This comment was marked as resolved.

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.

New Release? RELEASE PLEASE