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

Fix building of develop branch without adding vulnerabilities #1225

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

hans-schmidt
Copy link
Contributor

@hans-schmidt hans-schmidt commented Jan 5, 2023

In Sept-2022 the qt project changed the URL for downloading qt-5.12.11, which broke Ravencoin's build system.
Fixing this for the same version of qt requires only updating the URL as was done for Evrmore in Oct-2022.

Ravencoin PR-1224 was merged recently, but it was merged (perhaps unintentionally?) into the master branch.

I believe that PR also makes numerous unneeded changes including:
-making minor changes to the consensus ProgPow files in src/crypto/ethash/lib
-disabling libravenconsensus from being built
-downgrading libzmq from v4.3.4 to v4.3.1, which is undesirable since v4.3.1 contains multiple security issues including a heap overflow vulnerability.

This PR affects only develop branch at this time. But I suggest that action should be taken regarding PR-1224 before using master branch for the next release.

See also PR #1226

@barrystyle
Copy link

Develop branch is missing commit c8197cd, rendering this PR irrelevant.

Copy link
Contributor

@HyperPeek HyperPeek left a comment

Choose a reason for hiding this comment

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

Only change is download url from official sources to fix build of develop, approved

@HyperPeek
Copy link
Contributor

Develop branch is missing commit c8197cd, rendering this PR irrelevant.

The question is how the next release should be prepared -- before we did all testing in develop and only merged to master upon comprehensive discussion and checks for the release. I suggest to discuss the workflow once more so we do not end up with a complex mix of branches that are difficult to maintain.

@jeroz1
Copy link
Contributor

jeroz1 commented Jan 5, 2023

Thank you for the clarifications and discussions.

It seems that this PR by itself excluding most recent change to master, does not result in a successful build on the master branch.

See: https://github.com/barrystyle/Ravencoin/actions/runs/3845620333/jobs/6549974270

Can someone else replicate this?

@hans-schmidt
Copy link
Contributor Author

The normal workflow has been to do all work on develop branch and then merge to master branch as features are verified release-ready. Building and experimenting in between releases was always done on develop branch. So we didn't normally build master in between releases, and in the past we did not make changes to master independent of develop to fix changes made by dependency projects (v4.3.2.1 was not buildable from master for a long period of time due to such changes by dependencies).

I don't think it's a good idea to make changes on develop and master branches independently since that increases the risk of bugs and security failures.

@hans-schmidt
Copy link
Contributor Author

Thank you for the clarifications and discussions.

It seems that this PR by itself excluding most recent change to master, does not result in a successful build on the master branch.

See: https://github.com/barrystyle/Ravencoin/actions/runs/3845620333/jobs/6549974270

Can someone else replicate this?

Not true. See PR #1226

@HyperPeek
Copy link
Contributor

I agree. When building from the "clean" master after the release merge it only requires the single url update of PR #1226 last commit (the only non-reverting one). I consider this a far cleaner approach as it also leaves the master without code change which is highly desirable for any future release.

@hans-schmidt
Copy link
Contributor Author

Develop branch is missing commit c8197cd, rendering this PR irrelevant.

Not true.

develop branch at v4.6.1 release:
commit 7864c39

master branch at v4.6.1 release:
commit c8197cd

If you do a "git diff" you will see that they are identical. They have different hashes only because of the odd way that github handles merges.

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.

4 participants