Skip to content

Add missing pieces for PR #1692 #1710

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
UdjinM6 merged 7 commits into from
Dec 19, 2017
Merged

Add missing pieces for PR #1692 #1710

UdjinM6 merged 7 commits into from
Dec 19, 2017

Conversation

@OlegGirko
Copy link

@OlegGirko OlegGirko commented Nov 2, 2017 •
edited
Loading

This PR fixes build and test problems for PR #1692.

See individual commits for more detailed descriptions of changes and why they are needed.

These fixes work when building on modern Fedora 26.
However Travis tests may fail for now, but I know why and working on fix for old Ubuntu that Travis uses.

fanquake and others added 3 commits November 2, 2017 01:31
@fanquake @OlegGirko
[depends] fontconfig 2.12.1
739aa0a
@OlegGirko
[depends] fontconfig 2.12.6
c6c2c4e 
This fixes build with glibc 2.25 caused by conflict with CHAR_WIDTH macro
from TS 18661-1:2014.

Also, a patch is added to remove some redundant declarations from fcobjs.c
file because they may conflict with include file autogenerated by gperf.

Signed-off-by: Oleg Girko <ol@infoserver.lv>
@OlegGirko
Don't set PKG_CONFIG_SYSROOT_DIR env variable to build Qt.
3adac27 
Setting PKG_CONFIG_SYSROOT_DIR environment variable to "/"
for building Qt makes pkgconf behave strange
(remove "/" prefix instead of adding it).

And it makes no sense to set this variable for old pkgconfig anyway.

Signed-off-by: Oleg Girko <ol@infoserver.lv>
@OlegGirko
Copy link
Author

OlegGirko commented Nov 2, 2017

Now only macOS build fails because it's unable to download macOS SDK.

@schinzelh
Copy link

schinzelh commented Nov 2, 2017

Thanks, i think I'll add Python3 migration to #1692 instead of removing the utf-8 characters though

@UdjinM6 UdjinM6 added this to the 12.3 milestone Nov 2, 2017
@OlegGirko
Copy link
Author

OlegGirko commented Nov 3, 2017

Python 3 migration (Bitcoin PR bitcoin#7814) is quite intrusive, changing 72 files.
Removing two UTF-8 graphics symbols from strings is much simpler. They make no sense anyway.

@OlegGirko
Copy link
Author

OlegGirko commented Nov 7, 2017

OK, I've backported Bitcoin PR bitcoin#7814 that switches tests to use Python 3.

@OlegGirko
Copy link
Author

OlegGirko commented Nov 7, 2017

All tests are green except the macOS one: it's unable to download SDK.

@OlegGirko OlegGirko changed the title [WIP] Add missing pieces for PR #1692 Add missing pieces for PR #1692 Nov 7, 2017
@schinzelh
Copy link

schinzelh commented Nov 7, 2017 •
edited
Loading

@OlegGirko please change the domain in SDK_URL in travis.yml from bitcoin.jonasschnelli.ch to static.dashdot.io

@OlegGirko
Copy link
Author

OlegGirko commented Dec 7, 2017

@schinzelh: Doesn't work. "404 Not Found".

@UdjinM6
Copy link

UdjinM6 commented Dec 7, 2017

Why is it not https://bitcoincore.org/depends-sources/sdks which is the current one? #1692 changed it for some reason but it probably shouldn't.

@UdjinM6
Copy link

UdjinM6 commented Dec 7, 2017 •
edited
Loading

#1692 in general looks quite messy now and should be reworked imo. I'd prefer it to be more like #1697 tbh i.e. backport commits that merge corresponding PRs instead of backporting the whole set of individual commits inside each of these PRs (in case a PR is backported with all of its commits ofc).

@OlegGirko
Copy link
Author

OlegGirko commented Dec 7, 2017

OK, I've just added backport of Bitcoin PR bitcoin#8304 to fix SDK_URL instead of my own fix. Let's see how it works.

@OlegGirko
Copy link
Author

OlegGirko commented Dec 7, 2017

@UdjinM6: I'd prefer to have have one Dash PR for one backported Bitcoin PR.
And it would be better to merge each PR without fast-forwarding instead of squashing it into a single commit.

@OlegGirko
Copy link
Author

OlegGirko commented Dec 7, 2017

OK, now all tests pass successfully.

UdjinM6
UdjinM6 approved these changes Dec 19, 2017
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 merged commit c9c36ca into dashpay:update_depends Dec 19, 2017
@OlegGirko OlegGirko deleted the pr-1692 branch December 19, 2017 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

12.3

Development

Successfully merging this pull request may close these issues.

6 participants

@OlegGirko @schinzelh @UdjinM6 @fanquake @sipa @laanwj