Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@mehmetf
Copy link
Contributor

@mehmetf mehmetf commented Sep 18, 2018

It is common for toolchains to #define unix .. or #define win .. which breaks the compilation.

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

Which toolchain is this?

@mehmetf
Copy link
Contributor Author

mehmetf commented Sep 19, 2018

This is the internal Google toolchain.

@jamesderlin
Copy link
Contributor

Preprocessor macros that aren't in all uppercase make me sad. =(

(Conversely, things that aren't preprocessor macros that are in all uppercase also make me sad.)

@mehmetf
Copy link
Contributor Author

mehmetf commented Sep 19, 2018

I agree. I want to also talk to our C++ gurus to see how we can augment that behavior. #define unix 1 just seems like a pretty bad idea in general; almost like reserving a keyword in an absurd way.

I just figured this is pretty harmless for the engine and well contained. If it was touching 10+ files, I would not have done it.

@mehmetf mehmetf merged commit 2b25fd7 into flutter:master Sep 19, 2018
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 19, 2018
flutter/engine@2e8e96f...2b25fd7

git log 2e8e96f..2b25fd7 --no-merges --oneline
2b25fd7 Don't use unix or win namespaces (flutter/engine#6277)
60c5bb8 Return null instead of throwing if an instance method is passed to PluginUtilities.getCallbackHandle (flutter/engine#6260)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 19, 2018
flutter/engine@2e8e96f...7ec1d21

git log 2e8e96f..7ec1d21 --no-merges --oneline
7ec1d21 Compute maxIntrinsicWidth based on the width of styled runs added to the line breaker (flutter/engine#6281)
2b25fd7 Don't use unix or win namespaces (flutter/engine#6277)
60c5bb8 Return null instead of throwing if an instance method is passed to PluginUtilities.getCallbackHandle (flutter/engine#6260)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 20, 2018
flutter/engine@2e8e96f...f3d51b0

git log 2e8e96f..f3d51b0 --no-merges --oneline
f3d51b0 Fix Top, Left, and Right padding for fullscreen android apps. (flutter/engine#6282)
7ec1d21 Compute maxIntrinsicWidth based on the width of styled runs added to the line breaker (flutter/engine#6281)
2b25fd7 Don't use unix or win namespaces (flutter/engine#6277)
60c5bb8 Return null instead of throwing if an instance method is passed to PluginUtilities.getCallbackHandle (flutter/engine#6260)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 20, 2018
flutter/engine@2e8e96f...1501359

git log 2e8e96f..1501359 --no-merges --oneline
1501359 Roll src/third_party/skia 1d6281d4bb47..9369031fde2d (27 commits) (flutter/engine#6284)
f3d51b0 Fix Top, Left, and Right padding for fullscreen android apps. (flutter/engine#6282)
7ec1d21 Compute maxIntrinsicWidth based on the width of styled runs added to the line breaker (flutter/engine#6281)
2b25fd7 Don't use unix or win namespaces (flutter/engine#6277)
60c5bb8 Return null instead of throwing if an instance method is passed to PluginUtilities.getCallbackHandle (flutter/engine#6260)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 20, 2018
flutter/engine@2e8e96f...71457a6

git log 2e8e96f..71457a6 --no-merges --oneline
71457a6 Roll dart to c688d0c0c3ad3dece3a79ce0e115d787a94707ea. (flutter/engine#6285)
1501359 Roll src/third_party/skia 1d6281d4bb47..9369031fde2d (27 commits) (flutter/engine#6284)
f3d51b0 Fix Top, Left, and Right padding for fullscreen android apps. (flutter/engine#6282)
7ec1d21 Compute maxIntrinsicWidth based on the width of styled runs added to the line breaker (flutter/engine#6281)
2b25fd7 Don't use unix or win namespaces (flutter/engine#6277)
60c5bb8 Return null instead of throwing if an instance method is passed to PluginUtilities.getCallbackHandle (flutter/engine#6260)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 20, 2018
flutter/engine@2e8e96f...2302bea

git log 2e8e96f..2302bea --no-merges --oneline
2302bea Roll src/third_party/skia 9369031fde2d..c9873a5f8b8e (11 commits) (flutter/engine#6287)
71457a6 Roll dart to c688d0c0c3ad3dece3a79ce0e115d787a94707ea. (flutter/engine#6285)
1501359 Roll src/third_party/skia 1d6281d4bb47..9369031fde2d (27 commits) (flutter/engine#6284)
f3d51b0 Fix Top, Left, and Right padding for fullscreen android apps. (flutter/engine#6282)
7ec1d21 Compute maxIntrinsicWidth based on the width of styled runs added to the line breaker (flutter/engine#6281)
2b25fd7 Don't use unix or win namespaces (flutter/engine#6277)
60c5bb8 Return null instead of throwing if an instance method is passed to PluginUtilities.getCallbackHandle (flutter/engine#6260)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.
@mehmetf mehmetf deleted the fix_namespace branch May 19, 2020 23:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants