Skip to content

Don't retrieve repository's SHA in prebuild step. #1426

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 28, 2021
Merged

Don't retrieve repository's SHA in prebuild step. #1426

merged 1 commit into from
Oct 28, 2021

Conversation

daleglass
Copy link
Contributor

@daleglass daleglass commented Oct 24, 2021 •
edited
Loading

Doing this means source can only be built when checked out from git and can't be build from an archive.

It's also completely useless. All that's being done is outputting it into the log.

@daleglass
Don't retrieve repository's SHA in prebuild
526f526 
Doing this means source can only be built when checked out from git,
and can't be build from an archive.
@vircadia-build-notifier
Copy link

The following links are available:
build (macOS-10.15, client)

build (ubuntu-18.04, full)

build (macOS-10.15, full)

build (self-hosted_debian-11_aarch64, full)

build (windows-latest, full)

@digisomni
Copy link
Member

Is it not possible that this check is meant for automated builds / checking consistency to ensure what's being built is the same as another? What about just converting it to throw a log message or two upon failure but not actually stop the build from continuing?

@daleglass
Copy link
Contributor Author

@digisomni Maybe, but I don't think we really need it to be here.

If you're checking out something from git in GHA, Jenkins or such, it's trivial to get the commit's hash right there. I think that's where such code should go.

This change was made to make it easier to generate packages. Currently builds fail if you try to build against a .zip downloaded from github due to this code.

@Penguin-Guru
Copy link

It is also good to simplify the build system and reduce dependencies.

@digisomni
Copy link
Member

Good point, if we want SHA checking it may be included in whatever CI/CD application it is.

@digisomni digisomni added housekeeping Code or documentation cleanup needs CR (code review) labels Oct 25, 2021
@digisomni digisomni added this to the 2021.2.0 Selene Release milestone Oct 25, 2021
digisomni
digisomni approved these changes Oct 25, 2021
View reviewed changes
Copy link
Member

@digisomni digisomni left a comment

Choose a reason for hiding this comment

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

It built on all platforms so I assume that all instances were caught.

@digisomni digisomni added CR Approved At least one code reviewer has approved the PR. and removed needs CR (code review) labels Oct 25, 2021
@daleglass daleglass merged commit 54db82b into vircadia:master Oct 28, 2021
@digisomni digisomni changed the title Don't retrieve repository's SHA in prebuild Don't retrieve repository's SHA in prebuild STEP. Oct 30, 2021
@digisomni digisomni changed the title Don't retrieve repository's SHA in prebuild STEP. Don't retrieve repository's SHA in prebuild step. Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@digisomni digisomni digisomni approved these changes

Assignees
No one assigned
Labels
CR Approved At least one code reviewer has approved the PR. housekeeping Code or documentation cleanup
Projects
None yet
Milestone
2022.1.0 Selene Release
Development

Successfully merging this pull request may close these issues.
4 participants
@daleglass @vircadia-build-notifier @digisomni @Penguin-Guru