IsaacSchemm / MultiCD

A shell script package for creating combination disks from Linux ISOs.
http://multicd.us
213 stars 47 forks source link

Added build directory #57

Closed stheno closed 7 years ago

stheno commented 7 years ago

Added ability to have differentiation between different desktop arch and versions allowing multiples on one iso. Dropped server implementation from previous PR. Will look at that later.

IsaacSchemm commented 7 years ago

In the EDITS section, why do you have \t instead of \t? I'm ending up with actual "\t"s in my file. When I change it to \t in EDITS, I get tabs and it works.

Other than that, it looks good. I'll make the changes to the -o option in another branch first, and then merge this one when I'm done.

IsaacSchemm commented 7 years ago

The plugin still seems to pick up on the server ISO and add a menu entry for it that doesn't go anywhere. Could you look into that?

The "back to main menu" link is also not working, but I think that would be an easy fix.

stheno commented 7 years ago

Yep, I just noticed it and have been working on that. Weird on the tab issue. I have not run across that issue. What do you mean "\t instead of \t" I'll try to duplicate it before I commit this last fix.

Just love when odd bugs show up or are noticed AFTER pushing a PR lol

stheno commented 7 years ago

Ok I was not able to duplicate what so ever the issue with tabs. Server bit fixed tho.

IsaacSchemm commented 7 years ago

I was testing it on Cygwin. That could be part of the issue.

On Jun 9, 2017 5:15 PM, "stheno" notifications@github.com wrote:

Ok I was not able to duplicate what so ever the issue with tabs. Server bit fixed tho.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/IsaacSchemm/MultiCD/pull/57#issuecomment-307512480, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZGK-dTGZ6lvRE-Sdw5W-OmFlvOzjzVks5sCcQWgaJpZM4N1t6U .

stheno commented 7 years ago

Ah yea. I am testing from vbox

IsaacSchemm commented 7 years ago

I found the root of the tabs issue. Cygwin's default /bin/sh is bash, which exhibits the problem I described; Ubuntu's is dash, which doesn't. I'm thinking I'll just avoid the issue by replacing those tabs with spaces, if that's OK with you.

stheno commented 7 years ago

Sounds good, I was thinking that may need to happen for compatibility across systems.

stheno commented 7 years ago

I don't know anything about cygwin. but can it be forced to use dash? Maybe avoid weird cross language issues in general?

IsaacSchemm commented 7 years ago

Cygwin does come with dash (/usr/bin/dash), so just changing the first line of ubuntu.sh should fix it. However, this will make the plugin not work if dash isn't installed. I'm not sure how widespread dash is - e.g. whether Fedora or FreeBSD come with it.

stheno commented 7 years ago

good point. Spaces it is :-) I'll put those in my PR

stheno commented 7 years ago

Ok fixed spaces. All should be good to go now?

stheno commented 7 years ago

I may as well ask this then... Why use bin/sh instead of bin/bash in the files then? I was actually trying to make sure it was dash compat thinking sh meant dash not bash Genuine question as I have not looked through all the code of the whole project to see if that would be answered by looking.

IsaacSchemm commented 7 years ago

Most of the code works in either one. I think this is the first time I've found something that works in dash but not in bash.

stheno wrote:

I may as well ask this then... Why use bin/sh instead of bin/bash in the files then? I was actually trying to make sure it was dash compat thinking sh meant dash not bash Genuine question as I have not looked through all the code of the whole project to see if that would be answered by looking.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/IsaacSchemm/MultiCD/pull/57#issuecomment-307586513, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZGKzPGa6YLTgBw24wYivPILFRW_ajXks5sCvNZgaJpZM4N1t6U.

stheno commented 7 years ago

Once you approve and merge this I have a iso downloader I am working on too