Skip to content

[msbuild] Added logging for all of the overridden MSBuild default tasks #1829

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.

Sign up for GitHub

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

Jump to bottom
Merged
jstedfast merged 1 commit into from
Mar 7, 2017
Merged

[msbuild] Added logging for all of the overridden MSBuild default tasks #1829

jstedfast merged 1 commit into from
Mar 7, 2017

Conversation

@jstedfast
Copy link
Member

@jstedfast jstedfast commented Mar 6, 2017

No description provided.

spouliot
spouliot reviewed Mar 6, 2017
View reviewed changes
Copy link
Contributor

@spouliot spouliot left a comment

Choose a reason for hiding this comment

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

I though the non-Base types should be left empty so XVS could subclass them while keeping feature parity ?

Does not this makes the logging Mac/XS specific ?

@jstedfast
Copy link
Member Author

jstedfast commented Mar 6, 2017

It might, I'm not entirely sure which classes they subclass (e.g. do they subclass Base or the one I added the logging to?)

Either way, I think this is useful. It really helped me debug some stuff for the last PR.

@monojenkins
Copy link
Collaborator

monojenkins commented Mar 6, 2017

Build failure

@jstedfast
Copy link
Member Author

jstedfast commented Mar 6, 2017

build failure is unrelated to these changes

@spouliot
Copy link
Contributor

spouliot commented Mar 7, 2017

@jstedfast no doubt it's useful :) but it would be too for builds coming from Windows, so maybe it should be in the Base type ? @kzu @josegallardo thoughts ?

@jstedfast
Copy link
Member Author

jstedfast commented Mar 7, 2017

@spouliot ah, yes, understood. I'll wait for @kzu and/or @josegallardo to comment before merging this in case it'd be better to put the logging on the base class.

@kzu
Copy link
Contributor

kzu commented Mar 7, 2017

I don't see the point of this. A diagnostic build will render all task names and inputs and outputs already....

@jstedfast
Copy link
Member Author

jstedfast commented Mar 7, 2017

It doesn't for xbuild on Mac... all you get is the name of the task, but none of the property values. Perhaps once we switch to MSBuild that won't be a problem (not sure as I don't recall what the MSBuild logs look like)

@kzu
Copy link
Contributor

kzu commented Mar 7, 2017

I would most certainly not add code that when switching to MSBuild requires some form of disabling to avoid duplicating that info all over the place.

And introducing new code to support a soon-to-be-legacy build system seems odd.

@jstedfast
Copy link
Member Author

jstedfast commented Mar 7, 2017

Sounds like the Windows-side already logs the task properties but xbuild doesn't.

I'm thinking it's still worth merging for xbuild side of things even if it means "extra verbose" when diagnostic logging is enabled and the user has enabled experimental msbuild support.

Once we make the switch to msbuild, it sounds like we could remove all of our existing task property logging.

@kzu
Copy link
Contributor

kzu commented Mar 7, 2017 via email •
edited
Loading

@jstedfast
Copy link
Member Author

jstedfast commented Mar 7, 2017

Because that would require hacking xbuild which I don't want to do :)

@kzu
Copy link
Contributor

kzu commented Mar 7, 2017

I'm sure @radical can add that in no time :p

@jstedfast
Copy link
Member Author

jstedfast commented Mar 7, 2017

We could ask, but by the time he implements it, we'll have already switched to msbuild.

@jstedfast jstedfast merged commit 15fa18e into master Mar 7, 2017
@jstedfast jstedfast deleted the msbuild-logging-improvements branch March 7, 2017 22:40
jstedfast added a commit that referenced this pull request Mar 17, 2017
@jstedfast
[msbuild] Added logging for all of the overridden MSBuild default tas…
694bd21 
…ks (#1829)
spouliot added a commit that referenced this pull request Mar 29, 2017
@spouliot
Merge pull request #1872 from xamarin/mono-2017-02
743c28f 
* Bump to mono master

* [build] Profiles cleanup

* Remove 4.x profile dependency

* Replace mdbrebase tool with pathmap and fix broken debug symbols mapping in public builds

* Bump mono and linker submodule

* Add netstandard to the build

* [build] Register linker for submodule checks

* Bump mono

* [build] Add .NET Core open source key

* [build] Update debug symbols to pdb

* [msbuild] Remove smcs compiler overrides

* Update after another Cecil breaking change

* Fixes mmp execution during build to use correct Mono.Cecil. For some reason setting MONO_PATH does not work

* [linker] Update list of Facade assemblies

*  [mtouch] Copy pdb debug symbols when available

* [msbuild] Update test to reflect Mono.Dynamic.Interpreter.dll removal

* [tuner] Update System.Core rule

* [msbuild] Update few test expected debug symbols

* Fixes missing Mono.Security dependency

* Updates mtouch.csproj

* Switch to Mono 2017-02 branch

* Adds pdb handling to mtouch

* Bump linker

* Wire up configuration to MonoTouchSweepStep

* Mono branch name update

* Remove mdbrebase which is no longer needed (compiler does it now)

* Update monomac tuner

* Revert "[linker] Inline BinaryCompatibility.TargetsAtLeast_Desktop_V4_5[_1] (#1518)"

This reverts commit e72b654.

because it's no longer needed with latest mono

* Bump maccore for mdoc path fix

* Add missing reference

* Bump linker

* Updates mmp.csproj files path

* [tests][mac] Terminate test processes with extreme prejudice. Fixes #52604.

https://bugzilla.xamarin.com/show_bug.cgi?id=52604

* Bump Xamarin Studio to get fix for bug #52648.

Bump Xamarin Studio to get fix for bug #52648 causing build failures.

* Update system Mono to 2017-02

* More mdb to pdb checks update

* Update tests to work with csc compiler

* Another system mono bump

* [tests] Fix mlaunch path, it's no longer shipped with Xamarin Studio. (#1842)

* Fixes tuner IL optimization for get_Size to work with short branches

* Update IntPtrSizeOptimization test to consider Debug mode IL output

* Xamarin.Mac.FSharp.targets - Fix imports to work correctly with ..

.. MSBuildExtensionsPath* .

`MSBuildExtensionsPath*` supports search paths only if the string
`$(MSBuildExtensionsPath)` (or *32/*64) appear in the `Project`
attribute of the `Import` element. Otherwise the value of the property
or the default value is used. So, restructure to conditionally import
rather than a import using a property with the final path.

* [msbuild] Index into the correct item array when printing an error message (#1792)

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=52860

When the ImageAssets contain an item that doesn't live within
a *.xcassets directory, index into ImageAssets[] rather than
items[] which hasn't been populated yet.

Also fixed the tagsList for-loop to use tagsList.Count instead
for correctness (even though tags.Count and tagsList.Count
should always be identical).

* [msbuild] Ignore .DS_Store files when cloning asset catalogs (#1776)

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=52851

The problem here is that the .DS_Store file was included in
the .csproj file but did not exist on disk, so when we went
to clone that file into the obj/ dir before running actool
on it, File.Copy() would fail because the file did not
actually exist.

Since these files are worthless anyway, we can safely ignore
them.

Also added logic to verify that files exist before copying
them in order to report a better error than an exception
stack trace.

* [msbuild] Make a build error more clearly worded (#1794)

Related to https://bugzilla.xamarin.com/show_bug.cgi?id=52847

* [msbuild] Remove an unused enum that causes confusion (#1798)

* [msbuild] sanity check TargetiOSDevice property for conflicts (#1799)

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=52847

If the device archs do not match the configuration, don't
do a device-specific build.

* [msbuild] Make sure to codesign appex dylibs (#1812)

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=52745

Added a new CodesignNativeLibraries task that scans for
and then codesigns each *.dylib and *.metallib in the
app bundle (minus those in the PlugIns and Watch dirs).

* [msbuild] Run the _ComputeTargetArchitectures target before cleaning (#1828)

Some of the Clean steps need the _ComputeTargetArchitectures
target to run before they can properly do their thing because
they depend on DeviceSpecific paths.

Also added logic to rm -rf extension *.dSYM and framework *.dSYM
dirs in the container app bin dir (which fails on xbuild due to
a bug in xbuild... yay!)

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=53007

* [msbuild] ALWAYS log ibtool/actool exitCode != 0 as an error (#1840)

Log this as an error even if the log file does not exist and/or
is parseable.

Previously, it was possible that if the log file didn't exist
or it was perfectly validly formatted, we would never log that
error code and so it's possible that we wouldn't log *any*
error at all (e.g. empty log file).

* [msbuild] Added logging for all of the overridden MSBuild default tasks (#1829)

* [msbuild] Fixed PathUtils.AbsoluteToRelative() logic (#1861)

* [msbuild] Fixed PathUtils.AbsoluteToRelative() logic

Don't require the 'absolute' path argument to be a FullPath.

The code already calls Path.GetFullPath() on the 'absolute'
path anyway.

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=53176

* Fixed ResolveSymbolicLinks() to return the input path if realpath() fails

* [msbuild] Use stamp files to force container app's _CompileToNative (#1878)

Fixes https://bugzilla.xamarin.com/show_bug.cgi?id=53303

* Bump mono

* Another mono bump

* Mono bump

* Update .gitignore with pdb files

* Bump mono

* Mono bump

* Revert "Bump maccore for mdoc path fix"

This reverts commit 50f4564.

* Bump maccore

* Bump mono

* Remove TlsProviderFactoryCallback

* Update linker tests after appletls merge

* Mono bump

* Bump mono to include fix for bug #53481

* Bump mono

* Bump system mono to get fix for #53481.

https://bugzilla.xamarin.com/show_bug.cgi?id=53481

* Bump mono

* Bump mono

* Remove Mono.Dynamic.Interpreter from mtouch tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rolfbjarne rolfbjarne rolfbjarne approved these changes

+2 more reviewers

@spouliot spouliot spouliot left review comments

@VincentDondain VincentDondain VincentDondain approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@jstedfast @monojenkins @spouliot @kzu @rolfbjarne @VincentDondain @dnfclas