Add patches to builds #8086

Merged
merged 9 commits into from Nov 29, 2017

Conversation

Projects
None yet
5 participants
Owner

nskaggs commented Nov 15, 2017

Description of change

This adds support for applying the patches during snap creation, or via a local build using the makefile. This effectively deprecates the old python script. The script has been left in for now until the CI build process has transitioned away from using it.

QA steps

Build juju with make build and snapcraft. Ensure the patches are applied as part of the process.

Makefile
@@ -49,6 +49,7 @@ godeps:
endif
build: godeps
+ cat $(PWD)/patches/*.diff | patch -f -u -p1 -r- -d $(PWD)/../../../
@wallyworld

wallyworld Nov 15, 2017

Owner

We can't modify what is supposed to be a clean source tree - this will mess with people's repos.

@nskaggs

nskaggs Nov 15, 2017

Owner

Yea, everyone seems against overloading 'build'. I moved it to release-build, and added an 'undo' patches.

Makefile
+
+release-build: godeps
+ cat $(PWD)/patches/*.diff | patch -f -u -p1 -r- -d $(PWD)/../../../
+ go build $(PROJECT)/...
@howbazaar

howbazaar Nov 15, 2017

Owner

probably install rather than build

@howbazaar

howbazaar Nov 15, 2017

Owner

How about another target that does the relese build then remove patches.

@howbazaar

howbazaar Nov 15, 2017

Owner

These are both phony tasks.

Owner

nskaggs commented Nov 15, 2017

Updated to add even more targets, and recursively cleanup after release-install

Owner

nskaggs commented Nov 24, 2017

@howbazaar can you review again please?

+build: godeps go-build
+
+add-patches:
+ cat $(PWD)/patches/*.diff | patch -f -u -p1 -r- -d $(PWD)/../../../
@wupeka

wupeka Nov 28, 2017

Member

Maybe create a .PATCHED file marking that the source tree is patched, added (and checked) in add-patches and removed in remove-patches? This way calling add-patches twice won't break anything

@nskaggs

nskaggs Nov 29, 2017

Owner

Calling add-patches twice will fail to re-apply the patch, but doesn't break anything. It can still be safely removed with remove-patches.

Owner

nskaggs commented Nov 29, 2017

$$merge$$

Contributor

jujubot commented Nov 29, 2017

Status: merge request accepted. Url: http://ci.jujucharms.com/job/github-merge-juju

Contributor

jujubot commented Nov 29, 2017

Build failed: Tests failed
build url: http://ci.jujucharms.com/job/github-merge-juju/614

Contributor

jujubot commented Nov 29, 2017

Build failed: Tests failed
build url: http://ci.jujucharms.com/job/github-merge-juju/615

@jujubot jujubot merged commit 558d7da into juju:develop Nov 29, 2017

1 check failed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment