Fix dataoff attribute access on ext_fnhdr

[ila-embsys]

Issue discription

Just discovered on some of my OBS project the error while building:

[18/18] (openSUSE:Tools) obs-service-tar-scm_0.10.43_all.deb: 100%|################################################################################################| Time: 0:00:00
Traceback (most recent call last):
  File "/usr/bin/osc", line 33, in <module>
    sys.exit(load_entry_point('osc==1.8.3', 'console_scripts', 'osc')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/babysitter.py", line 227, in main
    sys.exit(run(commandline.OscMainCommand()))
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/babysitter.py", line 70, in run
    prg.main(argv)
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 565, in main
    exit_code = cmd.run(args)
                ^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 249, in run
    return cmd.run(args)
           ^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 509, in run
    return self.func(args.command, args, *args.positional_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 7477, in do_build
    return osc_build.main(self.get_api_url(), store, opts, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/build.py", line 1194, in main
    fetcher.run(bi)
  File "/usr/lib/python3.11/site-packages/osc/fetch.py", line 267, in run
    self.fetch(i, prefix=prefix)
  File "/usr/lib/python3.11/site-packages/osc/fetch.py", line 146, in fetch
    self.move_package(tmpfile.name, pac.localdir, pac)
  File "/usr/lib/python3.11/site-packages/osc/fetch.py", line 168, in move_package
    pkgq = packagequery.PackageQuery.query(tmpfile, extra_rpmtags=(1044, 1051, 1052))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/util/packagequery.py", line 89, in query
    pkgqueryresult = pkgquery.read(all_tags, self_provides, *extra_tags)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/util/debquery.py", line 46, in read
    arfile.read()
  File "/usr/lib/python3.11/site-packages/osc/util/ar.py", line 203, in read
    self._fixupFilenames()
  File "/usr/lib/python3.11/site-packages/osc/util/ar.py", line 150, in _fixupFilenames
    self.__file.seek(self.ext_fnhdr.dataoff, os.SEEK_SET)
                     ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'dataoff'

Attempt to fix

I quickly looked through the code and noticed that the last commits probably introduce a broken logic:

The dataoff attribute of ext_fnhdr can be existent only after calling the self._appendHdr() function. However, this function internally have a note that it happens only in some conditions while handling something that have 'very log filenames'. So ext_fnhdr could be None after all.

def _appendHdr(self, hdr):
        # GNU uses an internal '//' file to store very long filenames
        if hdr.file.startswith(b'//'):
            self.ext_fnhdr = hdr
        else:
            self.hdrs.append(hdr)

The commit 'd61b781' probably introduces an optimisation to read ext_fnhdr_data once and beforehand before the next while loop will be repeatedly accessing it. However, in some cases as noted in self._appendHdr() the ext_fnhdr could be None, so I added here a condition to prepare ext_fnhdr_data only if ext_fnhdr is not None.

The code that access ext_fnhdr_data is assumed to be only for 'very long filenames' handling, so if no 'very long filenames' that no attempt to access to ext_fnhdr_data. So it is probably safe to leave ext_fnhdr_data as None.

The assertion error

After patching the code to pass previously described issue the next assertion check is occurs:

fetching packages for 'Ubuntu:24.04': | Elapsed Time: 0:01:04                                                                                                                     
Traceback (most recent call last):
  File "/usr/bin/osc", line 33, in <module>
    sys.exit(load_entry_point('osc==1.8.3', 'console_scripts', 'osc')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/babysitter.py", line 227, in main
    sys.exit(run(commandline.OscMainCommand()))
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/babysitter.py", line 70, in run
    prg.main(argv)
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 565, in main
    exit_code = cmd.run(args)
                ^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 249, in run
    return cmd.run(args)
           ^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 509, in run
    return self.func(args.command, args, *args.positional_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/commandline.py", line 7477, in do_build
    return osc_build.main(self.get_api_url(), store, opts, args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/build.py", line 1194, in main
    fetcher.run(bi)
  File "/usr/lib/python3.11/site-packages/osc/fetch.py", line 289, in run
    self.__fetch_cpio(buildinfo.apiurl)
  File "/usr/lib/python3.11/site-packages/osc/fetch.py", line 127, in __fetch_cpio
    self.__download_cpio_archive(apiurl, project, repo, arch, package, **pkgs)
  File "/usr/lib/python3.11/site-packages/osc/fetch.py", line 96, in __download_cpio_archive
    self.move_package(tmpfile, pac.localdir, pac)
  File "/usr/lib/python3.11/site-packages/osc/fetch.py", line 168, in move_package
    pkgq = packagequery.PackageQuery.query(tmpfile, extra_rpmtags=(1044, 1051, 1052))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/util/packagequery.py", line 89, in query
    pkgqueryresult = pkgquery.read(all_tags, self_provides, *extra_tags)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/osc/util/debquery.py", line 46, in read
    arfile.read()
  File "/usr/lib/python3.11/site-packages/osc/util/ar.py", line 206, in read
    self._fixupFilenames()
  File "/usr/lib/python3.11/site-packages/osc/util/ar.py", line 165, in _fixupFilenames
    assert h.file[0:1] == b"/"
           ^^^^^^^^^^^^^^^^^^^
AssertionError

I guess the assertion is checking the condition that the next code actually will handle the 'very long filenames'. So if the check is not pass that I think some guard is missing.

# long file name
assert h.file[0:1] == b"/"

Previous code had the condition to continue while loop that probably prevented handling the very long filenames if they actually are not.

if not h.file.startswith(b'/'):
    continue

So I just restored it and everything becomes works fine.

I didn't go deep into the task's domain but just handled errors. So please check the suggestion to merge carefully.

[dmach]

Hi, thanks for the patch.
It looks good to me, I'm only not sure about the Restore start symbol check commit.
Wouldn't be better to remove it and let the code fail on the following asserts to avoid masking a problem?

I've also updated the PR by pushing a commit with a test.

[ila-embsys]

It looks good to me, I'm only not sure about the Restore start symbol check commit.
Wouldn't be better to remove it and let the code fail on the following asserts to avoid masking a problem?

I am not sure what the asserts checks, but some builds are failing because of them. Does it mean that some fetched packages that are processing are malformed? What the problem the commit 'Restore start symbol check' masks?

[dmach]

It looks good to me, I'm only not sure about the Restore start symbol check commit.
Wouldn't be better to remove it and let the code fail on the following asserts to avoid masking a problem?

I am not sure what the asserts checks, but some builds are failing because of them. Does it mean that some fetched packages that are processing are malformed? What the problem the commit 'Restore start symbol check' masks?

if not h.file.startswith(b'/'):
    continue

This skips to the next header and the following assert never triggers:

# long file name
assert h.file[0:1] == b"/"

[dmach]

Anyway, the code does what it's supposed to do, merging.