Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Fixed relative path handling for pack and publish command #7166

Conversation

dasMulli
Copy link

... when invoked outside the project directory.

This changes the output directory for pack and publish to be expanded to a full path based on the current working directory. (This is probably most useful when invoking the pack command on a solution to generate multiple nupkg files into a single output directory).

The original behaviour was introduced with the move to MSBuild since it treats relative paths as relative to the project being built.

⚠️ This is a breaking change ⚠️

Pros of taking it:

  • Introduce a breaking change only for a major version increment. (fixing this bug would probably have to wait for a 3.0 release)
  • People aware of this issue are either using the commands from a project directory or pass absolute paths (did a quick search on GitHub).
  • This issue has been reported a number of times and seems to be agreed on to be a buggy behaviour in discussions. (#5996, #6700, #4765, #6063)

Cons of taking it:

  • It is a breaking change.

cc @piotrpMSFT @livarcocc @eerhardt @nguerrera (don't really know who to tag)

Fixes #6700, #4765

@nguerrera
Copy link
Contributor

I am supportive of this change. We have gotten a lot of feedback about the current behavior being wrong. I think we should take the break.

@livarcocc
Copy link

I am supportive of this change as well. I just wonder if this is enough or if we should do this in other places as well. Like, in all outputs or maybe all the places where we can specify paths.

@wli3 wli3 changed the base branch from release/2.0.0 to master August 4, 2017 01:14
@wli3
Copy link

wli3 commented Aug 4, 2017

I changed the base to master, release/2.0.0 is stabilizing

@dasMulli
Copy link
Author

dasMulli commented Aug 4, 2017

I just wonder if a change like this should even be made for a minor version increment (2.1). It is breaking, but it really is generally agreed to be a bug.
If so, I think I found additional places where this is needed. I'll rework the PR over the weekend.

@wli3
Copy link

wli3 commented Aug 7, 2017

@dasMulli we cannot decide if this can go to 2.1 yet, since this is a breaking change. This change will need to hold off for a while. Also we also need to make sure this behavior, not relevant to current directory, is the same across the board.

@wli3 wli3 self-requested a review August 7, 2017 20:02
Copy link

@wli3 wli3 left a comment

Choose a reason for hiding this comment

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

Hold off, since it is breaking change

@dasMulli
Copy link
Author

@livarcocc @wli3 can we close this against a single issue (and close the other ones as duplicate)? this one would need an overhaul anyway to check for any new possibly project relative paths.
If a 3.* version ever comes along, I'll surely remember this anyway 🤞
But until then, I don't think there's a point in keeping this open for years.

@wli3
Copy link

wli3 commented Jan 31, 2018

@dasMulli agree, closing :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dotnet publish output parameter doesn't work
7 participants