pypa / flit

Simplified packaging of Python modules
https://flit.pypa.io/
BSD 3-Clause "New" or "Revised" License
2.14k stars 130 forks source link

Remove destination data files if they exist. #658

Closed antarcticrainforest closed 9 months ago

antarcticrainforest commented 10 months ago

We recently switched to flit and I came across this issue when I installed a package using the "symlink" mode. What I did was adding more data files and then re installing the package to create the symlinks to the recently added data files:

flit install -s
Traceback (most recent call last):
FileExistsError: [Errno 17] File exists: '/home/wilfred/workspace/deployment/assets/share/freva/deployment/config/create_tables.sql' -> '/tmp/py_311/share/freva/deployment/config/create_tables.sql'

Since without -s any existing data files would be overridden anyways I figured it's ok to delete any existing data files.

takluyver commented 10 months ago

Hi Martin! Long time no see. How are you?

Thanks, I think this change makes sense. Just for neatness, though, let's try deleting it and ignore the error if it's not there. Pathlib makes this easy with .unlink(missing_ok=True). It won't usually make a difference, but it's one less way to hit odd behaviour if something else is adding/deleting files at the same time.

antarcticrainforest commented 9 months ago

Hi Thomas, all good on my end.

Yes, that makes sense what you're saying. I didn't see that you're already using pathlib in the code.

Btw. give me a shout when you're next time around in HH.

takluyver commented 9 months ago

Thanks! I was actually just there last week - the next visit will probably be around early December, I'll let you know when I've figured it out.

Yeah, I'm wildly inconsistent on using pathlib vs os.path. :smile:

takluyver commented 9 months ago

My bad, I had missed the note that missing_ok was added in Python 3.8.

I think we can drop Python 3.7 support from the main flit package now, as it's EOL anyway. I'll do a separate PR for that.

takluyver commented 9 months ago

OK, that's done now in #660. I'll merge that tomorrow if no-one objects, then we can re-run the tests on this one.

takluyver commented 9 months ago

Closing & reopening to re-run tests against the latest commit.

takluyver commented 9 months ago

Tests are passing now. Thanks Martin!