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

MSVC build broken (due to latest changes) #187

Closed
mgreter opened this issue Sep 6, 2016 · 11 comments
Closed

MSVC build broken (due to latest changes) #187

mgreter opened this issue Sep 6, 2016 · 11 comments

Comments

@mgreter
Copy link
Contributor

mgreter commented Sep 6, 2016

Getting errors via AppVeyor: https://ci.appveyor.com/project/sass/libsass/build/job/nmh2dbt1fcrjyxg3

Also reproducible locally:

D:\github\libsass\sassc\win>msbuild sassc.sln
Microsoft (R) Build Engine version 14.0.25420.1
Copyright (C) Microsoft Corporation. All rights reserved.

Building the projects in this solution one at a time. To enable parallel build, please add the "/m" switch.
Build started 06.09.2016 16:39:55.
Project "D:\github\libsass\sassc\win\sassc.sln" on node 1 (default targets).
ValidateSolutionConfiguration:
  Building solution configuration "Debug|Win32".
Project "D:\github\libsass\sassc\win\sassc.sln" (1) is building "D:\github\libsass\sassc\win\sassc.vcxproj" (2) on node 1 (default targets).
D:\github\libsass\sassc\win\sassc.vcxproj(169,3): error MSB4019: The imported project "D:\github\libsass\libsass\win\libsass.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk.
Done Building Project "D:\github\libsass\sassc\win\sassc.vcxproj" (default targ
ets) -- FAILED.

Done Building Project "D:\github\libsass\sassc\win\sassc.sln" (default targets)
 -- FAILED.


Build FAILED.

"D:\github\libsass\sassc\win\sassc.sln" (default target) (1) ->
"D:\github\libsass\sassc\win\sassc.vcxproj" (default target) (2) ->
  D:\github\libsass\sassc\win\sassc.vcxproj(169,3): error MSB4019: The imported project "D:\github\libsass\libsass\win\libsass.targets" was not found. Confirm that the path in the <Import> declaration is correct, and that the file exists on disk.

    0 Warning(s)
    1 Error(s)

Suspecting changes to LIBSASS_DIR variable in #184 (//CC @nschonni)

@mgreter
Copy link
Contributor Author

mgreter commented Sep 6, 2016

Reverting the change to LIBSASS_DIR at least fixes the issue locally:
<LIBSASS_DIR>..\..</LIBSASS_DIR>

Note that (all?) build instructions want you to clone sassc as a subdirectory of libsass. One reason to do it that way in CI is that we don't know if we have write permissions beside our main directory (i.e. when building libsass). Or was there any other reason why the variable was changed? Might also be worth noting that msbuild will be inside the "win" directory when executed.

@nschonni
Copy link
Contributor

nschonni commented Sep 6, 2016

Sorry about that, i think the best solution to this would be

  <LIBSASS_DIR Condition=" '$(SASS_LIBSASS_PATH)'  == '' ">..\..\libsass</LIBSASS_DIR>

That way the libsass build can use it's own directory, but still keep this as an out of source/peer folder for SassC that the bootstrap creates. I'm going to look at setting up Appveyor here too.

@mgreter
Copy link
Contributor Author

mgreter commented Sep 6, 2016

I'm no MSVC expert, but I was under the impression that you could already overwrite the properties via
msbuild sassc.sln /P:LIBSASS_DIR=..\..

A short tests indicates that it works, albeit yielding a different/later error with current master*.
//CC @am11: you probably know this area best. Any suggestions?
*My MSVC env seems to be broken on my desktop machine ...

@mgreter
Copy link
Contributor Author

mgreter commented Sep 6, 2016

Found the right shell to compile it successfully with the command from above:
msbuild sassc.sln /P:LIBSASS_DIR=..\..
Therefore I would suggest to revert the change to keep the previous behavior and advice to use /P:LIBSASS_DIR=. for out of tree builds. I'm open for reasons to change it, but it would need according updates to the documentation and the CI build.

@mgreter
Copy link
Contributor Author

mgreter commented Sep 6, 2016

On a side note, if you want to try to add appveyor you may want to fork both projects to your local github and enable them in your own appveyor. Then update the build script to checkout sassc from your own repo. I always used this setup as you can easily trigger 100s of build without disturbing anyone.

@mgreter
Copy link
Contributor Author

mgreter commented Sep 6, 2016

Opened PR to revert the default value. @nschonni: I hope you can work with the option I used above? On another note the regular unix makefiles also use the same default AFAICS.

@nschonni
Copy link
Contributor

nschonni commented Sep 6, 2016

I'll try the environment variable approah when I get home. If that doesn't work, I'll land the revert.
I had updated the Linux instructions with #183, but I missed that part of the Makefile.

@am11
Copy link
Contributor

am11 commented Sep 6, 2016

Sorry couldn't reply earlier.
#188 fix looks good. Thanks!
Would be nice to have AppVeyor setup for this repo. I can prepare a PR if the feelings are mutual. 😄

@xzyfer
Copy link
Contributor

xzyfer commented Sep 7, 2016

@am11 we would love this. I believe @nschonni has been working towards to this goal. It'd be worth collaborating in Slack :)

@nschonni
Copy link
Contributor

nschonni commented Sep 7, 2016

@am11 feel free to do the Appveyor stuff if you have time

@am11
Copy link
Contributor

am11 commented Sep 7, 2016

Thanks. Added the CI thingy here: #189. 😹

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

No branches or pull requests

4 participants