Skip to content
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.

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

Appveyor #852

Merged
merged 1 commit into from
Feb 5, 2018
Merged

Appveyor #852

merged 1 commit into from
Feb 5, 2018

Conversation

marsupial
Copy link
Contributor

Adjusted to only save dependency cache after first build /changes to appveyor.yml
CI shows an error in such cases, but next run will build OSL & test it.

There is a possibility that enough time exists to build up the dependencies and then build OSL without testing, but went conservative.

@marsupial
Copy link
Contributor Author

So this fails as it's a change to appveyor.yml, but success on #853 verifies the cache was successfully uploaded and working!

appveyor.yml Outdated
@@ -102,18 +108,29 @@ before_build:
cmake ..\.. -G "$GENERATOR" -DCMAKE_CONFIGURATION_TYPES="$env:CONFIGURATION" -DCMAKE_PREFIX_PATH="$DEP_DIR" -DCMAKE_INSTALL_PREFIX="$DEP_DIR"
cmake --build . --config $env:CONFIGURATION --target INSTALL

# IlmBase / OpenEXR
#cd $DWN_DIR
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is all commented out, it can get deleted.

@lgritz
Copy link
Collaborator

lgritz commented Feb 2, 2018

This seems to incorporate a bunch of things I was experimenting with in my own tree... which are half-baked. Are you sure you want that?

@marsupial
Copy link
Contributor Author

No, just based it off that branch assuming they were fully-baked.
Can chop the first to off if you like.

@lgritz
Copy link
Collaborator

lgritz commented Feb 2, 2018

No, the "lgritz" repo is just my personal workspace. If I haven't submitted a PR, it's at best half-baked, and could very well a failed experiment that I abandoned but forgot to delete the branch. :-)

But I do like your idea of a two-stage build, where at least it will save the built dependencies even if it doesn't have time to complete the OSL build and test, so that the next time it runs it will be much faster.

I'm confused, though -- if it uses the cache to avoid downloading and building the dependencies, what happens when the dependencies themselves change? Is it expected that we should alter appveyor.yml in that case, which will flush the cache?

Of course, this would all be so much better if we could get pre-built OpenEXR and LLVM rather than having to build them from scratch. Is there really not a stash of prebuilt llvm that also has the llvm-config that we need?

@lgritz
Copy link
Collaborator

lgritz commented Feb 2, 2018

By the way, what I was doing in that branch was experimenting with ways to improve the appveyor use (in particular, speed it up to make it more reliably run in the 1 hour allotment).

The snippet that flushes the cache if appveyor.yml changes is good.

Also, the addition of -DCMAKE_CXX_FLAGS="/w /EHsc" to the Ilmbase and OpenEXR builds is helpful to get rid of the tens of thousands of warnings that were cluttering up the appveyor output. I was finding that looking at the appveyor logs was very tedious with all those warnings.

The rest is dubious, just things I was trying out.

@marsupial
Copy link
Contributor Author

Ok, I'll pick out those two parts (appveyor.yml and CMake flags) and update.

Also not really addressed, should OIIO be a cached dependency or is CI against OIIO-master more beneficial?

Down the line Travis & Appveyor from OIIO could be publishing the binaries of releases and then OSL just downloads those instead of building.

@lgritz
Copy link
Collaborator

lgritz commented Feb 2, 2018

I definitely want to build against OIIO master so that if something on the OIIO side changes in such a way that it breaks OSL (but wasn't revealed in the OIIO tests, or that we didn't realize had the implication of breaking OSL), we want to know immediately so that we can fix it.

Always build OIIO from master instead of caching it.
Limit MSBuild output to interesting notes only.
@marsupial
Copy link
Contributor Author

Updated to build against OIIO.master & invalidate cache when appveyor.yml changes.

Still builds OIIO & OSL when the dependency cache is rebuilt to catch any source errors that may have occurred from the dependency changes.

In that case though, the OSL test suite is skipped in favor of getting the cache uploaded.

@lgritz
Copy link
Collaborator

lgritz commented Feb 5, 2018

LGTM, merging.

@lgritz lgritz merged commit e410f16 into AcademySoftwareFoundation:master Feb 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants