dfinke / ImportExcel

PowerShell module to import/export Excel spreadsheets, without Excel
https://www.powershellgallery.com/packages/ImportExcel/
Apache License 2.0
2.48k stars 399 forks source link

broken version pushed to powershellgallery #1599

Closed barthem closed 5 months ago

barthem commented 6 months ago

seems that broken versions of importexcel are pushed to powershellgallery. if i try to run import-excel with version 7.8.7 i get the following error.

Import-Module : Could not load file or assembly 'file:///C:\Program Files\WindowsPowerShell\Modules\Importexcel\7.8.7\EPPlus.dll' or one of its dependencies. The system cannot find the file specified.

version 7.8.6 seems to be working fine.

evild3ad commented 6 months ago

https://www.powershellgallery.com/packages/ImportExcel/7.8.7 0 downloads

dfinke commented 6 months ago

Yes. 7.8.8 was published and had no issues: https://www.powershellgallery.com/packages/ImportExcel/7.8.8

Tuumke commented 6 months ago
At C:\Program Files\WindowsPowerShell\Modules\ImportExcel\7.8.8\Public\Export-Excel.ps1:127 char:17
+         catch { throw "Could not open Excel Package $path" }
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : OperationStopped: (Could not open ...RolGroepen.xlsx:String) [], RuntimeException
    + FullyQualifiedErrorId : Could not open Excel Package C:\Users\myuser\AppData\Local\Temp\4\{0:yyyyMMdd}-{0:HHmmss}-OTA-Export-ADRolGroepen.xlsx

My code which used to work:

## Temporary output file
$tempPath = [System.IO.Path]::GetTempPath()
$fileName = "{0:yyyyMMdd}-{0:HHmmss}-" + $environment + "-Export-ADRolGroepen" -f (Get-Date)
$outXlsx = $tempPath + $fileName + ".xlsx"

## get date in between these parts ##

$groups | Export-Excel -Path $outXlsx -AutoSize -AutoFilter
PS C:\Users\myuser> Get-Module -Name ImportExcel

ModuleType Version    Name                                ExportedCommands
---------- -------    ----                                ----------------
Script     7.8.8      ImportExcel                         {Add-ConditionalFormatting, Add-ExcelChart, Add-ExcelDataValidationRule, Add-ExcelName...}

PS C:\Users\myuser>

I've requested one of our teams to roll out the machine again or reinstall the module (since i can't). Hope that fixes it for me

-edit- Won't be able to check until tuesday probably.

dfinke commented 6 months ago

Bummer. Couple things. The only change in the code was for Import-Excel. So hopefully this is related to whatever the publishing error was.

The counts on the 7.8.8 are going up. When published, it stayed at zero for a couple of days.

Tuumke commented 6 months ago

Found some time on my day off. Fresh server with 7.8.8. rolled out:

Could not open Excel Package C:\Users\myuser\AppData\Local\Temp\2\{0:yyyyMMdd}-{0:HHmmss}-OTA-Export-ADRolGroepen.xlsx
At C:\Program Files\WindowsPowerShell\Modules\ImportExcel\7.8.8\Public\Export-Excel.ps1:127 char:17
+         catch { throw "Could not open Excel Package $path" }
+                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : OperationStopped: (Could not open ...RolGroepen.xlsx:String) [], RuntimeException
    + FullyQualifiedErrorId : Could not open Excel Package C:\Users\myuser\AppData\Local\Temp\2\{0:yyyyMMdd}-{0:HHmmss
   }-OTA-Export-ADRolGroepen.xlsx

Might this not be related to this issue but a complete seperate issue? If so, i can post a seperate one.

scriptingstudio commented 6 months ago

At first sight you have illegal chars in the filename: :. Remember of naming rules: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions

UPDATE Maybe it makes sense to validate input $path to clarify the root of errors. (split-path $path -leaf) -match '[\<\>\:"/\\|\?\*]'

dfinke commented 6 months ago

@scriptingstudio put that in the module?

This is the first time this type of issue has been reported.

Could use something like this.

 $invalidChars = [System.IO.Path]::GetInvalidFileNameChars()

    foreach ($char in $invalidChars) {
        if ($fileName.Contains($char)) {
            return $false
        }
    }
scriptingstudio commented 6 months ago

Another form - oneliner: if ([System.IO.Path]::GetInvalidFileNameChars() -match ([char[]](split-path $path -leaf) -join '|')) {throw 'bla-bla'}

I think [System.IO.Path]::GetInvalidFileNameChars() is too wide range given that this is a rather rare case. A simple regexp would be enough as a starter solution in Open-ExcelPackage. if ((split-path $path -leaf) -match '[\<\>\:"/\\|\?\*]') {throw 'bla-bla'}

A small but qualitative improvement.

Tuumke commented 6 months ago

At first sight you have illegal chars in the filename: :. Remember of naming rules: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#naming-conventions

UPDATE Maybe it makes sense to validate input $path to clarify the root of errors. (split-path $path -leaf) -match '[\<\>\:"/\\|\?\*]'

Thank you sir! I don't know how i didn't get this. Even in checken the $outXlsx variable.. Fixed it by moving the -f (Get-Date) :

# From this #
$tempPath = [System.IO.Path]::GetTempPath()
$fileName = "{0:yyyyMMdd}-{0:HHmmss}-" + $environment + "-Export-ADRolGroepen" -f (Get-Date)
$outXlsx = $tempPath + $fileName + ".xlsx"

# To This #
$tempPath = [System.IO.Path]::GetTempPath()
$fileName = "{0:yyyyMMdd}-{0:HHmmss}-" -f (Get-Date) + $environment + "-Export-ADRolGroepen"
$outXlsx = $tempPath + $fileName + ".xlsx"
scriptingstudio commented 6 months ago

@Tuumke you missed parentheses around format string: $fileName = ( "{0:yyyyMMdd}-{0:HHmmss}-" + $environment + "-Export-ADRolGroepen" ) -f (Get-Date)

nlsdg commented 6 months ago

Indeed: the parentheses are the problem. If you have more than one operator in a command, you need to be careful of operator precedence.

You could do the format for the whole filename in one string, so you only need the format operator and no additional string concatenation. That's what the format operator was designed to do: it accepts multiple placeholders in the string, and an array of values.

$fileName = "{0:yyyyMMdd}-{0:HHmmss}-{1}-Export-ADRolGroepen.xlsx" -f (Get-Date), $environment

nlsdg commented 5 months ago

The latest version reported on the Powershell Gallery is 7.8.9. The latest release according to Github is 7.8.7. Is there still some problem with the publishing?

dfinke commented 5 months ago

Pushed the release on GitHub