Skip to content

Conversation

@stevegury
Copy link
Contributor

Hi Mark,

I've found a problem when trying to build the master branch of xsbt on Windows (especially because I use paths with spaces).
There was some paths used by proguard in InstallExtractProject that was not properly escaped.

When trying to fixed this, I've found that they was a trait ProguardProject where all the proguard stuffs was defined. I've refactored InstallExtractProject to mix this trait, I've not see any problem with that, can you confirm that this fix is correct ?

Thanks,
Steve Gury

stevegury and others added 2 commits February 16, 2011 23:29
…efactor InstallExtarctProject in order to be a subclass of the Proguard trait
@harrah
Copy link
Member

harrah commented Feb 22, 2011

Hi Steve,

Thanks for working on this. Comments:

  1. The changes to ProguardProject look ok except perhaps you meant log.debug instead of log.error in the second commit?
  2. The diff to InstallExtractProject shows up as touching every line, although it looks like most lines aren't touched. Can you fix this?
  3. The file permissions are changed from 644 to 755 (perhaps a windows+git issue?), which isn't needed.

Thanks,
Mark

@stevegury
Copy link
Contributor Author

Mark,

I've made the appropriate changes, as requested.
Here is some quick comments on your remarks:

  1. You're right, it was some forgotten debugging trace
  2. My IDE change the "tab indented" file into a "double space indented" file, my bad
  3. You're right it was a cygwin + git issue

Thanks for reading my code,
Steve

@harrah
Copy link
Member

harrah commented Feb 24, 2011

Thanks, Steve. It looks good now. I assume you replaced @ with -include because @ didn't mix well with escaping?

-Mark

@stevegury
Copy link
Contributor Author

My first guess was that @ didn't work with quoted path, but actually no, "@" and "-include " are equivalent.
I can change "-include " to "@" if you prefer ?

Steve

@harrah
Copy link
Member

harrah commented Mar 6, 2011

Hi Steve,

Sorry it took so long to get back to you. Using -include is not a problem. Can you squash the commits into one, though?

Thanks,
Mark

…efactor InstallExtarctProject in order to be a subclass of the Proguard trait

Removed path escaping on classpath...

Remove unuseful log.error, change space indent with tab indent, restore file permission to 644
@stevegury
Copy link
Contributor Author

Hi Mark,

Here is the squash commit (3da8e4f)

Enjoy,
Steve

@harrah
Copy link
Member

harrah commented Mar 23, 2011

Done, thanks.

@harrah harrah closed this Mar 23, 2011
eed3si9n pushed a commit to eed3si9n/sbt that referenced this pull request Apr 22, 2017
Conflicts:
	src/main/scala/sbt/CrossBuilding.scala
eed3si9n pushed a commit to eed3si9n/sbt that referenced this pull request Sep 22, 2020
eed3si9n pushed a commit to eed3si9n/sbt that referenced this pull request Sep 22, 2020
eed3si9n pushed a commit to eed3si9n/sbt that referenced this pull request Apr 20, 2021
eed3si9n added a commit that referenced this pull request Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants