PHPC-1121: Added Appveyor config / PHPC-1114: Add template.rc file #764

derickr · 2018-02-21T12:07:52Z

https://jira.mongodb.org/browse/PHPC-1121

jmikola · 2018-02-23T15:51:18Z

.appveyor.yml

+
+cache:
+  - c:\build-cache -> .appveyor.yml
+  - c:\build-cache\sdk -> .appveyor.yml


Isn't this redundant if c:\build-cache will already be invalidated when .appveyor.yml changes?

Yes, I think so. But it was in the original example that Anatol had for me.

jmikola · 2018-02-23T15:52:47Z

.appveyor.yml

@@ -0,0 +1,82 @@
+image: Visual Studio 2015
+version: '{branch}.{build}'


What is this used for? Does {build} increment with each invocation of Appveyor?

Yes — it creates the "build name" on the right hand side at: https://ci.appveyor.com/project/derickr/mongo-php-driver/history

jmikola · 2018-02-23T15:55:58Z

.appveyor.yml

+      ARCHITECTURE: x86
+      ZTS_STATE: disable
+      APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2015
+      PHP_BUILD_CRT: vc14


Appveyor has a VS 2013 build environment. Is there any reason we can't use that for PHP 5.5 and 5.6?

No real reason, except that we'd be creating binaries for outdated PHP versions, and building binaries takes a looong time on AppVeyor already.

I think it's worth adding that if we're to rely on this instead of manually creating test builds for PHP 5.5 and 5.6 before releasing.

If build times are very long, I'd also be amenable to disabling Appveyor for PRs and simply having it monitor branches on the mongodb repo.

I tried that, but no luck:
https://ci.appveyor.com/project/derickr/mongo-php-driver/build/master.18/job/yc9jeqbrawjkuh97#L51

The new SDK only supports 2015 and 2017: https://github.com/Microsoft/php-sdk-binary-tools#requirements

jmikola · 2018-02-23T16:00:13Z

.appveyor.yml

+version: '{branch}.{build}'
+
+cache:
+  - c:\build-cache -> .appveyor.yml


Given that you've offloaded most of the work to scripts in .appveyor/, shouldn't you also invalidate if any of those change, too? It looks like Appveyor would support this, per Multiple build cache dependencies.

- c:\build-cache -> .appveyor.yml, appveyor\*.cmd

I'm thinking ahead, but this might avoid needing to manually invalidate the cache.

I've never had the cache create a problem for me with Xdebug, so didn't really think much about this.

jmikola · 2018-02-23T16:01:37Z

.appveyor/build_task.cmd

+
+	if %errorlevel% neq 0 exit /b 3
+
+	cmd /c configure.bat --disable-all --with-mp=auto --enable-cli --%ZTS_STATE%-zts --enable-json --with-openssl --enable-mongodb=shared --enable-object-out-dir=%PHP_BUILD_OBJ_DIR% --with-config-file-scan-dir=%APPVEYOR_BUILD_FOLDER%\build\modules.d --with-prefix=%APPVEYOR_BUILD_FOLDER%\build --with-php-build=%DEPS_DIR%


Other than the "mongodb" replacements here, was this basically copied from another project?

Second question: are you only compiling the DLL, or do you still have to build php.exe each time as well?

Yes, copied from Xdebug, which was copied from something else.

We're still building php.exe too. I think that's fine for now, as "it works", and messing with this just takes too long for it to be worthwhile.

Ok. IIRC, @weltling was talking about in #php.pecl:

<weltling> Derick, btw. perhaps you could check the phpize based approach https://github.com/krakjoe/apcu/blob/master/appveyor.yml <weltling> you can spare some time per single job by not building all the php, still need to fetch deps in your case <weltling> see the build_script: section in the yml file <weltling> for the tests - also of course php is needed, but in that case it fetches the precompiled version, too <weltling> so except the extension itself, in most case nothing needs to be compiled <weltling> with some exts i was helping out, the build time dropped reasonably <weltling> here it fetches https://github.com/krakjoe/apcu/blob/master/appveyor.yml#L99 <weltling> basically, there it fetches the dev package, generates a job file and runs it, all packed into yml <weltling> if there are no external deps, it's a way easier and faster approach, with deps might need some more lines

If you don't want to handle this in this PR, I would suggest creating a PHPC ticket so we can track this for later. Feel free to copy/paste that chat log into the ticket for future reference, too.

jmikola · 2018-02-23T16:04:24Z

phongo_version.h

+
+#define PHP_MONGODB_VERSION "1.5.0-dev"
+#define PHP_MONGODB_STABILITY "devel"
+#define PHP_MONGODB_VERSION_DESC 1,5,0,0


Instructions for bumping the version in CONTRIBUTING.md should be updated in the same commit (assuming that still refers to php_phongo.h).

OK, will fix that (thought I had).

jmikola · 2018-02-23T16:06:54Z

template.rc

+            VALUE "FileDescription", FILE_DESCRIPTION "\0"
+            VALUE "FileVersion", PHP_MONGODB_VERSION
+            VALUE "InternalName", FILE_NAME "\0"
+            VALUE "LegalCopyright", "Copyright � 2014-2018 MongoDB, Inc.\0"


Why do I see a Unicode replacement character here?

I'd also suggest using "present" as the upper bound, per DRIVERS-329. This could be rewritten as:

Copyright 2014-present MongoDB, Inc.

The file encoding is win-1252 (see line 12), I guess GitHub doesn't get that.

Ok. I think we should still use "present" as the date's upper bound, though. That will ensure this never goes out of date.

jmikola · 2018-02-23T16:10:21Z

template.rc

+            VALUE "Comments", THANKS_GUYS "\0"
+            VALUE "CompanyName", "MongoDB, Inc.\0"
+            VALUE "FileDescription", FILE_DESCRIPTION "\0"
+            VALUE "FileVersion", PHP_MONGODB_VERSION


Is this being pulled directly from phongo_version.h? If so, both it and THANKS_GUYS are declared with #define. Why do you add a "\0" to THANKS_GUYS but not here?

Don't know. I copied this from Xdebug where it worked for the past 10 years :-)

jmikola · 2018-02-23T16:11:57Z

.appveyor.yml

+  - .appveyor\install.cmd
+
+build_script:
+  - .appveyor\build.cmd


This currently runs now tests and is only verifying that we can build the DLL, correct?

It does run tests that don't need a mongod server, but right now I'm not making the build fail due to failing tests. We need to follow up on that.

jmikola · 2018-02-23T16:15:12Z

.appveyor/install.cmd

+	if not exist "%PHP_BUILD_CACHE_SDK_DIR%" (
+		echo Cloning remote SDK repository
+		rem git clone -q --depth=1 --branch %SDK_BRANCH% %SDK_REMOTE% "%PHP_BUILD_CACHE_SDK_DIR%" 2>&1
+		git clone --branch %SDK_BRANCH% %SDK_REMOTE% "%PHP_BUILD_CACHE_SDK_DIR%" 2>&1


Why does the commented command above have -q --depth=1. You use those arguments for cloning PHP below, so why not for the SDK as well?

Snippet I got from Anatol. I guess the rem line can go.

I'd suggest adding -q --depth=1 to this git clone, though. I don't see any reason you need the full commit history, and -q will make the output a bit quieter. Since progress won't be reported to stderr, we can probably remove the redirection with 2>&1 as well.

jmikola · 2018-02-23T16:16:24Z

.appveyor/install.cmd

+	)
+
+	if "%PHP_REL%"=="master" (
+		echo git clone -q --depth=1 --branch=%PHP_REL% https://github.com/php/php-src C:\projects\php-src


Just pointing out that these echo lines are inconsistent with those for the SDK repository. Would it be preferable to write "Cloning remote PHP repository (%PHP_REL%)"?

Maybe. I don't care enough to go through another hours long iteration for this.

jmikola · 2018-02-23T16:16:55Z

.appveyor/install.cmd

+	) else (
+		echo git clone -q --depth=1 --branch=PHP-%PHP_REL% https://github.com/php/php-src C:\projects\php-src
+		git clone -q --depth=1 --branch=PHP-%PHP_REL% https://github.com/php/php-src C:\projects\php-src
+	)


Any reason we can't cache the php-src repository as well?

The cache is limited in space, you can't just chuck everything in it.

Noted. I was expecting this wouldn't be massive, since you're only using a depth of 1.

In any event, I suppose we can revisit this when optimizing later to not recompile php.exe each time.

jmikola · 2018-02-23T18:04:00Z

.appveyor/install.cmd

+	) else (
+		echo git clone -q --depth=1 --branch=PHP-%PHP_REL% https://github.com/php/php-src C:\projects\php-src
+		git clone -q --depth=1 --branch=PHP-%PHP_REL% https://github.com/php/php-src C:\projects\php-src
+	)


Noted. I was expecting this wouldn't be massive, since you're only using a depth of 1.

In any event, I suppose we can revisit this when optimizing later to not recompile php.exe each time.

jmikola · 2018-02-23T18:05:50Z

.appveyor/install.cmd

+	if not exist "%PHP_BUILD_CACHE_SDK_DIR%" (
+		echo Cloning remote SDK repository
+		rem git clone -q --depth=1 --branch %SDK_BRANCH% %SDK_REMOTE% "%PHP_BUILD_CACHE_SDK_DIR%" 2>&1
+		git clone --branch %SDK_BRANCH% %SDK_REMOTE% "%PHP_BUILD_CACHE_SDK_DIR%" 2>&1


I'd suggest adding -q --depth=1 to this git clone, though. I don't see any reason you need the full commit history, and -q will make the output a bit quieter. Since progress won't be reported to stderr, we can probably remove the redirection with 2>&1 as well.

jmikola · 2018-02-23T18:09:20Z

CONTRIBUTING.md

 ```

 The above would be changed to:

 ```
 #define PHP_MONGODB_VERSION "1.1.9-dev"
 #define PHP_MONGODB_STABILITY "devel"
+#define PHP_MONGODB_VERSION_DESC 1,1,9,0


How do we increment this if going from 1.4.0-beta1 back to 1.4.0-dev and later to 1.4.0-rc1? I assume the value might transition from the following states:

1,4,0,0 (original 1.4.0-dev)

1,4,0,1 (first beta)

1,4,0,2 (reverted back to dev)

1,4,0,3 (first RC)

If so, can we summarize this in the "Note: If this is an alpha, beta, or RC release..." paragraph below?

jmikola · 2018-02-23T18:10:14Z

template.rc

+            VALUE "FileDescription", FILE_DESCRIPTION "\0"
+            VALUE "FileVersion", PHP_MONGODB_VERSION
+            VALUE "InternalName", FILE_NAME "\0"
+            VALUE "LegalCopyright", "Copyright � 2014-2018 MongoDB, Inc.\0"


Ok. I think we should still use "present" as the date's upper bound, though. That will ensure this never goes out of date.

jmikola · 2018-02-23T18:13:34Z

.appveyor/build_task.cmd

+
+	if %errorlevel% neq 0 exit /b 3
+
+	mkdir c:\tests_tmp


I missed that the test suite was being executed here.

I think this really belongs in its own test_script.cmd file, which can be run from the test_script phase in appveyor.yml. https://www.appveyor.com/docs/appveyor-yml/ has an example of this:

# to run your custom scripts instead of automatic tests test_script: - echo This is my custom test script

If this poses problems with making the build fail (as you mentioned in #764 (comment)), which can't be resolved by Allow Failing Jobs, then we can leave this as-is. Please make a PHPC ticket to revisit this later, though.

The problem was more that in order to run the tests in a separate script, you'd have to store the output of the build tasks, and basically restore things for every task. Not worth the effort, as this works just fine™. I wouldn't even want to add a PHPC ticket for this.

jmikola · 2018-02-23T18:16:07Z

.appveyor/install.cmd

+
+	if not exist "%PHP_BUILD_CACHE_SDK_DIR%" (
+		echo Cloning remote SDK repository
+		git clone --branch %SDK_BRANCH% %SDK_REMOTE% "%PHP_BUILD_CACHE_SDK_DIR%" 2>&1


I'd suggest adding -q --depth=1 to this git clone, though. I don't see any reason you need the full commit history, and -q will make the output a bit quieter. Since progress won't be reported to stderr, we can probably remove the redirection with 2>&1 as well.

jmikola

I created PHPC-1131 to track the outstanding task of refactoring to avoid full php.exe compilation.

LGTM to copyright date changes below.

jmikola · 2018-02-26T16:34:34Z

phongo_version.h

@@ -0,0 +1,24 @@
+/*
+ * Copyright 2014-2018 MongoDB, Inc.


Since this file was just created, the date should start at 2018. This should read "2018-present" per DRIVERS-329.

jmikola · 2018-02-26T16:35:13Z

template.rc

+            VALUE "FileDescription", FILE_DESCRIPTION "\0"
+            VALUE "FileVersion", PHP_MONGODB_VERSION "\0"
+            VALUE "InternalName", FILE_NAME "\0"
+            VALUE "LegalCopyright", "Copyright � 2014-2018 MongoDB, Inc.\0"


This should read "2018-present" per DRIVERS-329.

My last comment on this appears to have been hidden by recent diffs.

… the Windows binary

These are not present in PHP's own template.rc file and only exist here because they were copied from Xdebug (see: mongodb#764 (comment)).

derickr force-pushed the PHPC-1121-appveyor branch 12 times, most recently from 530c7c5 to ebd6b0e Compare February 23, 2018 12:02

derickr changed the title ~~[WIP] Added Appveyor config~~ PHPC-1121: Added Appveyor config / PHPC-1114: Add template.rc file Feb 23, 2018

jmikola reviewed Feb 23, 2018

View reviewed changes

derickr force-pushed the PHPC-1121-appveyor branch from e794fae to baeeb39 Compare February 26, 2018 07:44

jmikola approved these changes Feb 26, 2018

View reviewed changes

derickr force-pushed the PHPC-1121-appveyor branch from e893412 to 5852267 Compare February 27, 2018 10:02

derickr added 2 commits February 27, 2018 13:09

PHPC-1121: Added Appveyor config

2b87363

PHPC-1114: Add template.rc file to embed the right MongoDB version in…

a7576aa

… the Windows binary

derickr force-pushed the PHPC-1121-appveyor branch from 5852267 to a7576aa Compare February 27, 2018 13:10

derickr merged commit a7576aa into mongodb:master Feb 27, 2018

derickr added a commit that referenced this pull request Feb 27, 2018

Merged pull request #764

8ebc871

jmikola added a commit to jmikola/mongo-php-driver that referenced this pull request Jul 8, 2020

Remove null bytes from template.rc

6fe08b8

These are not present in PHP's own template.rc file and only exist here because they were copied from Xdebug (see: mongodb#764 (comment)).

		@@ -0,0 +1,82 @@
		image: Visual Studio 2015
		version: '{branch}.{build}'


		if %errorlevel% neq 0 exit /b 3

		cmd /c configure.bat --disable-all --with-mp=auto --enable-cli --%ZTS_STATE%-zts --enable-json --with-openssl --enable-mongodb=shared --enable-object-out-dir=%PHP_BUILD_OBJ_DIR% --with-config-file-scan-dir=%APPVEYOR_BUILD_FOLDER%\build\modules.d --with-prefix=%APPVEYOR_BUILD_FOLDER%\build --with-php-build=%DEPS_DIR%

PHPC-1121: Added Appveyor config / PHPC-1114: Add template.rc file #764

PHPC-1121: Added Appveyor config / PHPC-1114: Add template.rc file #764

Uh oh!

Conversation

derickr commented Feb 21, 2018 • edited by jmikola Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmikola Feb 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

derickr commented Feb 21, 2018 •

edited by jmikola

Loading

jmikola Feb 23, 2018 •

edited

Loading