Skip to content

Conversation

@crystalct
Copy link

No description provided.

Copy link
Member

@miigotu miigotu left a comment

Choose a reason for hiding this comment

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

@bucanero please review.

@bucanero
Copy link

bucanero commented Jun 20, 2020

@miigotu sure, I'll take a look on the PR and the syscalls.

about Cygwin, I don't have a windows environment so I can't really check those changes. Is there any Windows-dev around?

@crystalct , do you have more Cygwin changes to submit? if you have, perhaps it will be better to create a brach like cygwin-patch, and commit all the changes there, and keep it in a dedicated PR.

@miigotu
Copy link
Member

miigotu commented Jun 20, 2020

Yes, please keep changes separated. 1 for adding the syscall, another for changes like cygwin/Mac support.

You don't need to create a new pr to do it, you can force push to the branch you made this PR with.

Copy link

@bucanero bucanero left a comment

Choose a reason for hiding this comment

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

Here's my review. For utime(), please check the changes I suggested.

About cygwin changes, I don't have Windows so I couldn't test, but since it's limited in #ifdef __CYGWIN__ tags, it won't break the build in other platforms.

@miigotu
Copy link
Member

miigotu commented Jun 21, 2020

@crystalct please don't get discouraged and give up lol, we appreciate your contributions!

@crystalct
Copy link
Author

It was my first pull request :P
Next will be better ^_^
About cygwin, i work with it but 64bit version. After ps3 toolchain, psl1ght and ps3libraries installation, cgcomp does't work. Source code was correct so it was hard to understand the problem.
Cgcomp + cg.dll (64 bit) compiled source shaders using relative path but not full path sources.
That is a workaround. I should try to operate at 32bit (all intsallations of course) to see if the problem is the same.

@crystalct
Copy link
Author

Branch created for cygwin.
Utime modified as Bucanero tips.

Copy link

@bucanero bucanero left a comment

Choose a reason for hiding this comment

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

utime() looks good 👍

btw, good catch finding that sysLv2FsUtime() doesn't handle a NULL parameter as expected.

grazie @crystalct ! 😄

@miigotu miigotu merged commit a768d74 into ps3dev:master Jun 21, 2020
@crystalct
Copy link
Author

About cygwin.. i confirm 32bit version has the same problem. In addition i discover that gcc inside a cygwin installation doesn't have WIN32 defined,bacause it's POSIX. So if in sources code is necessary a #ifdef WIN32, with cygwin it's not true.

@crystalct
Copy link
Author

@bucanero, access still old not working version.

@bucanero
Copy link

@bucanero, access still old not working version.

Right, I've just submitted a fix based on your feedback #86 👍

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.

3 participants