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

Allow move command to run on other OS besides Windows #4

Closed
wants to merge 6 commits into from

Conversation

scleaver
Copy link

Not everyone runs Windows lads ;)

@Piedone
Copy link
Member

Piedone commented Dec 12, 2020

Thank you! At one point I did think about this, but then I figured Seth will do it if he needs it :D.

Lombiq.Gulp.Extensions.csproj Outdated Show resolved Hide resolved
Lombiq.Gulp.Extensions.csproj Outdated Show resolved Hide resolved
@Piedone
Copy link
Member

Piedone commented Jan 3, 2021

Did you have a chance to look at this @scleaver?

@scleaver
Copy link
Author

scleaver commented Jan 6, 2021

@Piedone - please check this will work for Windows as it works in Mac (although it leaves a folder structure behind in the node_modules)... particularly check that my change to InitCommonNodeModulesStampFile mets your approval. ../../node_modules/.install-stamp seemed a bit fragile to me and I thought it would be better if it used the solution dir instead $(SolutionDir)\node_modules\.install-stamp

@scleaver scleaver mentioned this pull request Jan 6, 2021
Lombiq.Gulp.Extensions.csproj Outdated Show resolved Hide resolved
@scleaver
Copy link
Author

scleaver commented Jan 6, 2021

Apologies - should be good now.

@scleaver
Copy link
Author

scleaver commented Jan 6, 2021

The move task failed in a Linux build - currently testing to see why.

@scleaver
Copy link
Author

scleaver commented Jan 6, 2021

@Piedone - I could not get <Move SourceFiles="@(FilesToMove)" DestinationFolder="$(SolutionDir)node_modules\%(RecursiveDir)" /> to work within a Linux build pipeline. It kept reporting after the move task that the "The file "/home/vsts/work/1/s/node_modules/.install-stamp" cannot be created. Could not find a part of the path '/home/vsts/work/1/s/node_modules/.install-stamp'." - it seems that for some reason the move task does not work correctly so the time-stamp can not be created but there is no additional information on the move task in the log so I don't know why.

The move task works fine on my Mac though so not sure.

I moved back to <Exec Command="mv node_modules $(SolutionDir)" Condition="'$(OS)' != 'Windows_NT' AND Exists('node_modules')" /> and it went smoothly. The move task moves individual files not the folders so the empty folders remain in the project folder however the %(RecursiveDir) worked to maintain the folder structure when moving.

I'll leave it with you as to what to do here... I still think it is improved over the original. It just depends on whether you want to solve the move task issue or not in order to use it.

@Piedone
Copy link
Member

Piedone commented Jan 7, 2021

I see. Well, the joys of hooking into MSBuild internals, been there, done that :).

This will be fine for now, thank you. Just please be so kind and add a short comment to the commands why we're using them and not Move.

@Piedone
Copy link
Member

Piedone commented Jan 7, 2021

Thank you. I had some further fixes in the https:/Lombiq/Gulp-Extensions/commits/issue/FINI-506. I'll merged this there and if everything is alright, then later that to dev.

I can't change the base branch of this PR though so I don't think I can make GitHub think it is merged... So I'm just closing this and watch #9 for getting merged.

@Piedone Piedone closed this Jan 7, 2021
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.

2 participants