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

Use target-prefixed executables when cross-compiling #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Skirmisher
Copy link

This adds some logic to determine if cross-compilation is occurring, and picks the right prefix to use depending on the target, similar to what the cc crate does. It also reads the environment to allow the executable to be overridden by the user, rather than just the library author.

I removed the conditional .exe suffix to make things simpler, since Windows doesn't need it when running from PATH.

Also allow the executable prefix or path to be overridden by the environment.
@BenjaminRi
Copy link

@Skirmisher This bug is now fixed in v0.1.13 of my fork

@Skirmisher
Copy link
Author

@BenjaminRi Thanks for letting me know! However, the commits on your fork have lost the authorship info from other contributors (e.g. myself and @Nilstrieb). Could you please fix this (preferably by rebasing appropriately) and force-update your repo? I can provide assistance if necessary.

@BenjaminRi
Copy link

@Skirmisher You are correct in that they lost their authorship. I didn't fork the winres repository, I reuploaded it. The reason why I did this is because GitHub does not allow searching forks (presumably to save computing resources on their servers). But I do want the fork to be searchable. I then manually patched the changes into the repository because the patches were very small and this was the quickest way to do it. Is the attribution important to you? It's literally a couple of lines of code. I didn't expect anyone to be bothered by this. I didn't intend to claim authorship over your patches, it was just the fastest way to streamline the improvements you people made.

@Skirmisher
Copy link
Author

Skirmisher commented Dec 9, 2022

@BenjaminRi Whether or not you used GitHub's "fork" button doesn't matter—rebasing or merging commits between branches, whether they belong to the same repo or different ones, is just part of the standard git workflow. I appreciate your intention; however, you should be preserving commits as a general rule, including contributor authorship (regardless of how many lines they are responsible for), unless the authors have indicated otherwise. Failure to do so is very disrespectful, makes it more difficult to understand the code's history in the future, and can even present legal issues; not to mention that copying changes around outside of a git context is error-prone. If you intend for your fork to be the new "source of truth" for an unmaintained project, then it is especially good practice to keep its commit history as clean as possible, for the sake of longevity and posterity.

To merge others' changes into your fork, start with the original master branch checked out, then invoke this command:

git remote add -f -t <name-of-branch-with-changes> <name-of-new-remote> <other-repo-url>

This will add the other fork as a remote to your local repository, and then fetch the branch with the commits you want right afterward. Then, just run

git merge <name-of-new-remote>/<name-of-branch-with-changes>

to immediately make a merge commit pulling in the remote branch's changes (after editing the merge commit's message).

If you want to edit the changes, run git rebase -i and follow the instructions to select the commits to modify and make your edits. However, I would recommend you instead create new commits if you are changing something more substantial than version numbers or whitespace. You can rebase and/or cherry-pick your existing personally-authored commits onto the merged ones if you keep them on a separate branch from the original repo's HEAD.

@BenjaminRi
Copy link

@Skirmisher I'll look into it when I have time. I primarily care about the function of the crate, and this is what I will prioritize. However, I do recognize that organizational details can matter as well.

@BenjaminRi
Copy link

BenjaminRi commented Feb 12, 2023

@Skirmisher I put it right and added your original commit into the history, as you asked (BenjaminRi/winresource@44068cf). Your name will forever be embedded in my humble fork, granting you negligible amounts of fame and recognition. I hope I now corrected my mistake to your satisfaction. Thank you for your contribution, I appreciate it.

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