Skip to content

Conversation

@amano-kenji
Copy link

@amano-kenji amano-kenji commented Dec 2, 2025

Where parent directory creation is needed, shutil/create-dirs is called before os/mkdir. Otherwise, jpm may crash with errors.

I tested this pull request against spork on my gentoo system. It works.

It fixes #104.

Where parent directory creation is needed,
shutil/create-dirs is called before os/mkdir. Otherwise, jpm may
crash with errors.
@amano-kenji amano-kenji changed the title Create directories recursively. Create parent directories. Dec 2, 2025
@sogaiu
Copy link

sogaiu commented Dec 2, 2025

Not sure who's responsibility it should be to make sure all of the directories have been created.

In any case, if they are going to be created, may be there should be attempts for the following locations too?

It's true that in the latter case, download-bundle is the only caller of download-tar-bundle ATM and it tries to create the parent directory.

However, download-tar-bundle is public so presumably there could be other callers in the future or if someone wants to use parts of jpm in some other manner.

@amano-kenji
Copy link
Author

amano-kenji commented Dec 2, 2025

  • It seems that (os/mkdir (find-build-dir)) above (create-dirs metaname) is redundant because metaname is in build-dir. metaname is derived from lname. metaname is build-dir/spork/json.meta.janet if lname is build-dir/spork/json.so
  • I was trying to minimize the number of create-dirs call sites because create-dirs may slow down jpm a bit. That's why I didn't insert create-dirs in download-tar-bundle. I haven't yet figured out what to do with download-tar-bundle and download-git-bundle. Should I call create-dirs in download-git-bundle and download-tar-bundle because they are in public API?

(create-dirs metaname) makes an above os/mkdir call redundant.
@sogaiu
Copy link

sogaiu commented Dec 2, 2025

Thanks for your thoughts.

I think even if some of these bits are redundant now, they might not be later if the code changes.

I read the insertion of create-dirs calls as having the intent of "please make sure the needed directories exist".

The attempt at optimizing based on earlier parts of functions that have definitions that extend beyond a single screen [1] seems like a potentially fragile thing to me.

I doubt the extra calls to create-dirs would amount to much in the way of slow-downs, but that's just a guess. Measurements seem like a better way to settle that question if it's really something of concern.

In any case, if the current state of the PR helps to solve your situation, may be that's really the main thing that counts.


[1] It is much harder to hold the entirety of the definition in one's head compared to if you can see the whole thing on the screen and consequently my sense is that one's processing of the content could be adversely affected. For reference, declare-native is over 100 lines long, I don't typically see even 60 lines on my screen and even if I could I think it would be a strain to perceive and think about.

@amano-kenji
Copy link
Author

I guess download-git-bundle and download-tar-bundle shouldn't be in public API. I will wait for bakpakin.

@sogaiu
Copy link

sogaiu commented Dec 2, 2025

Waiting for comments sounds fine, but at this point, they have already been public for quite some time [1], I'm not sure it's a great idea to make them private at this point from the perspective of preventing breakage.

Also, the two functions are advertised at the janet-lang.org site:


[1] That looks like over 4 years to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

jpm fails to install spork/json and other native modules.

2 participants