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

--with-build-dir= don't find src files #4342

Open
M4iKZ opened this issue Sep 9, 2024 · 17 comments · May be fixed by #4245
Open

--with-build-dir= don't find src files #4342

M4iKZ opened this issue Sep 9, 2024 · 17 comments · May be fixed by #4245
Assignees
Milestone

Comments

@M4iKZ
Copy link

M4iKZ commented Sep 9, 2024

Hello,
I'm trying to move the build folder in another path, but makefile (linux) or ninja (windows) doesn't find the src files:

for example from my makefile:
/mnt/m/botan/build/linux/build/obj/lib/base_buf_comp.o: src/lib/base/buf_comp.cpp $(CXX) $(LIB_FLAGS) $(BUILD_FLAGS) -DBOTAN_IS_BEING_BUILT -I

the error I get on make (same on ninja):
make: *** No rule to make target 'src/lib/base/buf_comp.cpp', needed by '/mnt/m/botan/build/linux/build/obj/lib/base_buf_comp.o'.

this is my command for linux, on windows it's the same with platform and folder specifics:
python3 configure.py --disable-deprecated-features --no-install-python-module --disable-shared-library --optimize-for-size --prefix=/mnt/m/botan-linux/release --build-targets=static --without-documentation --with-build-dir=/mnt/m/botan/build/linux --minimized-build

should I add something?

@M4iKZ
Copy link
Author

M4iKZ commented Sep 9, 2024

I added two lines into config file and now I can compile, but install fail because it doesn't find the script:

at line 1935 after for (obj_file, src) in zip(objects, sources):
src = os.getcwd() + os.sep + src

and at line 2122 before variables definition:
source_paths.src_dir = os.getcwd() + os.sep + source_paths.src_dir

now I'm investigating how to fix the install script location

@reneme
Copy link
Collaborator

reneme commented Sep 9, 2024

Perhaps this open pull request is helpful: #4245

@M4iKZ
Copy link
Author

M4iKZ commented Sep 9, 2024

Perhaps this open pull request is helpful: #4245

I tried it, instead of configure.py it takes all path, but fail to exec the script entirely on windows

@reneme
Copy link
Collaborator

reneme commented Sep 10, 2024

Could you be a bit more specific, please?

I'm assuming you applied the patch in #4245. Then, it works for you on Linux but the script's execution fails on Windows entirely?
Or did you just try the $(pwd)/configure.py that is mentioned in the pull request's description?

@reneme reneme self-assigned this Sep 10, 2024
@M4iKZ
Copy link
Author

M4iKZ commented Sep 10, 2024

Could you be a bit more specific, please?

I'm assuming you applied the patch in #4245. Then, it works for you on Linux but the script's execution fails on Windows entirely? Or did you just try the $(pwd)/configure.py that is mentioned in the pull request's description?

I cloned again the repo because I saw a new update and now with that patch it compile without my edits
(didn't tested on linux yet)

but then fail the ninja install:

ninja: fatal: CreateProcess: is the command line too long?

because takes all obj with the full path...

@M4iKZ
Copy link
Author

M4iKZ commented Sep 10, 2024

On windows I fixed with brute edit at line 299 on def src_info(self, typ)
return (self.lib_sources, self.libobj_dir)
to
return (self.lib_sources, ""obj" + os.sep + "lib"")

update, maybe I can remove the prefix from it, but I must do it in that function and not before so the script create the right path for the make and ninja

@reneme
Copy link
Collaborator

reneme commented Sep 10, 2024

but then fail the ninja install: [...]
because takes all obj with the full path...

Oh no. I'm guessing the actual "install" isn't what is actually failing here, right? But rather the link command for the static and/or shared library. Hence, ninja libs probably fails as well, correct?

@reneme
Copy link
Collaborator

reneme commented Sep 10, 2024

As a side note: there might be a workaround that lets you build out-of-source without the patch in #4245 for the time being:
If you create your build directory and just run configure.py from there without specifying --with-build-dir= (similarly as you would do with a cmake-based build system), that should work too.

Like so:

git clone https://github.com/randombit/botan my_botan_checkout
mkdir my_botan_build_dir
cd my_botan_build_dir
../my_botan_checkout/configure.py --build-tool=ninja
ninja

... regardless, we'll have to look into the issue with the "too long command line".

@M4iKZ
Copy link
Author

M4iKZ commented Sep 10, 2024

I'll try for sure, on linux my edit doesn't work because make don't find anymore the obj
on windows I fixed with my edits, but still it's only a workaround, where is compiled the install command line?
so the full path could be removed there instead

@M4iKZ
Copy link
Author

M4iKZ commented Sep 10, 2024

but then fail the ninja install: [...]
because takes all obj with the full path...

Oh no. I'm guessing the actual "install" isn't what is actually failing here, right? But rather the link command for the static and/or shared library. Hence, ninja libs probably fails as well, correct?

Yes, I confirm that also command libs doesn't work.
It's because obj keep the full path in the command, creating a very long command that ninja doesn't like at all.

the solution I found on windows it's remove the path and it works, but that edit then doesn't work on linux

@M4iKZ
Copy link
Author

M4iKZ commented Sep 10, 2024

on linux only with the patch #4245 works fine, even tho the command is huge too, but maybe I got an idea to test out

@reneme
Copy link
Collaborator

reneme commented Sep 10, 2024

Yes, I confirm that also command libs doesn't work.

Thanks!

on linux only with the patch #4245 works fine, even tho the command is huge to

My best guess is that Windows has an upper limit on its command length and Linux doesn't. Or at least, Linux' limit is much bigger 😞 I'll look into a way to fix this.

@reneme reneme linked a pull request Sep 10, 2024 that will close this issue
@M4iKZ
Copy link
Author

M4iKZ commented Sep 10, 2024

I don't know if you will like this solution with the edit at line 299, but with patch #4245 it works both on linux and windows:

def src_info(self, typ):
    buildlen = len(self.build_dir) - 5 # keep build/
    
    if typ == 'lib':
        libobj_dir = self.libobj_dir
        if libobj_dir.startswith(self.build_dir):
            libobj_dir = libobj_dir[buildlen:]
        
        return (self.lib_sources, libobj_dir)
    if typ == 'cli':
        cliobj_dir = self.cliobj_dir
        if cliobj_dir.startswith(self.build_dir):
            cliobj_dir = cliobj_dir[buildlen:]

        return (self.cli_sources, self.cliobj_dir)
    if typ == 'test':
        testobj_dir = self.testobj_dir
        if testobj_dir.startswith(self.build_dir):
            testobj_dir = testobj_dir[buildlen:]

        return (self.test_sources, self.testobj_dir)
    if typ == 'fuzzer':
        fuzzobj_dir = self.fuzzobj_dir
        if fuzzobj_dir:
            if fuzzobj_dir.startswith(self.build_dir):
                fuzzobj_dir = fuzzobj_dir[buildlen:]

        return (self.fuzzer_sources, self.fuzzobj_dir)
    if typ == 'examples':
        example_obj_dir = self.example_obj_dir
        if example_obj_dir:
            if example_obj_dir.startswith(self.build_dir):
                example_obj_dir = example_obj_dir[buildlen:]

        return (self.example_sources, self.example_obj_dir)
    raise InternalError("Unknown src info type '%s'" % (typ))

@reneme
Copy link
Collaborator

reneme commented Sep 10, 2024

Thanks for sharing your patch. I'll look into it before the Botan 3.6.0 release. (I'm assuming you're unblocked for now and this isn't super time critical at this point.)

@reneme reneme added this to the Botan 3.6.0 milestone Sep 10, 2024
@M4iKZ
Copy link
Author

M4iKZ commented Sep 10, 2024

I fixed for my needs, I hope could help to fix out for the ninja "problems"

@M4iKZ
Copy link
Author

M4iKZ commented Sep 11, 2024

I tested the edit I did also with the default build dir "build" and because I keep the folder also with longer build path string, I can compile on both linux and windows without problems

@reneme
Copy link
Collaborator

reneme commented Oct 2, 2024

@M4iKZ In #4350 I introduced support for "response files" in our Ninja builds. If you combine the patches in #4245 and #4350 If you check out #4245 after I rebased onto latest master, you should be able to build on Windows (using Ninja) without the issue you pointed out earlier. This currently applies to the Ninja build only, as Makefiles don't have explicit support for "response files". I'm hoping that this is enough of a workaround for your use case.

@randombit randombit modified the milestones: Botan 3.6.0, Botan 3.7.0 Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants