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

Introduce remove, the helper for cleaning temporary build items #123

Closed
nightroman opened this issue Mar 20, 2018 · 15 comments
Closed

Introduce remove, the helper for cleaning temporary build items #123

nightroman opened this issue Mar 20, 2018 · 15 comments

Comments

@nightroman
Copy link
Owner

nightroman commented Mar 20, 2018

Build scripts often produce temporary files and folders and require commands like

Remove-Item X, Y -Recurse -Force -ErrorAction Ignore

What's wrong with the above command? It is verbose. And it has a flaw. If X
cannot be removed (locked files, permissions, etc.) the command ignores it.
If this is not desired, we should remove -ErrorAction Ignore. But then
the command also fails on missing items. Thus, we should use Test-Path
in addition and the simple frequent task becomes not so simple.

TODO

Introduce a straightforward helper remove, so that the above frequent
operation can be invoked as

remove X, Y

UPDATE: Using Test-Path is not that simple, too, see PowerShell/PowerShell#6473
Thus, the proposed robust helper dealing with the issues would be really nice.

@nsartor
Copy link

nsartor commented Mar 20, 2018

I like this idea, and often use remove-item in my scripts.
Would this take a collection of objects as an argument? Could we pass a collection from get-childItem for example?

@nightroman
Copy link
Owner Author

Collection - yes. X, Y means it is going to be [string[]].

I did not plan the pipeline input. Why would you need this? Can you give an example?

@nsartor
Copy link

nsartor commented Mar 20, 2018

I don't think I'd need the output pipelined, but some way to check success or failure would be great:
Try{
(get-childItem some_path -recurse -filter *.tmp) | IB_Remove
}
Catch {
exit 1
}

@nightroman
Copy link
Owner Author

For the moment I am not planning this. The command is going to be dumb simple, at least its first version. It removes items with given names in the current directory and subdirectories, normally it is the build root. remove is not the replacement for Remove-Item. If you feel like remove is not enough for something then just use Remove-Item as usual.

@nsartor
Copy link

nsartor commented Mar 20, 2018

Ah ok. I won't be able to use it without any sort of logging/output to know of success or failure, but it might be useful to others. :)

@nightroman
Copy link
Owner Author

You will be able to catch errors, endeed. The command is going to write errors as usual if it cannot remove something. But if it does not find something this is not an error.

@nightroman
Copy link
Owner Author

nightroman commented Mar 20, 2018

Here is the prototype, you may start playing and giving some feedback
UPDATE: changed my mind, see further
<removed as potentially not safe>

@nightroman
Copy link
Owner Author

nightroman commented Mar 21, 2018

Here is a test in the dev branch: Remove.test.ps1

The task ErrorLockedFile shows terminating and non-terminating errors at your service.

@nightroman
Copy link
Owner Author

nightroman commented Mar 21, 2018

I have several doubts about the first version and I am thinking of the alternative approach

foreach($_ in $Path) {
    if (Test-Path $_) {
        Remove-Item $_ -Force -Recurse
    }
}

So that remove ... is similar to the mouthful Remove-Item ... -Force -Recurse -ErrorAction Ignore,
that is it removes existing items and ignores missing items. And at the same time:

  • it is short and clear
  • it fails if it cannot remove existing items, the important difference

The parameter $Path in this case is the same as Path of Remove-Item. This is cleaner.

@nightroman
Copy link
Owner Author

nightroman commented Mar 23, 2018

Turns out Test-Path *\X when X is hidden gets false. So the original task of removing items is even more complicated.

TODO: Investigate further and submit an issue to PowerShell. DONE
DONE: Work around this issue in remove. Add the test task HiddenInSubdirectory.

@nightroman
Copy link
Owner Author

v5.3.0

@majkinetor
Copy link

It would be good to oput what is done or not done ? Maybe with -Verbose ?

@nightroman
Copy link
Owner Author

Maybe. Like this, perhaps?

function Remove-BuildItem([Parameter(Mandatory=1)][string[]]$Path) {
	if ($Path -match '^[.*/\\]*$') {*Die 'Not allowed paths.' 5}
	$v = $PSBoundParameters['Verbose']
	try {
		foreach($_ in $Path) {
			if (Get-Item $_ -Force -ErrorAction 0) {
				if ($v) {Write-Verbose "remove: removing '$_'." -Verbose}
				Remove-Item $_ -Force -Recurse
			}
			elseif ($v) {
				Write-Verbose "remove: skipping '$_'." -Verbose
			}
		}
	}
	catch {
		*Die $_
	}
}

@majkinetor
Copy link

LGTM

@nightroman
Copy link
Owner Author

I opened a separate issue.

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

No branches or pull requests

3 participants