laravel / installer

The Laravel application installer.
https://laravel.com/docs
MIT License
646 stars 266 forks source link

Project directory is not empty (with --force) #129

Closed robsontenorio closed 4 years ago

robsontenorio commented 4 years ago

Description:

Error trying to create new app, even with --force option

Steps To Reproduce:

  [InvalidArgumentException]                        
  Project directory "/var/www/app/." is not empty.

Note 1: did not pass "app name" because I want app crafted on root folder /var/ww/app Note 2: that works on previous version of Laravel Installer

peter279k commented 4 years ago

Just notice that I have the similar issue when using the laravel new --dev on current folder.

Steps To Reproduce:

Expected

Actual

 _                               _
| |                             | |
| |     __ _ _ __ __ ___   _____| |
| |    / _` | '__/ _` \ \ / / _ \ |
| |___| (_| | | | (_| |\ V /  __/ |
|______\__,_|_|  \__,_| \_/ \___|_|

In NewCommand.php line 153:

  Application already exists!

new [--dev] [--jet] [-f|--force] [--] [<name>]

When using the laravel new --dev --force command to run above steps again, it will get following error:


 _                               _
| |                             | |
| |     __ _ _ __ __ ___   _____| |
| |    / _` | '__/ _` \ \ / / _ \ |
| |___| (_| | | | (_| |\ V /  __/ |
|______\__,_|_|  \__,_| \_/ \___|_|

Creating a "laravel/laravel" project at "./"

  [InvalidArgumentException]
  Project directory "/home/directory/app" is not empty.

I think it should be good to add asking question about Application /home/directory/app is existed. Do you want to override them? interactively.

driesvints commented 4 years ago

Should be fixed I think

hailwood commented 4 years ago

@driesvints it's definitely not fixed in the latest release (v4.0.3) which was released after your comment, so I'd assume the fix would have been in there if it was truly fixed.

Edit: After looking more into this, the issue is that Laravel uses --force to delete the named directory if it's not ".". However if it is "." then it just skips that option. Unfortunately composer create-project has no way to say "do your thing even through this directory is not empty" so there's not a lot here that the installer can do to fix it.

It would I believe be worth throwing an error if the name "." and the --force flag are used together, and also updating the option description to explain what it's doing e.g. "Remove the project directory if it already exists",

I wouldn't have expected --force to delete the named directory - I would have expected whatever files were there to be kept (and possible overwritten of the installer wanted to place files with the same name). Possibly even renaming the option to "--delete" or similar would be appropriate.

driesvints commented 4 years ago

@hailwood can you send a pr?

hailwood commented 4 years ago

@driesvints I'm happy to, what would you like in the PR?

  1. Throw error if "." path and --force flag used together
  2. Update description of --force to "Remove the project directory if it already exists"
  3. Rename --force to --delete
driesvints commented 4 years ago

Think 1. makes sense to me. Use the $this->error method instead of throwing an exception.

cfreear commented 4 years ago

From this am I correct in assuming that while using laravel new . --force to create a new Laravel app in the current directory that may contain other files (.git, .idea etc.) used to work on previous versions of installer it was not a supported use case?

driesvints commented 4 years ago

Not 100% sure about that.

cfreear commented 4 years ago

No worries, looks like it's a wontfix from composers side anyway. My typical workflow involves opening an empty folder in phpstorm (which creates an .idea folder) and using the built in terminal to run laravel new. I'll just have to remember to do it the other way round! 😅

hailwood commented 4 years ago

You are correct @cfreear. Thing is that before we were simply downloading and extracting a zip file which meant things were quite happy to just extract in place.

Now we're using composer create-project which due to it essentially being a git clone and then deleting the .git directory, doesn't work in a non-empty directory.

Realistically we could fix this by using create-project into a tmp directory, and then copying the files over, but I don't believe it's worth the additional complexity it would introduce.

driesvints commented 4 years ago

@hailwood agreed

SiestaCat commented 1 year ago

I need to create new project and only .git directory is present. I think this is a bug because .git directory doesn't affects in any way.