Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ability to set the TYPE/CREATOR resource fork attributes of file(s) inside newly-created cd-rom ISO images of type HFS #1377

Merged
merged 9 commits into from
Nov 19, 2023

Conversation

i-to-z
Copy link
Contributor

@i-to-z i-to-z commented Nov 18, 2023

See #1376

…e(s) inside newly-created cd-rom ISO images of type HFS
#
# The mapping entries below are derived from
# https://github.com/Netatalk/netatalk/blob/branch-netatalk-2-3/config/AppleVolumes.system
# , which is under a GNU GPL license.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this particular line about GNU GPL, since it may imply the mapping entries fall under this license. The PiSCSI codebase is distributed under BSD-3-Clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# See more at https://linux.die.net/man/1/genisoimage
iso_args = ["-hfs", "-map", str(genisoimage_hfs_resource_fork_map_file_path)]
logging.info(
"Found and using the genisoimage hfs map file at %s .",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Why is there two spaces between %s and .? ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@rdmark
Copy link
Member

rdmark commented Nov 19, 2023

It's bizarre that the backend_tests job is failing here.

 > [web 12/18] RUN ./easyinstall.sh --run_choice=12     && sudo apt-get remove build-essential --yes     && sudo apt autoremove -y     && sudo apt-get clean     && sudo rm -rf /var/lib/apt/lists/*:
0.183 /bin/sh: 1: ./easyinstall.sh: Permission denied

I wonder if the permissions model changed for branches originating from forks...?

@i-to-z
Copy link
Contributor Author

i-to-z commented Nov 19, 2023

Regrading
./easyinstall.sh: Permission denied : the file was no-longer executable. I fixed it. Perhaps I messed that up when reverting my changes to the file....

@rdmark
Copy link
Member

rdmark commented Nov 19, 2023

Oh yes, that was it. Nice catch.

I also ran a test from my own fork which passed: #1382

Copy link

sonarcloud bot commented Nov 19, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@rdmark
Copy link
Member

rdmark commented Nov 19, 2023

Regrading ./easyinstall.sh: Permission denied : the file was no-longer executable. I fixed it. Perhaps I messed that up when reverting my changes to the file....

These days once I've locked down a changeset in a PR after a few roundtrips of changes, I like to sqash the commits in the PR branch locally and then force-push a "clean" commit (or commits). It helps avoid unwanted changes slipping through.

@rdmark rdmark merged commit 287b9d7 into PiSCSI:develop Nov 19, 2023
6 of 7 checks passed
rdmark pushed a commit that referenced this pull request May 1, 2024
…e(s) inside newly-created cd-rom ISO images of type HFS (#1377)

* Added ability to set the TYPE/CREATOR resource fork attributes of file(s) inside newly-created cd-rom ISO images of type HFS

* Added file genisoimage_hfs_resource_fork_map.txt under python/web and modified web.py to find it at that location
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.

2 participants