constructor icon indicating copy to clipboard operation
constructor copied to clipboard

fix cache directory creation

Open jaimergp opened this issue 1 year ago • 7 comments

Description

Closes #411

Checklist - did you ...

  • [x] Add a file to the news directory (using the template) for the next release's release notes?
  • [ ] Add / update necessary tests?
  • [ ] Add / update outdated documentation?

jaimergp avatar Aug 15 '22 11:08 jaimergp

@jaimergp is this one ready?

larsoner avatar Aug 15 '22 16:08 larsoner

I cannot reproduce the original issue, so no clue. Hoping @hoechenberger can take a look :D

jaimergp avatar Aug 15 '22 17:08 jaimergp

Sorry, I cannot test this at the moment :( but the changes look good to me

hoechenberger avatar Aug 15 '22 18:08 hoechenberger

@phyordia Can you take a look here? It would help a lot if you could confirm whether this fixes the bug you found! Thanks!

jaimergp avatar Aug 16 '22 09:08 jaimergp

@jaimergp Thanks for the PR, but I don't think it fixes the bug, only makes it more interesting: now it sets set setuid bit correctly, but the "not writable" error is still there.

same setup as described in #411 , but using the latest miniconda and on a M1 machine now instead.

(base) [redacted] % constructor   --cache-dir cache --output-dir dist .
platform: osx-arm64
MKDIR SAFE!!!  <-- @phyordia added this to the PR code just to make sure it was going through the changes. 
Traceback (most recent call last):
  File "[redacted]/miniconda3/bin/constructor", line 8, in <module>
    sys.exit(main())
  File "[redacted]/miniconda3/lib/python3.9/site-packages/constructor/main.py", line 272, in main
    main_build(dir_path, output_dir=out_dir, platform=args.platform,
  File "[redacted]/miniconda3/lib/python3.9/site-packages/constructor/main.py", line 136, in main_build
    fcp_main(info, verbose=verbose, dry_run=dry_run, conda_exe=conda_exe)
  File "/[redacted]/miniconda3/lib/python3.9/site-packages/constructor/fcp.py", line 460, in main
    has_conda, extra_envs_info) = _main(
  File "[redacted]/miniconda3/lib/python3.9/site-packages/constructor/fcp.py", line 401, in _main
    pc_recs, _urls, dists, has_conda = _fetch_precs(
  File "[redacted]/miniconda3/lib/python3.9/site-packages/constructor/fcp.py", line 334, in _fetch_precs
    pc_recs = _fetch(download_dir, precs)
  File "[redacted]/miniconda3/lib/python3.9/site-packages/constructor/fcp.py", line 113, in _fetch
    assert pc.is_writable, download_dir + " does not exist or is not writable"
AssertionError: [redacted]/constructor_test/cache/osx-arm64 does not exist or is not writable

sets the last "x" correctly:

(base) [redacted] % ls -lha | grep cache
drwxrwsr-x   3 [redacted]    96B Aug 16 09:19 cache 
(base) [redacted] % ls -lha cache
drwxrwsr-x  2 [redacted]   64B Aug 16 09:19 osx-arm64

If I pull out the _find_out_of_date_precs() outside of the if verbose: in fcp.py line 320, it works just fine!:

    precs = exclude_packages(precs, exclude, error_on_absence=not extra_env)
    more_recent_versions = _find_out_of_date_precs(precs, channel_urls, platform)
    if verbose:
        _show(name, version, platform, download_dir, precs, more_recent_versions)

I didn't have much time yet to explore this rabbit hole, but definitely something downstream from _find_out_of_date_precs() makes it work.

phyordia avatar Aug 16 '22 13:08 phyordia

Thanks @phyordia! Well, I think it's still due to that SubdirData.query_all function, which relies on create_cache_dir. The other call in that function is to PackageCacheData.first_writable(), which also has directory-creating side effects :D

Funnily enough, the classmethod ends up calling the sudo-safe mkdir thingy, so we'll see if we had missed something. Pushing a new commit in a minute. Could you try please?

jaimergp avatar Aug 16 '22 14:08 jaimergp

@jaimergp actually the (or at least a possible) fix is quite simple(r). I'll submit a PR soon.

phyordia avatar Aug 16 '22 14:08 phyordia