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

using LF for .sh generators #6670

Merged
merged 8 commits into from Mar 27, 2020

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Mar 12, 2020

Changelog: Bugfix: Use always LF line separator for .sh scripts generated by virtualenv generators.
Docs: Omit

Close #6662

Trying to see if this breaks something

#tags: slow

@memsharded memsharded requested a review from jgsogo March 12, 2020 16:09
@memsharded
Copy link
Member Author

@jgsogo

This PR is almost passing, except the Windows test. VirtualenvWindowsBashTestCase, that has:

 def package_info(self):
                # Basic variable
                self.env_info.USER_VAR = r"some value with space and \\ (backslash)"

As I am doing a total replace of backslashes, then this is a problem. Any suggestion?

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

The user reported that the colleague needed to change D:\... into /d/..., not only the backward to forward slashes...

@jokram
Copy link

jokram commented Mar 13, 2020

The user reported that the colleague needed to change D:... into /d/..., not only the backward to forward slashes...

I'm the user and we should ignore this, because I couldn't reproduce it: see #6662 (comment)

Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

It looks like that the file ending can be merged, but not the path modification according to @jokram , who is doing an exhaustive investigation: #6662 (comment) So, at least, these two things have to be separated in different PRs

@jgsogo jgsogo added this to the 1.24 milestone Mar 25, 2020
conans/client/generators/virtualenv.py Outdated Show resolved Hide resolved
conans/client/generators/virtualenv.py Outdated Show resolved Hide resolved
conans/client/generators/virtualenv.py Outdated Show resolved Hide resolved
conans/client/generators/virtualenv.py Outdated Show resolved Hide resolved
@memsharded
Copy link
Member Author

Yes, agree to everything, reviewed.

@memsharded memsharded requested a review from jgsogo March 26, 2020 15:10
@memsharded memsharded assigned jgsogo and unassigned memsharded Mar 26, 2020
Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

Great! This one is ready \o/

@jgsogo jgsogo merged commit 0c147fe into conan-io:develop Mar 27, 2020
@memsharded memsharded deleted the feature/sh_generators_lf branch March 28, 2020 16:35
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.

[bug] activate.sh/deactivate.sh does not work in cygwin
3 participants