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

Allow output files to another directory #11

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

chirsz-ever
Copy link

This is especially convenient for build a software package for package managers.

RosBE-Unix/Base-i386/RosBE-Builder.sh Outdated Show resolved Hide resolved
RosBE-Unix/Base-i386/RosBE-Builder.sh Outdated Show resolved Hide resolved
echo "and in this case, it will treat this directory as root path then perform an"
echo "unattended installation to this directory."
echo
echo "Usually, you just call the script without any parameters and it will guide you"
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Are the extraneous spaces sprinkled through the sentences made in order to give the appearance of the displayed text to be "justified" ?

Copy link
Author

Choose a reason for hiding this comment

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

Yes. Shoud it use another layout scheme?

@@ -171,13 +171,13 @@ echo "Using CXXFLAGS=\"$CXXFLAGS\""
echo

if $rs_process_cpucount; then
rs_do_command $CC -s -o "$rs_prefixdir/bin/cpucount" "$rs_scriptdir/tools/cpucount.c"
rs_do_command $CC $CFLAGS $LDFLAGS -s -o "$rs_prefixdir/bin/cpucount" "$rs_scriptdir/tools/cpucount.c"
Copy link
Member

Choose a reason for hiding this comment

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

CFLAGS and LDFLAGS are exported, so they should be picked up by the host compiler anyway.
Apart from that, you can't just pass LDFLAGS to the C compiler.

@ColinFinck
Copy link
Member

Hey @chirsz-ever! Thanks for your PR.
Commenting on the individual commits:

  • e06de4a
    CFLAGS and LDFLAGS are exported, so they should be picked up by the host compiler anyway.
    Apart from that, you can't just pass LDFLAGS to the C compiler.

  • e34f2cf
    I don't understand what we need two directories for instead of a single installation directory.
    RosBE-Unix is deliberately made simple and without many options to guarantee that all ReactOS developers have the same set of build tools in a similar directory structure. Two directory parameters would spoil that idea without any benefits from what I can see.
    Furthermore, any change here would need to be synchronized with the buildtoolchain-*.sh scripts of RosBE-Windows. We already spent a good amount of time to develop scripts that work for both RosBE-Windows and RosBE-Unix, and I'd say: Never change a running system.

  • a0add1e
    Nice one! I have imported that to RosBE-Unix 2.3 #15, as I've modified the affected lines there.

  • 7b89717
    I already did something similar in be82770

@chirsz-ever
Copy link
Author

I don't understand what we need two directories for instead of a single installation directory.

The two directories are opensource software building system's convention (in my opinion). Autotools has PREFIX and DESTDIR option, and Cmake also has CMAKE_INSTALL_PREFIX and DESTDIR.

This PR is originly for writing a arch package, and I found it need some untidy hack. The main problem is that the installdir written in the compiled binaries is incompatible with the real installed path. I think other package managers may meet with similar things, and this could be solved by add the distdir optional argument. The distdir argument is optional, so the default installation would not take any change.

e06de4a
a0add1e
7b89717

I will do a rebase to fix these problems after #15 being merged.

Another problem is that the repeated scripts is too much, including RosBE-Unix/Base-i386/RosBE-Builder-[amd64].sh and RosBE-Windows/Buildtoolchain/buildtoolchain-*. I think the best solution is to use autoconf or other similar tools to generate build scripts. Maybe this can't catch up with version 2.3?

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

Successfully merging this pull request may close these issues.

4 participants