Skip to content

Add support for relative paths in --projects for eng/common/build.sh - #3910

Merged
vatsan-madhavan merged 1 commit into
masterfrom
dev/vatsan/allow-relative-proj-paths-unix
Jan 31, 2020
Merged

Add support for relative paths in --projects for eng/common/build.sh#3910
vatsan-madhavan merged 1 commit into
masterfrom
dev/vatsan/allow-relative-proj-paths-unix

Conversation

@vatsan-madhavan

Copy link
Copy Markdown
Member

PR in response to @tmat's comment at #3885 (comment).

I don't use OS X or Ubuntu regularly since WPF doesn't build on those ;-) I'm hoping that reviewers would take care to think about this submission very carefully before approving.

In practice, I don't think supplying multiple projects for --projects really works in build.sh.

/cc @chcosta

@chcosta chcosta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I couldn't get this to work because of how bash parses ;s. Do you have a simple repro for how to make this work with multiple projects?

root@dfd18152e6f0:/src/arcade-docker2# ./build.sh -projects Arcade.sln\;Arcade2.sln
Updated projects: /src/arcade-docker2/Arcade.sln;/src/arcade-docker2/Arcade2.sln
MSBUILD : error MSB1006: Property is not valid.
Switch: /src/arcade-docker2/Arcade2.sln

For switch syntax, type "MSBuild -help"
Build failed (exit code '1').
root@dfd18152e6f0:/src/arcade-docker2# ./build.sh -projects 'Arcade.sln;Arcade2.sln'
Updated projects: /src/arcade-docker2/Arcade.sln;/src/arcade-docker2/Arcade2.sln
MSBUILD : error MSB1006: Property is not valid.
Switch: /src/arcade-docker2/Arcade2.sln

For switch syntax, type "MSBuild -help"
Build failed (exit code '1').
root@dfd18152e6f0:/src/arcade-docker2# ./build.sh -projects Arcade.sln;Arcade2.sln
Updated projects: /src/arcade-docker2/Arcade.sln

@vatsan-madhavan

Copy link
Copy Markdown
Member Author

I couldn't get this to work because of how bash parses ;s. Do you have a simple repro for how to make this work with multiple projects?
Updated projects: /src/arcade-docker2/Arcade.sln;/src/arcade-docker2/Arcade2.sln

Could you get it to work prior to this change? I'd noted in my description this:

In practice, I don't think supplying multiple projects for --projects really works in build.sh.

Even prior to this change, I couldn't get multiple projects to build (by supplying full paths). You can try supplying them within double-quotes, but I don't think that will help either.

I was able to test the change with a single project though. --projects ./some/path/to/project.csproj should now work - it didn't seem to work before.

@chcosta

chcosta commented Sep 9, 2019

Copy link
Copy Markdown
Member

Sounds good, I'll file an issue to follow up on this separately. Thanks @vatsan-madhavan

@vatsan-madhavan
vatsan-madhavan requested a review from tmat September 9, 2019 23:05
@JohnTortugo

Copy link
Copy Markdown
Contributor

Ping. Is this still something that we want to merge?

@vatsan-madhavan

Copy link
Copy Markdown
Member Author

why not - merging now :)

@vatsan-madhavan
vatsan-madhavan merged commit 00402f1 into master Jan 31, 2020
@vatsan-madhavan
vatsan-madhavan deleted the dev/vatsan/allow-relative-proj-paths-unix branch January 31, 2020 00:35
@jkoritzinsky

Copy link
Copy Markdown
Member

This breaks using --projects on MacOS. MacOS doesn't support the -f option for readlink.

@jkoritzinsky

Copy link
Copy Markdown
Member

As a result, this is blocking coreclr's arcade uptake.

@ViktorHofer

Copy link
Copy Markdown
Member

and mono/linker. cc @marek-safar.

cc @markwilkie seems we have a testhole here?

@vatsan-madhavan

vatsan-madhavan commented Feb 4, 2020

Copy link
Copy Markdown
Member Author

Let's revert this to get Arcade uptake moving forward and we can look into bringing this back later. 😞

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.

5 participants