Skip to content

Conversation

@umlaeute
Copy link

Description

This adds a new job to the BUILD workflow, that generates a tarball with all the submodules (as GitHub's "Source Download" will happily exclude all the submodules, resulting in an incomplete tarball that fails to build).

The generated tarball excludes all repository configuration files (everything starting with .git*),
in order to not interfere with downstream consumers of the tarball, who might have their own ideas how to configure a repository.

@PatTheMav suggested in #7407 (comment) that i should extend the build_linux job, which I did not do for the following reasons:

  • the build-artifacts are platform-independent (there's nothing "linux"-specific to them, except that i picked tar.gz as the output format)
  • there's little point in creating the same tarball on multiple Ubuntu versions
  • the tarballs might be of interest, even if the Linux builds fail (for whatever reasons)

Motivation and Context

This is motivated by creating the Debian-official packages of obs-studio ("Debian-official" being the packages that are shipped by Debian itself (and thus by derivatives like Ubuntu), as opposed to .deb-packages shipped by obs-studio itself).

On Debian, we use release tarballs for creating our packages, and as such it is important that these packages contain all the necessary bits and bolts.

Closes: #7407

How Has This Been Tested?

This has been tested by running the workflow in my fork of OBS-studio.
https://github.com/umlaeute/obs-studio/actions/runs/3135647019

For whatever reasons, all the other build-jobs fail with some Cmake specific error. Since I did not touch these jobs, I think the problem is somewhere else (e.g. hidden in the project settings).

Types of changes

?

Checklist:

  • My code has been run through clang-format. (does not apply)
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation. (does not apply)

@umlaeute
Copy link
Author

sidenote: the action i linked above as a "proof of test" is obviously on my master branch and contains multiple commits.
this is because the given job is limited to only run on master and release/* branches (so it wouldn't run on my release-tarballs branch).
the commits have been squashed into the final branch for this PR.

@gxalpha gxalpha added Seeking Testers Build artifacts on CI CI labels Sep 27, 2022
Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

This seems overly complicated to me - I'm fine with running this as a separate job for the reasons you mentioned, but the whole business of manipulating files and appending submodules to the tar archive seems unnecessary to me.

Just run git ls-files --recurse-submodules | tar caf ../obs-studio.tar.gz -T- in the checkout directory and don't mind the .gitignore files that might be left over - IMO they belong to our source tree.

From what I could tell that single command gives us all the files from all submodules, except if I missed something?

You also forgot to add the artefact to the release creation job - we'd also want to add the tarball to our releases as well then.

@umlaeute
Copy link
Author

don't mind the .gitignore files that might be left over - IMO they belong to our source tree.

how so?
having repository configuration in a source tarball is an endless source of troubles for us downstream packagers.

  1. when you download the tarball, there is no repository anywhere. what's the benefit of having repository configuration in it then?
  2. if you then import the tarball into your own repository, the configuration for a different repository (upstream) keeps interfering with the downstream needs.

the thing is: our needs are different from yours. we do want to see any build artifacts that is left around.

you might of course argue that we could simply remove the repository configuration that we don't like on our side, but in general we want to change the upstream sources as little as possible, and as i don't see any reason to do include the repo-config in an explicitly non-repo artifact, i ask you to just remove it. if the source tarballs had already been useable as such, i would have submitted a patch to drop any repository configuration anyway.

@PatTheMav
Copy link
Member

Fine by me, still git ls-files --recurse-submodules -- . ':!:.git*' | tar caf ../obs-studio.tar.gz -T- should get you a working tarball in a single command - or are there files missing from that archive?

@umlaeute
Copy link
Author

umlaeute commented Sep 30, 2022

Fine by me, still git ls-files --recurse-submodules -- . ':!:.git*' | tar caf ../obs-studio.tar.gz -T- should get you a working tarball in a single command - or are there files missing from that archive?

your command is of course much simpler and more elegant than mine.
it doesn't exclude the .git* files in subdirectories, though. (i'll try to come up with a solution for this, which might take over the weekend - there are other pressing matters to attend to :-))

@PatTheMav
Copy link
Member

Fine by me, still git ls-files --recurse-submodules -- . ':!:.git*' | tar caf ../obs-studio.tar.gz -T- should get you a working tarball in a single command - or are there files missing from that archive?

your command is of course much simpler and more elegant than mine. it doesn't exclude the.git*` files in subdirectories, though. (i'll try to come up with a solution for this, which might take over the weekend - there are other pressing matters to attent to :-))

That’s weird - the Git glob expression should take care of that, might need some tweaking.

@Polynomial-C
Copy link

Polynomial-C commented Nov 6, 2022

If you're not using an ancient tar version, you could also use the --exclude-vcs and --exclude-vcs-ignoresoption.

@umlaeute
Copy link
Author

umlaeute commented Nov 7, 2022

so i force pushed a change using the suggestion by @Polynomial-C

the code has become significantly simpler, basically running

  • tar -cvzf ${outfile} \
  • --transform='s|^\.|obs-studio|' \ - as we are running this from checked out source tree
  • --exclude-vcs --exclude-vcs-ignores --exclude '.git*' \ - removing all repository data and configuration
  • --sort=name --owner=0 --group=0 --numeric-owner --pax-option="exthdr.name=%d/PaxHeaders/%f,delete=atime,delete=ctime" --mtime="${commit_date}" \ - making the tarball reproducible (so multiple runs should create exactly the same tarball)
  • . - the source tree

a test-run of the job can be found at https://github.com/umlaeute/obs-studio/actions/runs/3410965320/jobs/5674593971
the entire workflow with the produced artifact is available at https://github.com/umlaeute/obs-studio/actions/runs/3410965320

@PatTheMav
Copy link
Member

Nicely done, looks neat now. I will take a closer look tomorrow but looks fine at a first glance.

@umlaeute
Copy link
Author

ping @PatTheMav

@umlaeute
Copy link
Author

ping...

@derrod derrod requested a review from PatTheMav January 26, 2023 12:20
Copy link
Member

@PatTheMav PatTheMav left a comment

Choose a reason for hiding this comment

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

Seems fine to me - @tytan652 any objections to merge this?

Copy link
Collaborator

@tytan652 tytan652 left a comment

Choose a reason for hiding this comment

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

Running this all the time is my biggest complain, creating useless artifacts except when we need one for a release/tag.

Edit: But there is no alternative, if I understood correctly.

Add a new job to the BUILD workflow, that generates a tarball with all
the submodules (as GitHub's "Source Download" will happily exclude all
the submodules, resulting in an incomplete tarball that fails to build).

The generated tarball excludes all repository configuration files
(everything starting with .git*, as to not interfere with downstream
consumers of the tarball, who might have their own ideas how to
configure a repository).

Closes: obsproject#7407
@umlaeute
Copy link
Author

umlaeute commented Feb 13, 2023

Running this all the time is my biggest complain, creating useless artifacts except when we need one for a release/tag.

Edit: But there is no alternative, if I understood correctly.

afaict, the current pipelines will generate the following artifacts on each push:

artifact size
obs-studio-flatpak-5b370d4e3-x86_64 139 MB
obs-studio-macos-arm64-5b370d4e3 146 MB
obs-studio-macos-x86_64-5b370d4e3 156 MB
obs-studio-ubuntu-20.04-5b370d4e3 108 MB
obs-studio-ubuntu-20.04-5b370d4e3-dbgsym 80.8 MB
obs-studio-ubuntu-22.04-5b370d4e3 108 MB
obs-studio-ubuntu-22.04-5b370d4e3-dbgsym 72.9 MB
obs-studio-windows-x64-5b370d4e3 186 MB
obs-studio-windows-x86-5b370d4e3 167 MB
TOTAL 1160MB

the source-job adds another artifact with a size of approximately 32MB.
The worry that this job imposes a problem, seems a bit ... exaggerated to me.

But anyhow: I see that e.g. the Windows Installer resp macOS notarized image jobs seem to only run for releases by using

if: ${{ startsWith(github.ref, 'refs/tags/') && github.event_name != 'pull_request' }}

if you want I can add this condition (but that means that the pipeline won't be run before you merge, which imho is even less desirable; but ymmv, so just tell me if you require me to do so).

@umlaeute
Copy link
Author

@tytan652 how does this sound?

@tytan652
Copy link
Collaborator

@tytan652 how does this sound?

I let @PatTheMav having the final say.

The worry that this job imposes a problem, seems a bit ... exaggerated to me.

What I talked about was a piece of the problem. The main problem was discussed off-thread.

@umlaeute
Copy link
Author

What I talked about was a piece of the problem. The main problem was discussed off-thread.

ah i see. sorry for being snarky then.
is the off-thread discussion available somewhere for external people like me?

@tytan652
Copy link
Collaborator

is the off-thread discussion available somewhere for external people like me?

It was between OBS developers, so no.
But long-story short it was a quick discussion about:

  • how we do releases and this PR
  • the WIP future CI rewrite

So do not worry about any of that.

@umlaeute
Copy link
Author

@tytan652 how does this sound?

I let @PatTheMav having the final say.

any news on this?

@umlaeute
Copy link
Author

umlaeute commented Jun 3, 2023

@tytan652 @PatTheMav ping

@umlaeute
Copy link
Author

ping @tytan652 @PatTheMav

@umlaeute
Copy link
Author

umlaeute commented Aug 3, 2023

ping @tytan652 @PatTheMav

@PatTheMav
Copy link
Member

This is already partially implemented in the updated workflows, but will be enabled once Ubuntu-based builds use the updated CMake path sometime after the next release (similar to how it's already implemented in the plugintemplate).

As this was based on the old workflows and is not compatible with the updated CI system, I'll close this PR. But the feature will definitely be implemented.

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

Labels

CI Seeking Testers Build artifacts on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

release tarballs miss required sources

5 participants