build failed on OS X v. 10.13.3 (17D102) | Xcode Version 9.2 (9C40b)

[nyotis]

failed

[nyotis]

your build details are crystal-clear, unfortunately it seems I am missing something...

[nyotis]

the problem shows up for both MoltenVK Debug and Release targets.

/Users/nikyotis/Developer/MoltenVK/MoltenVKShaderConverter/MoltenVKSPIRVToMSLConverter/SPIRV-Tools/source/opt/instruction.cpp:513:10: Use of undeclared identifier 'spvInstructionBinaryToText'

Maybe I should just add the header? namespace prepending does not really help...

[nyotis]

Modifying the signature of /var/instruction.h && /var/instruction.cpp so that PrettyPrint returns void does not help either...

[nyotis]

[karl-lunarg]

This builds fine for me. I have the same version of Xcode as you. Also, the Travis-CI build is OK.

I do note that my version of Instruction.cpp has only 451 lines in it. And you are getting an error at line 513.

Are you sure you are using the latest sources?

If you do a git pull on the MoltenVK repo, you may need to do git submodule update in order to freshen the submodule content. The External/makeAll script does this submodule update for you. You may want to run this script again to get all the submodules updated and rebuilt.

Some of the other error messages in your screenshots also suggest that this could be the problem.

[nyotis]

Thanks @karl-lunarg. I am gonna try fresh once more.

[irobertmanley]

Still have the problem here on clean clone + makeAll or after getLatestAll.

[nyotis]

Problem persists here as well.. I will fork and have a look at the bug. NG

…

On Fri, Mar 9, 2018 at 2:52 PM, irobertmanley ***@***.***> wrote: Still have the problem here on clean clone + makeAll or after getLatestAll. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#87 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAoNH6n2t46-_e7jRRONHm7T9dpQ3SOPks5tcokLgaJpZM4ShGeu> .

-- Nikolaos Giotis

[nyotis]

[irobertmanley]

There are completely separate versions of instruction.cpp - one in val and one in opt. One version doesn't have it, the other does.

[nyotis]

Thanks Robert

…

On Fri, Mar 9, 2018 at 3:45 PM, irobertmanley ***@***.***> wrote: There are completely separate versions of instruction.cpp - one in var and one in opt. One version doesn't have it, the other does. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#87 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAoNHwhF6iv8JpUDBkkXsHBjURJSEVePks5tcpV1gaJpZM4ShGeu> .

-- Nikolaos Giotis

[nyotis]

What I do not understand is how Travis Continues Integration did not flag this at first place.. Maybe some Tests are missing?

…

On Fri, Mar 9, 2018 at 3:46 PM, Nikolaos Giotis ***@***.***> wrote: Thanks Robert On Fri, Mar 9, 2018 at 3:45 PM, irobertmanley ***@***.***> wrote: > There are completely separate versions of instruction.cpp - one in var > and one in opt. One version doesn't have it, the other does. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#87 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAoNHwhF6iv8JpUDBkkXsHBjURJSEVePks5tcpV1gaJpZM4ShGeu> > . > -- Nikolaos Giotis

-- Nikolaos Giotis

[irobertmanley]

Perhaps they are not using High Sierra and APFS? Still finding minor filesystem differences here that may affect the scripts.

[nyotis]

Sure. At least we test the source for High Sierra..

…

On Fri, Mar 9, 2018 at 3:50 PM, irobertmanley ***@***.***> wrote: Perhaps they are not using High Sierra and APFS? Still finding minor filesystem differences here that may affect the scripts. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#87 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAoNH_H8a9AC5Aj3yF9BTO8dcaxPUe5Nks5tcpbHgaJpZM4ShGeu> .

-- Nikolaos Giotis

[irobertmanley]

Worth noting that I only get this error after getLatestAll. There is a different error on clean makeAll regarding a missing generated include file.

[nyotis]

thanks for the heads-up

[karl-lunarg]

@irobertmanley You shouldn't really use getLatestAll. getLatestAll is for the case where a MoltenVK developer wants to update submodules to their latest versions. Often, a change in a submodule may require some adjustment in the rest of MoltenVK to cope with the change. For regular building, you really need to leave the submodule revisions alone and use makeAll (which should sync the submodule trees to the correct version).

As for the missing generated include file, see issue #84. I'll update that issue with a workaround.

[irobertmanley]

makeAll based build now working with that update.

[nyotis]

Thanks Karl! Nikolaos Giotis

…

On Mar 9, 2018 16:49, "Karl Schultz" ***@***.***> wrote: @irobertmanley <https://github.com/irobertmanley> You shouldn't really use getLatestAll. getLatestAll is for the case where a MoltenVK developer wants to update submodules to their latest versions. Often, a change in a submodule may require some adjustment in the rest of MoltenVK to cope with the change. For regular building, you really need to leave the submodule revisions alone and use makeAll (which should sync the submodule trees to the correct version). As for the missing generated include file, see issue #84 <#84>. I'll update that issue with a workaround. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#87 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAoNH9FPPYY1yQxRTc8kIFcKW9Q9s0lEks5tcqSbgaJpZM4ShGeu> .

[karl-lunarg]

@nyotis If you are building fine now, it would help @billhollings if you could close this issue. I think that #84 sufficiently tracks the other problem.

[nyotis]

both Debug and Release build now fine

[billhollings]

PR #98 changes the way that external libraries are fetched and built, and should fix this issue. Among other significant changes, the getLatestAll script has been removed.

As @karl-lunarg indicates...the getLatestAll script was unintentionally dangerous. It was only intended to be used by MoltenVK devs who are deliberately moving MoltenVK to use another version of an external library. It was not intended to be used as a mechanism for app devs to use different versions of the external libraries...as doing so causes issues like this.