AESCrypt icon indicating copy to clipboard operation
AESCrypt copied to clipboard

Add LDFLAGS to Makefile

Open 2011 opened this issue 3 years ago • 8 comments

Somewhere between releases 3.0.6b and 3.14 aescrypt compiled from the Linux source downloaded from https://www.aescrypt.com/download/v3/linux/aescrypt-3.14.tgz stopped honoring LDFLAGS.

This small patch fixes the problem:

--- Makefile  2018-10-21 00:27:35.000000000 +0000
+++ Makefile  2021-07-20 00:00:00.000000000 +0000
@@ -27,13 +27,13 @@
 all: aescrypt aescrypt_keygen
 
 aescrypt: $(AESCRYPT_OBJS)
-       $(CC) $(CFLAGS) $(LIBS) -o $@ $(AESCRYPT_OBJS)
+       $(CC) $(CFLAGS) $(LIBS) $(LDFLAGS) -o $@ $(AESCRYPT_OBJS)
 
 aescrypt_keygen: $(KEYGEN_OBJS)
-       $(CC) $(CFLAGS) $(LIBS) -o $@ $(KEYGEN_OBJS)
+       $(CC) $(CFLAGS) $(LIBS) $(LDFLAGS) -o $@ $(KEYGEN_OBJS)
 
 %.o: %.c %.h
-       $(CC) $(CFLAGS) -c $*.c
+       $(CC) $(CFLAGS) $(LDFLAGS) -c $*.c
 
 install: aescrypt
        install -o root -g root -m 755 aescrypt /usr/bin

I don't really understand how code flows between this repository and the web site, but I hope you can include this in future releases. :) Thank you.

2011 avatar Jul 21 '21 06:07 2011

The source started out the same, but the version on Github has had some superficial changes made to the code (e.g., some folks did not like the new // style comments), support for autotools (configure / make), etc. If there are bug fixes, I fix it in both places, but bugs are rare. I consider the code on aescrypt.com to be the authoritative code, since I do take care to ensure nothing unseen slips through. GitHub might have some stuff I did not carefully review.

It's not a big deal keeping both, since I rarely change the code (with good reason). (One day, I will create a new version since there are several features people have requested and things I want. I'll probably create a new repo for it at that point, likely several repos since the various platforms and common libraries really demands use of a better build system like cmake where I can better split common logic, pull in common libraries, etc. (Maybe you cringe at cmake, but I've found it to be pretty awesome for doing what I have in mind.)

Anyway, to your specific issue, why do you want LDFLAGS? We're not using any linker flags in the source on aescrypt.com. If I added it, it would have no effect, unless there's something you want to set before running make or something. Also, if it was added, I think it needs to go at the end. If memory serves, linker flags need to be tacked on the end, else things do not build properly. It might be just certain platforms, but I know for sure I've had issues before. And they ought not be needed in the compilation step, just the steps where the executables are linked. Still, I'm not favorable to adding it since it's not actually being used.

paulej avatar Jul 21 '21 17:07 paulej

Anyway, to your specific issue, why do you want LDFLAGS? We're not using any linker flags in the source on aescrypt.com. If I added it, it would have no effect, unless there's something you want to set before running make or something. Also, if it was added, I think it needs to go at the end. If memory serves, linker flags need to be tacked on the end, else things do not build properly. It might be just certain platforms, but I know for sure I've had issues before. And they ought not be needed in the compilation step, just the steps where the executables are linked. Still, I'm not favorable to adding it since it's not actually being used.

Thank you for your reply. I use gentoo, and saw a report in their bugzilla recently about aescrypt not respecting LDFLAGS. As you probably know, gentoo compiles virtually all packages from source, and thus depends on scripts, automation, and environment variables to do that with minimal user intervention. I don't write software, so I take absolutely no position on build systems - I only want something that people can use, and possbibly tweak. The "-static" option gets passed via LDFLAGS (if I understand, and I quite likely don't, you could make a configure script as another way to pass that option, but that, of course, adds complexity). I actually need a static aescrypt in my initramfs just to boot my system, so this did slow me down a bit when I encountered it. :)

2011 avatar Jul 25 '21 07:07 2011

Modifying the Makefile to have that is easy, but there's the risk it might not work as expected. For example, what if I set it to some value in Makefile? I would generally do that. So, if Gentoo wanted to pass something in, it would get reset.

Another issue is placement. You suggested having it some place that's not the last argument to the linker. I've seen problems doing that before, I think. I believe for other projects I've worked on, I've moved it to the very end.

Anyway, just having a placeholder would work?

Gentoo likely used the GitHub code with configure scripts. Somebody went through the effort to do that, and I was thinking it was for that project.

paulej avatar Jul 25 '21 15:07 paulej

Modifying the Makefile to have that is easy, but there's the risk it might not work as expected. For example, what if I set it to some value in Makefile? I would generally do that. So, if Gentoo wanted to pass something in, it would get reset.

Another issue is placement. You suggested having it some place that's not the last argument to the linker. I've seen problems doing that before, I think. I believe for other projects I've worked on, I've moved it to the very end.

Again, I don't build software, and don't understand much of this. I just passed on a patch that someone suggested to produce a statically linked binary (and that person placed the LDFLAGS argument in that location). I certainly would accept a better way to do it.

I don't understand the rationale of setting defaults for LDFLAGS if you have nothing there now. I just would like some way of building a static binary from the unmodified source code by either an environment variable or passing a configuration argument.

2011 avatar Jul 26 '21 08:07 2011

Are people trying to use the GitHub code or the code on aescrypt.com? I assume the latter since that looks like the Makefile there. However, I thought that gentoo and others were using the GitHub version. I'm not sure how to set LDFLAG for that one, either, since it uses autotools. I see it there and I tried setting it via configure and it failed. I don't care to waste time figuring that out.

Anyway, this one seems odd:

 %.o: %.c %.h
-       $(CC) $(CFLAGS) -c $*.c
+       $(CC) $(CFLAGS) $(LDFLAGS) -c $*.c

But, this did the trick to allow me to create a statically linked version by setting LDFLAGS to have "-static":

diff --git a/Linux/src/Makefile b/Linux/src/Makefile
index 06a33f8..8d4334d 100644
--- a/Linux/src/Makefile
+++ b/Linux/src/Makefile
@@ -19,18 +19,18 @@ AESCRYPT_OBJS=aescrypt.o aes.o sha256.o password.o keyfile.o
 KEYGEN_OBJS=aescrypt_keygen.o keyfile.o password.o
 # Linux does not need the iconv library included, though Mac and BSD do
 ifeq ($(shell uname -s), Linux)
-    LIBS=
+    LDLIBS=
 else
-    LIBS=-liconv
+    LDLIBS=-liconv
 endif
 
 all: aescrypt aescrypt_keygen
 
 aescrypt: $(AESCRYPT_OBJS)
-       $(CC) $(CFLAGS) $(LIBS) -o $@ $(AESCRYPT_OBJS)
+       $(CC) $(CFLAGS) $(LDLIBS) -o $@ $(AESCRYPT_OBJS) $(LDFLAGS)
 
 aescrypt_keygen: $(KEYGEN_OBJS)
-       $(CC) $(CFLAGS) $(LIBS) -o $@ $(KEYGEN_OBJS)
+       $(CC) $(CFLAGS) $(LDLIBS) -o $@ $(KEYGEN_OBJS) $(LDFLAGS)
 
 %.o: %.c %.h
        $(CC) $(CFLAGS) -c $*.c

Is that what you want? Note I renamed LDLIBS just for consistency with GCC's convention since that's probably well-accepted these days.

paulej avatar Jul 26 '21 19:07 paulej

Gentoo uses the code on aescrypt.com (the releases occur there).

That looks like it will do what I (and others) want. Thank you.

2011 avatar Jul 29 '21 01:07 2011

I updated the Linux source. The SHA-256 hash is 263c0abd1da22d8cffd181a2d99c6d90410e5c2c6deeb1d6286f01b08a2f6763.

Version number was updated to 3.15.

paulej avatar Jan 29 '22 22:01 paulej

And since I was in there, I did a lot of cleanup. I just published v3.16.

paulej avatar Jan 30 '22 22:01 paulej