You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/11/19 15:37:10 UTC

[GitHub] [maven] JurrianFahner opened a new pull request, #878: [MNG-7541] Implement powershell command

JurrianFahner opened a new pull request, #878:
URL: https://github.com/apache/maven/pull/878

   Following this checklist to help us incorporate your
   contribution quickly and easily:
   
    - [X] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
          for the change (usually before you start working on it).  Trivial changes like typos do not
          require a JIRA issue. Your pull request should address just this issue, without
          pulling in other changes.
    - [X] Each commit in the pull request should have a meaningful subject line and body.
    - [X] Format the pull request title like `[MNG-XXX] SUMMARY`, where you replace `MNG-XXX`
          and `SUMMARY` with the appropriate JIRA issue. Best practice is to use the JIRA issue
          title in the pull request title and in the first line of the commit message.
    - [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [X] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
          be performed on your pull request automatically.
    - [ ] You have run the [Core IT][core-its] successfully.
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [X] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   [core-its]: https://maven.apache.org/core-its/core-it-suite/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1474352343

   @michael-o From my perspective this PR is ready. What's your perspective on this position?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1358087993


##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,53 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>
+
+if (-not $env:MAVEN_SKIP_RC) {
+  if (Test-Path -Path $env:PROGRAMDATA\mavenrc.ps1 -PathType Leaf) { &$env:PROGRAMDATA"\mavenrc.ps1" $args }
+  if (Test-Path -Path $env:USERPROFILE\mavenrc.ps1 -PathType Leaf) { &$env:USERPROFILE"\mavenrc.ps1" $args }

Review Comment:
   No, look at the other files. First we load general, then specific. This give the user the option to override everything.



##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,53 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>
+
+if (-not $env:MAVEN_SKIP_RC) {
+  if (Test-Path -Path $env:PROGRAMDATA\mavenrc.ps1 -PathType Leaf) { &$env:PROGRAMDATA"\mavenrc.ps1" $args }
+  if (Test-Path -Path $env:USERPROFILE\mavenrc.ps1 -PathType Leaf) { &$env:USERPROFILE"\mavenrc.ps1" $args }

Review Comment:
   No, look at the other implementations. First we load general, then specific. This give the user the option to override everything.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #878: [MNG-7541] Implement powershell command

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1320959718

   Has this been tested with Windows PowerShell or PowerShell 7?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by GitBox <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1027127586


##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,9 @@
+# check mvn home
+if (-not $env:MAVEN_HOME) {
+    $env:MAVEN_HOME = (Get-Item $PSScriptRoot"\..")
+}

Review Comment:
   Interesting... I assumed it had the similar interpretation as JAVA_HOME (which is an environment variable). Always dangerous, making assumptions... 🤔 
   
   Thanks for pointing it out! I'll update the pull request. 
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1421482049

   The CoreIT tests have run well. #982 mentions to remove `.cmd` in master. Please let me know whether I need to continue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1730247235

   @Giovds @michael-o This branch contains the latest improvements from #982.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1509736437

   @michael-o Gentle reminder


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1334834141


##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,7 @@
+# check mvn home
+$MAVEN_HOME = (Get-Item $PSScriptRoot).Parent
+
+# check if maven command exists
+if (-not (Test-path "$MAVEN_HOME\bin\mvn.ps1")) {
+    Write-Error -Message "maven command (\bin\mvn.ps1) cannot be found" -ErrorAction Stop

Review Comment:
   Should be identical to others



##########
apache-maven/src/assembly/shared/run.ps1:
##########
@@ -0,0 +1,13 @@
+& $JAVACMD `
+  $MAVEN_OPTS `
+  $MAVEN_DEBUG_OPTS `
+  -classpath $LAUNCHER_JAR `

Review Comment:
   I would expect, at least the same behavior as the others...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1567897655


##########
apache-maven/src/assembly/shared/init.ps1:
##########
@@ -0,0 +1,83 @@
+# ==== END VALIDATION ====
+
+$CLASSWORLDS_CONF = "$MAVEN_HOME\bin\m2.conf"
+
+# Find the project basedir, i.e., the directory that contains the directory ".mvn".
+# Fallback to current working directory if not found.
+
+$WDIR = Get-Location
+
+# Look for the --file switch and start the search for the .mvn directory from the specified
+# POM location, if supplied.
+
+$i = 0
+$file_flag_found = $false
+foreach ($arg in $args) {
+  if (($arg -ceq "-f") -or ($arg -ceq "--file")) {
+    $file_flag_found = $true
+    break
+  }
+  $i += 1
+}
+
+function IsNotRoot {
+  param (
+    [String] $path
+  )
+  $root_path_base_name = ((get-item $path).Root.BaseName
+  $path_base_name = ((get-item $path).BaseName
+
+  return $root_path_base_name -ne $path_base_name
+}
+
+function RetrieveContentsJvmConfig {
+  param (
+    [String] $path
+  )
+
+  $jvm_config = $path + "\.mvn\jvm.config"
+
+  if (Test-Path $jvm_config) {
+    return $env:MAVEN_OPTS + " " + -Join (Get-Content $jvm_config)
+  }
+  return $env:MAVEN_OPTS

Review Comment:
   I don't understand your comment. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on pull request #878: [MNG-7541] Implement powershell command

Posted by GitBox <gi...@apache.org>.
JurrianFahner commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1320972669

   > Has this been tested with Windows PowerShell or PowerShell 7?
   
   Mainly with powershell 7.3.0, but now also with powershell 5.1.22621.608. 
   
   The test is done by running `mvn clean package` on a simple project. It will not cover all use-cases for the scripts that have been changed. Is there an example project to check all of these? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "Giovds (via GitHub)" <gi...@apache.org>.
Giovds commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1358084618


##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,53 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>
+
+if (-not $env:MAVEN_SKIP_RC) {
+  if (Test-Path -Path $env:PROGRAMDATA\mavenrc.ps1 -PathType Leaf) { &$env:PROGRAMDATA"\mavenrc.ps1" $args }
+  if (Test-Path -Path $env:USERPROFILE\mavenrc.ps1 -PathType Leaf) { &$env:USERPROFILE"\mavenrc.ps1" $args }

Review Comment:
   I was wondering if it would make more sense to prioritize looking in the userprofile first before going to a more generic location



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1334833714


##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,42 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+
+-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is localhost:8000
+-----------------------------------------------------------------------------
+#>
+
+# set title
+$Host.UI.RawUI.WindowTitle = $MyInvocation.MyCommand
+
+if (-not $env:MAVEN_DEBUG_ADDRESS ) {
+  $env:MAVEN_DEBUG_ADDRESS = "8000"

Review Comment:
   This I know, but still, line 27 does not correspond to line 35



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #878: [MNG-7541] Implement powershell command

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1320973095

   This is at least what I have, although I work from Windows Terminal with PowerShell 7 only:
   ```
   C:\Users\mosipov> $PSVersionTable
   
   Name                           Value
   ----                           -----
   PSVersion                      5.1.19041.1682
   PSEdition                      Desktop
   PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
   BuildVersion                   10.0.19041.1682
   CLRVersion                     4.0.30319.42000
   WSManStackVersion              3.0
   PSRemotingProtocolVersion      2.3
   SerializationVersion           1.1.0.1
   ```
   
   I'd expect that on a Windows CI Agent only the Windows PS will be used. Thus, this should be our minimum requirement.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1333573181


##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,42 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+
+-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is localhost:8000
+-----------------------------------------------------------------------------
+#>

Review Comment:
   Maybe license hedader and description should be separate comment blocks...



##########
apache-maven/src/assembly/shared/init.ps1:
##########
@@ -0,0 +1,83 @@
+# ==== END VALIDATION ====
+
+$CLASSWORLDS_CONF = "$MAVEN_HOME\bin\m2.conf"
+
+# Find the project basedir, i.e., the directory that contains the directory ".mvn".
+# Fallback to current working directory if not found.
+
+$WDIR = Get-Location
+
+# Look for the --file switch and start the search for the .mvn directory from the specified
+# POM location, if supplied.
+
+$i = 0
+$file_flag_found = $false
+foreach ($arg in $args) {
+  if (($arg -ceq "-f") -or ($arg -ceq "--file")) {
+    $file_flag_found = $true
+    break
+  }
+  $i += 1
+}
+
+function IsNotRoot {
+  param (
+    [String] $path
+  )
+  $root_path_base_name = ((get-item $path).Root.BaseName
+  $path_base_name = ((get-item $path).BaseName
+
+  return $root_path_base_name -ne $path_base_name
+}
+
+function RetrieveContentsJvmConfig {
+  param (
+    [String] $path
+  )
+
+  $jvm_location = $path + "\.mvn\jvm.config"

Review Comment:
   The name of this var is weird, this is not about the JVM location, but rather `$jvm_config`?



##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,7 @@
+# check mvn home
+$MAVEN_HOME = (Get-Item $PSScriptRoot).Parent
+
+# check if maven command exists
+if (-not (Test-path "$MAVEN_HOME\bin\mvn.ps1")) {
+    Write-Error -Message "maven command (\bin\mvn.ps1) cannot be found" -ErrorAction Stop

Review Comment:
   Why not `($MAVEN_HOME \bin\mvn.ps1)`?
   Do we have such a message with the other scripts?



##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,7 @@
+# check mvn home
+$MAVEN_HOME = (Get-Item $PSScriptRoot).Parent
+
+# check if maven command exists
+if (-not (Test-path "$MAVEN_HOME\bin\mvn.ps1")) {
+    Write-Error -Message "maven command (\bin\mvn.ps1) cannot be found" -ErrorAction Stop

Review Comment:
   Maven...



##########
apache-maven/src/assembly/shared/run.ps1:
##########
@@ -0,0 +1,13 @@
+& $JAVACMD `
+  $MAVEN_OPTS `
+  $MAVEN_DEBUG_OPTS `
+  -classpath $LAUNCHER_JAR `

Review Comment:
   Will this handle spaces in `$LAUNCHER_JAR`?



##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,42 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+
+-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is localhost:8000
+-----------------------------------------------------------------------------
+#>
+
+# set title
+$Host.UI.RawUI.WindowTitle = $MyInvocation.MyCommand
+
+if (-not $env:MAVEN_DEBUG_ADDRESS ) {
+  $env:MAVEN_DEBUG_ADDRESS = "8000"

Review Comment:
   This does not correspond to the description



##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,52 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+
+-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>

Review Comment:
   Same here



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1582075212


##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,7 @@
+# check mvn home
+$MAVEN_HOME = (Get-Item $PSScriptRoot).Parent
+
+# check if maven command exists
+if (-not (Test-path "$MAVEN_HOME\bin\mvn.ps1")) {
+    Write-Error -Message "Maven command ($MAVEN_HOME\bin\mvn.ps1) cannot be found" -ErrorAction Stop
+}

Review Comment:
   But how can this happen at all? This is a chicken-and-egg issue. This block is part of the merged `mvn.ps1`. If the file is not present that check will never happen, no? This confuses me.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #878: [MNG-7541] Implement powershell command

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1474362717

   > @michael-o From my perspective this PR is ready. What's your perspective on this position?
   
   I want to get back to this end of month.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "Giovds (via GitHub)" <gi...@apache.org>.
Giovds commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1358102359


##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,53 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>
+
+if (-not $env:MAVEN_SKIP_RC) {
+  if (Test-Path -Path $env:PROGRAMDATA\mavenrc.ps1 -PathType Leaf) { &$env:PROGRAMDATA"\mavenrc.ps1" $args }
+  if (Test-Path -Path $env:USERPROFILE\mavenrc.ps1 -PathType Leaf) { &$env:USERPROFILE"\mavenrc.ps1" $args }

Review Comment:
   You are right, I've misread it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1582075040


##########
apache-maven/src/assembly/shared/init.ps1:
##########
@@ -0,0 +1,83 @@
+# ==== END VALIDATION ====
+
+$CLASSWORLDS_CONF = "$MAVEN_HOME\bin\m2.conf"
+
+# Find the project basedir, i.e., the directory that contains the directory ".mvn".
+# Fallback to current working directory if not found.
+
+$WDIR = Get-Location
+
+# Look for the --file switch and start the search for the .mvn directory from the specified
+# POM location, if supplied.
+
+$i = 0
+$file_flag_found = $false
+foreach ($arg in $args) {
+  if (($arg -ceq "-f") -or ($arg -ceq "--file")) {
+    $file_flag_found = $true
+    break
+  }
+  $i += 1
+}
+
+function IsNotRoot {
+  param (
+    [String] $path
+  )
+  $root_path_base_name = ((get-item $path).Root.BaseName
+  $path_base_name = ((get-item $path).BaseName
+
+  return $root_path_base_name -ne $path_base_name
+}
+
+function RetrieveContentsJvmConfig {
+  param (
+    [String] $path
+  )
+
+  $jvm_config = $path + "\.mvn\jvm.config"
+
+  if (Test-Path $jvm_config) {
+    return $env:MAVEN_OPTS + " " + -Join (Get-Content $jvm_config)
+  }
+  return $env:MAVEN_OPTS

Review Comment:
   You are joining the content of the `jvm.config` file with `MAVEN_OPTS` variable in one guy. From a caller's perspective that should not be done and it is not obvious from where the config comes. Look at the original files, they do not merge them.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1027136370


##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,9 @@
+# check mvn home
+if (-not $env:MAVEN_HOME) {
+    $env:MAVEN_HOME = (Get-Item $PSScriptRoot"\..")
+}

Review Comment:
   These scripts are implementation details and can change from version to version therefore we cannot share those between other Maven homes reliably.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #878: [MNG-7541] Implement powershell command

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1320973205

   Try the Maven Core ITs with this. Make sure that `.cmd` isn't included, just in case.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1334826314


##########
apache-maven/src/assembly/shared/run.ps1:
##########
@@ -0,0 +1,13 @@
+& $JAVACMD `
+  $MAVEN_OPTS `
+  $MAVEN_DEBUG_OPTS `
+  -classpath $LAUNCHER_JAR `

Review Comment:
   To my knowledge, yes. You don't expect spaces to be handled? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1567900347


##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,7 @@
+# check mvn home
+$MAVEN_HOME = (Get-Item $PSScriptRoot).Parent
+
+# check if maven command exists
+if (-not (Test-path "$MAVEN_HOME\bin\mvn.ps1")) {
+    Write-Error -Message "Maven command ($MAVEN_HOME\bin\mvn.ps1) cannot be found" -ErrorAction Stop
+}

Review Comment:
   If `mvn.ps1` or `mvn.cmd` is removed or not accessible.  
   
   But it is indeed a corner case, shall we remove it? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1027108363


##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,9 @@
+# check mvn home
+if (-not $env:MAVEN_HOME) {
+    $env:MAVEN_HOME = (Get-Item $PSScriptRoot"\..")
+}

Review Comment:
   This is wrong. `MAVEN_HOME` is neither an environment variable, nor can it be passed externally. It is solely private. See https://github.com/apache/maven/blob/29283a10596c92a9283d789d4c661dd242cec916/apache-maven/src/assembly/shared/mvnvalidate



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1334818771


##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,42 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+
+-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is localhost:8000
+-----------------------------------------------------------------------------
+#>
+
+# set title
+$Host.UI.RawUI.WindowTitle = $MyInvocation.MyCommand
+
+if (-not $env:MAVEN_DEBUG_ADDRESS ) {
+  $env:MAVEN_DEBUG_ADDRESS = "8000"

Review Comment:
   But it correspond to the use in the debug string. 
   
   And with Java > 9, it is possible to specify ip and port, e.g. 8.8.8.8:8000
   Or if you want to bind to all network interfaces (which would be the same behavior as in java <9): *:8000
   To specify only port, will result with java > 9: 127.0.0.1:8000
   
   Luckily it is possible to override the environment variable.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1334839636


##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,42 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+
+-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is localhost:8000
+-----------------------------------------------------------------------------
+#>
+
+# set title
+$Host.UI.RawUI.WindowTitle = $MyInvocation.MyCommand
+
+if (-not $env:MAVEN_DEBUG_ADDRESS ) {
+  $env:MAVEN_DEBUG_ADDRESS = "8000"

Review Comment:
   I've changed the description.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1565386307


##########
apache-maven/src/assembly/shared/init.ps1:
##########
@@ -0,0 +1,83 @@
+# ==== END VALIDATION ====
+
+$CLASSWORLDS_CONF = "$MAVEN_HOME\bin\m2.conf"
+
+# Find the project basedir, i.e., the directory that contains the directory ".mvn".
+# Fallback to current working directory if not found.
+
+$WDIR = Get-Location
+
+# Look for the --file switch and start the search for the .mvn directory from the specified
+# POM location, if supplied.
+
+$i = 0
+$file_flag_found = $false
+foreach ($arg in $args) {
+  if (($arg -ceq "-f") -or ($arg -ceq "--file")) {
+    $file_flag_found = $true
+    break
+  }
+  $i += 1
+}
+
+function IsNotRoot {
+  param (
+    [String] $path
+  )
+  $root_path_base_name = ((get-item $path).Root.BaseName
+  $path_base_name = ((get-item $path).BaseName
+
+  return $root_path_base_name -ne $path_base_name
+}
+
+function RetrieveContentsJvmConfig {
+  param (
+    [String] $path
+  )
+
+  $jvm_config = $path + "\.mvn\jvm.config"
+
+  if (Test-Path $jvm_config) {
+    return $env:MAVEN_OPTS + " " + -Join (Get-Content $jvm_config)
+  }
+  return $env:MAVEN_OPTS
+}
+
+$basedir = ""
+
+if ($file_flag_found) {
+  # we need to assess if the file exists
+  # and then search for the maven project base dir. 
+  $pom_file_reference = $args[$i + 1]
+
+  if (Test-Path $pom_file_reference) {
+    $basedir = (Get-Item $pom_file_reference).DirectoryName
+  }
+  else {
+    $pom_file_error = "POM file " + $pom_file_reference + " specified the -f/--file command-line argument does not exist"
+    Write-Error -Message $pom_file_error -ErrorAction Stop
+  }
+}
+else {
+  # if file flag is not found, then the pom.xml is relative to the working dir 
+  # and the jvm.config can be found in the maven project base dir. 
+
+  $basedir = $WDIR
+
+  while (IsNotRoot($WDIR.Path)) {
+    if (Test-Path "$WDIR\.mvn") {
+      $basedir = $WDIR
+      break
+    }
+
+    if ($WDIR) {
+      $WDIR = Split-Path $WDIR      
+    }
+    else {
+      break
+    }  
+  }
+}
+
+$MAVEN_OPTS = (RetrieveContentsJvmConfig $basedir)

Review Comment:
   See above.



##########
apache-maven/src/assembly/shared/init.ps1:
##########
@@ -0,0 +1,83 @@
+# ==== END VALIDATION ====
+
+$CLASSWORLDS_CONF = "$MAVEN_HOME\bin\m2.conf"
+
+# Find the project basedir, i.e., the directory that contains the directory ".mvn".
+# Fallback to current working directory if not found.
+
+$WDIR = Get-Location
+
+# Look for the --file switch and start the search for the .mvn directory from the specified
+# POM location, if supplied.
+
+$i = 0
+$file_flag_found = $false
+foreach ($arg in $args) {
+  if (($arg -ceq "-f") -or ($arg -ceq "--file")) {
+    $file_flag_found = $true
+    break
+  }
+  $i += 1
+}
+
+function IsNotRoot {
+  param (
+    [String] $path
+  )
+  $root_path_base_name = ((get-item $path).Root.BaseName
+  $path_base_name = ((get-item $path).BaseName
+
+  return $root_path_base_name -ne $path_base_name
+}
+
+function RetrieveContentsJvmConfig {
+  param (
+    [String] $path
+  )
+
+  $jvm_config = $path + "\.mvn\jvm.config"
+
+  if (Test-Path $jvm_config) {
+    return $env:MAVEN_OPTS + " " + -Join (Get-Content $jvm_config)
+  }
+  return $env:MAVEN_OPTS

Review Comment:
   That is not correct. If the file is empty then it is empty. See approach in `init`.



##########
apache-maven/src/assembly/shared/init.ps1:
##########
@@ -0,0 +1,83 @@
+# ==== END VALIDATION ====
+
+$CLASSWORLDS_CONF = "$MAVEN_HOME\bin\m2.conf"

Review Comment:
   I'd keep it identical to `init.cmd`.



##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,43 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is 8000

Review Comment:
   Default value in `.cmd` is `localhost:8000`, please fix.



##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,7 @@
+# check mvn home
+$MAVEN_HOME = (Get-Item $PSScriptRoot).Parent
+
+# check if maven command exists
+if (-not (Test-path "$MAVEN_HOME\bin\mvn.ps1")) {
+    Write-Error -Message "Maven command ($MAVEN_HOME\bin\mvn.ps1) cannot be found" -ErrorAction Stop
+}

Review Comment:
   I wonder whether this is a relic from the past. In `mvnvalidate` this check isn't present and this file is part of `mvn`/`mvn.cmd`/`mvn.ps1`. How can this fail?`



##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,53 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>
+
+if (-not $env:MAVEN_SKIP_RC) {
+  if (Test-Path -Path $env:PROGRAMDATA\mavenrc.ps1 -PathType Leaf) { &$env:PROGRAMDATA"\mavenrc.ps1" $args }

Review Comment:
   Why not `Test-File`?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1334818771


##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,42 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+
+-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is localhost:8000
+-----------------------------------------------------------------------------
+#>
+
+# set title
+$Host.UI.RawUI.WindowTitle = $MyInvocation.MyCommand
+
+if (-not $env:MAVEN_DEBUG_ADDRESS ) {
+  $env:MAVEN_DEBUG_ADDRESS = "8000"

Review Comment:
   But it correspond to the use in the debug string. 
   
   And with Java > 9, it is possible to specify ip and port, e.g. 8.8.8.8:8000
   Or if you want to bind to all network interfaces: *:8000



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] JurrianFahner commented on a diff in pull request #878: [MNG-7541] Implement powershell command

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1334825689


##########
apache-maven/src/assembly/shared/mvnvalidate.ps1:
##########
@@ -0,0 +1,7 @@
+# check mvn home
+$MAVEN_HOME = (Get-Item $PSScriptRoot).Parent
+
+# check if maven command exists
+if (-not (Test-path "$MAVEN_HOME\bin\mvn.ps1")) {
+    Write-Error -Message "maven command (\bin\mvn.ps1) cannot be found" -ErrorAction Stop

Review Comment:
   No, we don't have such a message with the other scripts. From my point of view if the maven command can't be found, the script must stop and print an error so the problem can be resolved. 
   
   Although this would be an edge case. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on PR #878:
URL: https://github.com/apache/maven/pull/878#issuecomment-1763182870

   > It would be nice to fix the command terminal title, the rest LGTM
   
   I've created an issue to improve the command terminal title: https://issues.apache.org/jira/browse/MNG-7907
   
   The description is a little bit rough right now and needs some extra refinement. 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "Giovds (via GitHub)" <gi...@apache.org>.
Giovds commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1358059984


##########
apache-maven/src/assembly/maven/bin/mvnDebug.ps1:
##########
@@ -0,0 +1,43 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Debug Script
+
+Environment Variable Prerequisites
+
+JAVA_HOME           (Optional) Points to a Java installation.
+MAVEN_OPTS          (Optional) Java runtime options used when Maven is executed.
+MAVEN_SKIP_RC       (Optional) Flag to disable loading of mavenrc files.
+MAVEN_DEBUG_ADDRESS (Optional) Set the debug address. Default value is 8000
+-----------------------------------------------------------------------------
+#>
+
+# set title
+$Host.UI.RawUI.WindowTitle = $MyInvocation.MyCommand

Review Comment:
   This probably has the same 'issue' as as 3.x script: https://github.com/apache/maven/pull/982#discussion_r1324860640?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "Giovds (via GitHub)" <gi...@apache.org>.
Giovds commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1358075540


##########
apache-maven/src/assembly/shared/init.ps1:
##########
@@ -0,0 +1,83 @@
+# ==== END VALIDATION ====
+
+$CLASSWORLDS_CONF = "$MAVEN_HOME\bin\m2.conf"

Review Comment:
   Should this be part of `mvnlauncher.ps1` as it is never used in this file?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "JurrianFahner (via GitHub)" <gi...@apache.org>.
JurrianFahner commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1567902958


##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,53 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>
+
+if (-not $env:MAVEN_SKIP_RC) {
+  if (Test-Path -Path $env:PROGRAMDATA\mavenrc.ps1 -PathType Leaf) { &$env:PROGRAMDATA"\mavenrc.ps1" $args }

Review Comment:
   Is `Test-File` a valid command in powershell? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [MNG-7541] Implement powershell command [maven]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on code in PR #878:
URL: https://github.com/apache/maven/pull/878#discussion_r1582075348


##########
apache-maven/src/assembly/shared/validate.ps1:
##########
@@ -0,0 +1,53 @@
+<#
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+#>
+
+<#-----------------------------------------------------------------------------
+Apache Maven Startup Script
+
+Environment Variable Prerequisites
+
+  JAVA_HOME       (Optional) Points to a Java installation.
+  MAVEN_ARGS      (Optional) Arguments passed to Maven before CLI arguments.
+  MAVEN_OPTS      (Optional) Java runtime options used when Maven is executed.
+  MAVEN_SKIP_RC   (Optional) Flag to disable loading of mavenrc files.
+-----------------------------------------------------------------------------
+#>
+
+if (-not $env:MAVEN_SKIP_RC) {
+  if (Test-Path -Path $env:PROGRAMDATA\mavenrc.ps1 -PathType Leaf) { &$env:PROGRAMDATA"\mavenrc.ps1" $args }

Review Comment:
   You are totally right. My bad!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org