Repository: scrapy/scrapy
Pull request: fix: -O/-o option does not work with absolute paths on Windows
fix #5969 

Problem statement:
crawl -o and -O options does not work with absolute paths on Windows
Absolute paths on Windows has `:` as a part of disk names which leads to unexpected behaviour.

# Steps to Reproduce

Case 1: run command `scrapy crawl spider_name -O "C:\path\to\output.json"`

Case 2: run command `scrapy crawl spider_name -O "C:\path\to\output.json:json"`


# Expected behavior
For both commands the results of the crawling must be saved to `C:\path\to\output.json` file.

# Actual behavior
Case 1: error
```
Unrecognized output format '\path\to\output.json'. Set a supported one (('json', 'jsonlines', 'jsonl', 'jl', 'csv', 'xml', 'marshal', 'pickle')) after a colon at the end of the output URI (i.e. -o/-O <URI>:<FORMAT>) or as a file extension.
```

Case 2: no errors, but file is not created. **Note**: in this example I use system disk, although file is not created if any other disk is used (i.e., permissions in not the issue).

# Reproduces how often
Always

# Versions

```
Scrapy       : 2.9.0
lxml         : 4.9.2.0
libxml2      : 2.9.12
cssselect    : 1.2.0
parsel       : 1.8.1
w3lib        : 2.1.1
Twisted      : 22.10.0
Python       : 3.11.4 (tags/v3.11.4:d2340ef, Jun  7 2023, 05:45:37) [MSC v.1934 64 bit (AMD64)]
pyOpenSSL    : 23.2.0 (OpenSSL 3.1.1 30 May 2023)
cryptography : 41.0.1
Platform     : Windows-10-10.0.19044-SP0
```

Hints from issue discussion:

Commit message: fix: -O/-o option does not work with absolute paths on Windows

Patch to review:
diff --git a/scrapy/extensions/feedexport.py b/scrapy/extensions/feedexport.py
index 1cdc78f5915..d30b45682a9 100644
--- a/scrapy/extensions/feedexport.py
+++ b/scrapy/extensions/feedexport.py
@@ -9,7 +9,7 @@
 import sys
 import warnings
 from datetime import datetime
-from pathlib import Path
+from pathlib import Path, PureWindowsPath
 from tempfile import NamedTemporaryFile
 from typing import IO, Any, Callable, List, Optional, Tuple, Union
 from urllib.parse import unquote, urlparse
@@ -591,7 +591,7 @@ def _settings_are_valid(self):
 
     def _storage_supported(self, uri, feed_options):
         scheme = urlparse(uri).scheme
-        if scheme in self.storages:
+        if scheme in self.storages or PureWindowsPath(uri).drive:
             try:
                 self._get_storage(uri, feed_options)
                 return True
@@ -617,7 +617,7 @@ def _get_storage(self, uri, feed_options):
         It supports not passing the *feed_options* parameters to classes that
         do not support it, and issuing a deprecation warning instead.
         """
-        feedcls = self.storages[urlparse(uri).scheme]
+        feedcls = self.storages.get(urlparse(uri).scheme) or self.storages["file"]
         crawler = getattr(self, "crawler", None)
 
         def build_instance(builder, *preargs):
diff --git a/scrapy/utils/conf.py b/scrapy/utils/conf.py
index 3ade1d105b4..05d43e4567f 100644
--- a/scrapy/utils/conf.py
+++ b/scrapy/utils/conf.py
@@ -202,7 +202,8 @@ def check_valid_format(output_format):
     for element in output:
         try:
             feed_uri, feed_format = element.rsplit(":", 1)
-        except ValueError:
+            check_valid_format(feed_format)
+        except (ValueError, UsageError):
             feed_uri = element
             feed_format = Path(element).suffix.replace(".", "")
         else:
diff --git a/tests/test_commands.py b/tests/test_commands.py
index 014f50e92e5..478b82c3ccc 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -919,6 +919,63 @@ def start_requests(self):
         log = self.get_log(spider_code, args=args)
         self.assertIn("[myspider] DEBUG: FEEDS: {'stdout:': {'format': 'json'}}", log)
 
+    @skipIf(platform.system() == "Windows", reason="Linux only")
+    def test_absolute_path_linux(self):
+        spider_code = """
+import scrapy
+
+class MySpider(scrapy.Spider):
+    name = 'myspider'
+
+    start_urls = ["data:,"]
+
+    def parse(self, response):
+        yield {"hello": "world"}
+        """
+
+        args = ["-o", "/tmp/dump/output1.json:json"]
+        log = self.get_log(spider_code, args=args)
+        self.assertIn(
+            "[scrapy.extensions.feedexport] INFO: Stored json feed (1 items) in: /tmp/dump/output1.json",
+            log,
+        )
+
+        args = ["-o", "/tmp/dump/output2.json"]
+        log = self.get_log(spider_code, args=args)
+        self.assertIn(
+            "[scrapy.extensions.feedexport] INFO: Stored json feed (1 items) in: /tmp/dump/output2.json",
+            log,
+        )
+
+    @skipIf(platform.system() != "Windows", reason="Windows only")
+    def test_absolute_path_windows(self):
+        spider_code = """
+import scrapy
+
+class MySpider(scrapy.Spider):
+    name = 'myspider'
+
+    start_urls = ["data:,"]
+
+    def parse(self, response):
+        yield {"hello": "world"}
+        """
+        temp_dir = mkdtemp()
+
+        args = ["-o", f"{temp_dir}\\output1.json:json"]
+        log = self.get_log(spider_code, args=args)
+        self.assertIn(
+            f"[scrapy.extensions.feedexport] INFO: Stored json feed (1 items) in: {temp_dir}\\output1.json",
+            log,
+        )
+
+        args = ["-o", f"{temp_dir}\\output2.json"]
+        log = self.get_log(spider_code, args=args)
+        self.assertIn(
+            f"[scrapy.extensions.feedexport] INFO: Stored json feed (1 items) in: {temp_dir}\\output2.json",
+            log,
+        )
+
 
 @skipIf(platform.system() != "Windows", "Windows required for .pyw files")
 class WindowsRunSpiderCommandTest(RunSpiderCommandTest):


Code review comments:
diff_hunk: @@ -617,7 +617,7 @@ def _get_storage(self, uri, feed_options):
         It supports not passing the *feed_options* parameters to classes that
         do not support it, and issuing a deprecation warning instead.
         """
-        feedcls = self.storages[urlparse(uri).scheme]
+        feedcls = self.storages.get(urlparse(uri).scheme) or self.storages["file"]
path: scrapy/extensions/feedexport.py
text: @author:
Basically to avoid `KeyError` fallback to `file`

diff_hunk: @@ -919,6 +919,63 @@ def start_requests(self):
         log = self.get_log(spider_code, args=args)
         self.assertIn("[myspider] DEBUG: FEEDS: {'stdout:': {'format': 'json'}}", log)
 
+    @skipIf(platform.system() == "Windows", reason="Linux only")
+    def test_absolute_path_linux(self):
+        spider_code = """
+import scrapy
+
+class MySpider(scrapy.Spider):
+    name = 'myspider'
+
+    start_urls = ["data:,"]
+
+    def parse(self, response):
+        yield {"hello": "world"}
+        """
+
+        args = ["-o", "/tmp/dump/output1.json:json"]
path: tests/test_commands.py
text: @user1:
Can you please use mkdtemp() here too?

@author:
Done.

diff_hunk: @@ -617,7 +617,7 @@ def _get_storage(self, uri, feed_options):
         It supports not passing the *feed_options* parameters to classes that
         do not support it, and issuing a deprecation warning instead.
         """
-        feedcls = self.storages[urlparse(uri).scheme]
+        feedcls = self.storages.get(urlparse(uri).scheme) or self.storages["file"]
path: scrapy/extensions/feedexport.py
text: @user1:
```suggestion
        feedcls = self.storages.get(urlparse(uri).scheme, self.storages["file"])
```

(assuming it works that way)

@author:
I was wondering because i tried this before now i understand why did not work 🤡 

![image](https://github.com/scrapy/scrapy/assets/5853172/2b2f97e3-5905-4c00-9c8a-6bc00388efee)

Thanks @user1