Build failing on unstable update for PRs from fork

[pkienzle]

PR #151 is getting HTTP 403 Forbidden.

It looks like this is because the source purnimab:master is not part of reflectometry and so it cannot push the windows unstable exe to the release page. [Link may be broken if github expires build output.]

We still need to build and test from the fork but don't try deploy. I think we can do this by adding the following to unstable.yml:

jobs:
    test_and_build:
        …
    publish:
        if: github.repository_owner == 'reflectometry'
        …
    updateUnstableText:
        if: github.repository_owner == 'reflectometry'
        …

From prisma/prisma#3539

[bmaranville]

Ah - I applied a fix to the wrong file (test.yml, not unstable.yml) in 140c861

I was just going to disable publish for all pull requests, because I don't want the publish action to happen even if the pull request originates from a 'reflectometry' repo.

[pkienzle]

Do we want to tag the unstable with the branch name and/or commit hash?

That might remove some potential confusion if a change to another PR replaces unstable you have had a chance to grab it. In particular, if you send a link to a collaborator in an email you don't want them unexpectedly grabbing the wrong file.

This means we will have to remove previous builds. If we use a consistent naming pattern Refl1D-latest-COMMIT-… then we can remove Refl1D-latest*. (I see that we are using Refl1D-windows-exe-latest.zip; that should change to Refl1D-latest-windows-exe.zip even if we don't stamp the builds.

If we encode the date in the stamp then maybe we can remove everything more than one week old. If files are named for example Refl1D-unstable-YYYYMMDD-COMMIT-* then maybe we can remove anything less than Refl1D-unstable-$STAMP where STAMP=$(date -j -v-7d "+%Y%m%d")

[bmaranville]

Tagging is a good idea, so that people know what they've downloaded. Don't know if it's worth keeping previous builds. It might be just as hard to delete all previous builds as it is to keep some of them, though.

I would still prefer not to publish from any pull requests.

[pkienzle]

Publishing PRs lets you share the prebuilt app with the user who raised the issue before pulling into master. If you haven't had occasion to do this then I guess we don't need it.

[bmaranville]

I have occasionally wanted such a thing.

[pkienzle]

Build_Unstable is failing for me as well on merge with master:

could not find upload_url corresponding to release unstable

[bmaranville]

That's very odd... I re-ran the failing job and it worked the second time. Momentary github API failure?

[pkienzle]

Github gaslight: the above link now shows success :-(

Is it possible that the secrets are associated with bmaranville rather than reflectometry?

[bmaranville]

I don't think the credentials are tied to who pushes the button to re-run the job.

[pkienzle]

Just tried it. Yes it worked. I'll go with the "momentary failure" hypothesis for now.