Skip to content

Conversation

dongcarl
Copy link
Contributor

@dongcarl dongcarl commented Sep 9, 2019

Note: Will Did open issue about robustness of sed calls in depends.

@DrahtBot
Copy link
Contributor

Gitian builds for commit 8af835a (master):

Gitian builds for commit 97190bf (master and this pull):

@@ -178,9 +178,9 @@ define $(package)_preprocess_cmds
patch -p1 -i $($(package)_patch_dir)/no-xlib.patch &&\
echo "QMAKE_LINK_OBJECT_MAX = 10" >> qtbase/mkspecs/win32-g++/qmake.conf &&\
echo "QMAKE_LINK_OBJECT_SCRIPT = object_script" >> qtbase/mkspecs/win32-g++/qmake.conf &&\
sed -i.old "s|QMAKE_CFLAGS = |!host_build: QMAKE_CFLAGS = $($(package)_cflags) $($(package)_cppflags) |" qtbase/mkspecs/win32-g++/qmake.conf && \
sed -i.old "s|QMAKE_CFLAGS += |!host_build: QMAKE_CFLAGS = $($(package)_cflags) $($(package)_cppflags) |" qtbase/mkspecs/win32-g++/qmake.conf && \
sed -i.old "s|QMAKE_LFLAGS = |!host_build: QMAKE_LFLAGS = $($(package)_ldflags) |" qtbase/mkspecs/win32-g++/qmake.conf && \
Copy link
Member

@laanwj laanwj Sep 10, 2019

Choose a reason for hiding this comment

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

doesn't look like this line is working, there's no bare "QMAKE_LFLAGS" in my win32-g++/qmake.conf at all (qt 5.9.7 nor 5.9.8)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Digging a little into this... It would seem that QMAKE_LFAGS was removed in qt/qtbase@39fc377, and first made it into qt v5.9.0. We can just add QMAKE_LFLAGS = $($(package)_ldflags) back in though.

Copy link
Contributor Author

@dongcarl dongcarl Sep 16, 2019

Choose a reason for hiding this comment

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

See 1b4030e for my fix.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me, thanks for addressing this

QMAKE_LFLAGS was removed from qtbase/mkspecs/win32-g++/qmake.conf in
39fc377bf105ba09e2a8f9acae467dc789b96525. Here, we add it back in with
our LDFLAGS from depends before the first occurance of any
QMAKE_LFLAGS_* variable settings.
@dongcarl dongcarl force-pushed the 2019-09-fix-qt-cflags-pickup branch from aaec508 to 1b4030e Compare September 16, 2019 16:58
@dongcarl dongcarl changed the title depends: qt: Fix C{,XX}FLAGS pickup depends: qt: Fix {C{,XX},LD}FLAGS pickup Sep 17, 2019
fanquake added a commit that referenced this pull request Sep 18, 2019
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at #16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
@laanwj
Copy link
Member

laanwj commented Sep 18, 2019

Requested a new gitian build. ACK from me if it passes.

@DrahtBot
Copy link
Contributor

Gitian builds for commit 4b5e5ef (master):

Gitian builds for commit 5fde90b (master and this pull):

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 23, 2019
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
@laanwj
Copy link
Member

laanwj commented Sep 26, 2019

ACK 1b4030e

laanwj added a commit that referenced this pull request Sep 26, 2019
1b4030e depends: qt: Fix LDFLAGS pickup (Carl Dong)
6eb12ff depends: qt: Fix C{,XX}FLAGS pickup (Carl Dong)

Pull request description:

  Note: ~~Will~~ [Did](#16838) open issue about robustness of `sed` calls in `depends`.

ACKs for top commit:
  laanwj:
    ACK 1b4030e

Tree-SHA512: d0bfc8ea32118cd90bb323efab58661f2218a2cb0f150e716cfd5355c7e2a1eba70298a144b159941248170e2894659c376219edac2c79a9d777f6ada5fa6b2f
@laanwj laanwj merged commit 1b4030e into bitcoin:master Sep 26, 2019
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 26, 2019
1b4030e depends: qt: Fix LDFLAGS pickup (Carl Dong)
6eb12ff depends: qt: Fix C{,XX}FLAGS pickup (Carl Dong)

Pull request description:

  Note: ~~Will~~ [Did](bitcoin#16838) open issue about robustness of `sed` calls in `depends`.

ACKs for top commit:
  laanwj:
    ACK 1b4030e

Tree-SHA512: d0bfc8ea32118cd90bb323efab58661f2218a2cb0f150e716cfd5355c7e2a1eba70298a144b159941248170e2894659c376219edac2c79a9d777f6ada5fa6b2f
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Apr 3, 2020
Summary:
Backport of core [[bitcoin/bitcoin#16837 | PR16837]].

Depends on D5650.

Test Plan: Run the Gitian builds.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5656
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Dec 18, 2020
1b4030e depends: qt: Fix LDFLAGS pickup (Carl Dong)
6eb12ff depends: qt: Fix C{,XX}FLAGS pickup (Carl Dong)

Pull request description:

  Note: ~~Will~~ [Did](bitcoin#16838) open issue about robustness of `sed` calls in `depends`.

ACKs for top commit:
  laanwj:
    ACK 1b4030e

Tree-SHA512: d0bfc8ea32118cd90bb323efab58661f2218a2cb0f150e716cfd5355c7e2a1eba70298a144b159941248170e2894659c376219edac2c79a9d777f6ada5fa6b2f
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 27, 2021
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 28, 2021
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jun 29, 2021
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 1, 2021
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 12, 2021
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 13, 2021
93995c2 build: remove unnecessary qt xcb patching (fanquake)
4d45577 build: remove unnecessary macOS qt patching (fanquake)

Pull request description:

  While looking at bitcoin#16838 I found at least two cases of `sed` patching in depends that now seems to be redundant. There's possibly a [third case](bitcoin#16837 (comment)), but I haven't looked into that enough yet.

  Patching `0` -> `kCGMouseButtonLeft` should not be required, as [`kCGMouseButtonLeft`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/cocoa/qcocoacursor.mm?h=5.9#n82) has been used in the `cocoa/qcocoacursor.mm` source for a while.

  The include we were modifying in [`src/plugins/platforms/xcb/qxcbxsettings.cpp`](https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9#n47) was removed in [this commit](https://code.qt.io/cgit/qt/qtbase.git/commit/src/plugins/platforms/xcb/qxcbxsettings.cpp?h=5.9&id=78731b434e0e99ad108601249108e12d8a49c350).

ACKs for top commit:
  dongcarl:
    ACK 93995c2

Tree-SHA512: 5e0cbf317b798ce2e142a42b7fabf1d9e8e00d12f59589e98d790195ba27db60858f933b035c6e9cd0deadd8c3406f1ff4a4ed2af4a19e9b5b43aa97d04b9ecb
kwvg added a commit to kwvg/dash that referenced this pull request Nov 24, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants